Allow loading typefaces into SkiaRenderContext manually #2123

mennowo · 2025-01-24T08:19:11Z

Fixes #2118, by allowing loading fonts embedded in an arbitrary external library to be explicitly loaded into SkiaRenderContext.

Checklist

I have included examples or tests
I have updated the change log
I am listed in the CONTRIBUTORS file
I have cleaned up the commit history (use rebase and squash)

Changes proposed in this pull request:

Add a new public method AddTypeface to SkiaRenderContext to allow manually loading fonts

Remarks

There is no test, since I am unsure how to create one. I got started, but most of the logic/variables used is private to SkiaRenderContext and therefore not accessible from a test.

I rebased, but I am unsure how to clean up the commits that are in this PR now. I am happy to do that if someone can provide me with some pointer on how to proceed, or maybe it can just be squash-merged?

After this has been merged, I will also update this repo, so that OxyPlot is fully usable in a WebAssembly context. In case of interest, I am having great results with the altered version of OxyPlot in the browser; it looks nice, it's reactive, it's fast.

I'd be happy to merge that solution into oxyplot as well, but maybe it creates too much extra dependencies; let me know if there is interest in that regard. I might also create a nuget package for the OxyPlot.SkiaSharp.Blazor if I find the time.

@oxyplot/admins

In the past, there was ColumnSeries, which could have the categories be placed on the X axis. The BarSeries needs them on the Y axis. The new example show how to do this, but use keys to have to categories on bottom axis in the plot anyway. Update CONTRIBUTORS Update CHANGELOG.md

…into develop

VisualMelon · 2025-01-24T11:27:19Z

@Jonarw are you able to give this a bit of time? No serious precedent for the API but looks OK to me (PortableDocument.AddFont is the only thing that's obviously similar)

Jonarw

I think this is a good idea, nice that there is a simple way to make this work on WASM. See my one comment regarding API.

Regarding squashing / rebasing: A git rebase origin/develop should usually do the trick. But in this case I think a squash-merge should also do the trick, so no need to do anything on your end.

Jonarw · 2025-01-25T08:24:08Z

Source/OxyPlot.SkiaSharp/SkiaRenderContext.cs

+        /// <param name="fontWeight">The weight of the font (eg.: 400 for Regular, 700 for Bold; these are the only ones OxyPlot actually uses)</param>
+        /// <param name="typeface">Typeface instance to be added to cache, typically loaded from some embedded resource, eg.: <code>var typeface = SKTypeface.FromStream(stream);</code></param>
+        /// <returns>True if succesful, false if typeface with this family name and weight was already in cache</returns>
+        public bool AddTypeface(string fontFamily, double fontWeight, SKTypeface typeface)


This API does not allow replacing a previously added typeface with a new one. Is this a problem?

Maybe the function should always update the typefaceCache with the new typeface, and the return value should reflect if the typeface was added or if it already existed - what do you think?

Short of adding add/set/remove methods or hiding this behind some wrapper collection type, I'm not sure what would make sense.

Looking at this more closely - and thinking about how we might want to do this in other platforms - I don't think working with the typefaceCache makes sense either. I'd suggest we move the logic for looking up a type face (in GetTextPaint at the moment) into a small virtual method so that it has an extension point, and consider adding a separate dictionary of 'override' lookups, which - where available - are used instead of querying SKTypeface.FromFamilyName.

Extension point is worth it because a user may want to support a range of custom fonts, and if e.g. WASM needed it's own weird lookup mechanism then it could be implemented once and forgotten about. Dictionary is worth it to cover symbol cases, and it can just be an IDictionary<FontDescriptor, SkTypeface> if we want to keep it simple and not think about the API too hard.

Ok I'm happy to give this a go. This would mean making FontDescriptor public, which at the moment it is not.

One question: why would one need a seperate dictionary of 'override' lookups? To allow removing an added typeface later? Or some other use case? I would think typically one would load a font and expect it to be used to render a plot; wether or not it overrides something else is kind of irrelevant. Is there a use case for wanting the exact same FontDescriptor to later be related to a different typeface? From a performance perspective, I would want only fonts I need to be loaded and only once (which is also why I cache them in WASM, where I have multiple simultaneous instances of the PlotView component, and therefor of SkiaRenderContext).

It's an override because it will preclude looking up the font separately, but as you say that's not the main use-case (but we do have to define how it will behave if there is an existing font etc.).

Using the typefaceCache feels dodgy because it's intended as a cache, and currently disposes its elements when the SkiaRenderContext is lost: if OxyPlot didn't create the instance then we need to decide whether it should be disposing it (indeed, may be a problem for your use-case, but I don't know enough about SkiaSharp to really comment).

Personally I'd have no issue with making FontDescriptor public, but I could imagine that we might want a common FontDescriptor type for other platforms long term. That said, I'm not wedded to the idea of exposing an IDictionary'2: was really just throwing a simple idea out in hopes of some conversation and I'll generally defer to Jon on any Skia matters anyway.

Ok, yes, the fact that SkiaRenderContext will call Dispose() on elements in the cache may present an issue. I will try to find some time the coming 2 weeks to have a go at this.

@VisualMelon Looking into the now, I wonder why we need "a separate dictionary of 'override' lookups". Doesn't having an extenson point already solve most use cases? It allows writing a class that derives from SkiaRenderContext and then override public virtual SKTypeface GetTypeface(FontDescriptor fontDescriptor) (which I now implemented). Then, one can add any logic, caching, etc., to that derived class.

If there is a use case for having some sort of override lookup mechanism, I'm happy to look into it, but as it stands I don't really see the benefit, neither understand exactly what the requirements/specification for this would be.

Shall I remove the previous method AddTypeface?

From my point of view it should be enough to have the virtual SKTypeface GetTypeface(FontDescriptor fontDescriptor). As you say, derived classes can do their own caching as necessary.
I think the bool AddTypeface(string fontFamily, double fontWeight, SKTypeface typeface) should be removed then.

…ace retrieval

Jonarw · 2025-02-16T17:04:18Z

Source/OxyPlot.SkiaSharp/SkiaRenderContext.cs

    using System;
    using System.Collections.Generic;
    using System.Diagnostics;
+    using System.Drawing;


I don't think this is needed?

Ok I just commited these changes. Could it be merged like this? It would be nice if this makes it into a new release so I can switch back to using the nuget package instead of custom built dll. However, there is no rush. Also, I'm happy to add additional feedback if more changes are needed.

…peface method

Jonarw · 2025-02-25T19:24:29Z

CHANGELOG.md

 ### Added

 - Example to demonstrate how to create vertical BarSeries
+- Add a public method `AddTypeface` to `SkiaRenderContext` to allow manually adding typefaces to the cache


I think this line is obsolete now, otherwise this looks good to me now!

true, I can remove it later this week.

VisualMelon

I'm certainly happy with the changes as they are

mennowo · 2025-03-11T19:15:14Z

Ok nice, I can correct the change log later this week, and then it can be merged

mennowo added 6 commits December 11, 2024 15:05

oxyplot#2116 add full example for vertical bar series

03a70c6

oxyplot#2116 Alter version comment for most recent changes

9fac088

Allow manually loading typefaces into SkiaRenderContext

562fd51

Allow manually loading typefaces into SkiaRenderContext

31ca1ca

Merge branch 'develop' of https://github.com/CodingConnected/oxyplot …

53ac016

…into develop

mennowo mentioned this pull request Jan 24, 2025

Using new skiasharp support in Blazor 6 for Oxplot web component- A thought, not an issue. belucha/oxyplot.blazor#7

Open

Jonarw reviewed Jan 25, 2025

View reviewed changes

oxyplot#2123 Add virtual method GetTypeface to allow overriding typef…

5821f8f

…ace retrieval

Jonarw reviewed Feb 16, 2025

View reviewed changes

mennowo added 2 commits February 19, 2025 20:40

oxyplot#2123 remove AddTypeface method, add summary for virtual GetTy…

13eeee4

…peface method

oxyplot#2123 Remove using Systen.Drawing

7f75430

Jonarw reviewed Feb 25, 2025

View reviewed changes

VisualMelon approved these changes Mar 11, 2025

View reviewed changes

Allow loading typefaces into SkiaRenderContext manually #2123

Are you sure you want to change the base?

Allow loading typefaces into SkiaRenderContext manually #2123

Uh oh!

Conversation

mennowo commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Changes proposed in this pull request:

Remarks

Uh oh!

VisualMelon commented Jan 24, 2025

Uh oh!

Jonarw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualMelon Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mennowo Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualMelon left a comment

Choose a reason for hiding this comment

Uh oh!

mennowo commented Mar 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mennowo commented Jan 24, 2025 •

edited

Loading

VisualMelon Jan 25, 2025 •

edited

Loading

mennowo Feb 19, 2025 •

edited

Loading