Add support for SVG icons

[BillyDM]

I was really struggling to figure out how to create a custom icon font for my GUI library that actually worked with cosmic-text, so I just went ahead and implemented a feature to render SVG icons in glyphon as mentioned in #100.

I've managed to reuse the same text atlas, pipeline, and shader that the text renderer uses. All that was added is an IconRenderer and an IconSystem struct that behaves analogous to the TextRenderer and FontSystem structs.

I've also added a helper struct to simplify loading and parsing SVG icons.

[BillyDM]

On second thought I think the helper struct to load SVGs is unnecessary, so I removed it.

[grovesNL]

Thanks @BillyDM, this is really cool! I need to think about how we should expose this a bit, because people may want to bring their own SVG/vector crate but still be able to share the glyphon internals in some way.

One option is to expose some way to send glow a custom glyph to be used at a specific position. This way we could accept any kind of image instead of tying it to resvg. What would you think about that option?

I'd also like to make this design compatible with eventually being able to have inline boxes in cosmic-text (pop-os/cosmic-text#80), so custom icons could be added inline with text.

[BillyDM]

Ah yeah, it would be much better if there was a way to inline the custom icons with regular text (and possibly render icons directly in TextRenderer). Although making that compatible right now would be tricky.

As for the user being able to use any crate to render custom icons, perhaps we can just have the user pass in a closure that takes an icon ID and a size as input and raw pixel data as output?

[BillyDM]

Actually, I just got an idea how to make this work. I'll go ahead and do that (and also rebase to the latest commits).

[BillyDM]

Figured out how to properly rebase in VSCode this time :)

[BillyDM]

Alright, I reworked the API. Now there is no IconSystem or IconRenderer. Instead you just pass in extra InlineBox structs into TextRenderer::prepare, as well as a closure to rasterize the glyph.

[BillyDM]

Actually, thinking about it, if cosmic-text gets inline boxes, they would be part of the text buffer. So I simplified things a bit. Now you just pass in a vec of CustomGlyphDesc to TextRenderer::prepare.

[grovesNL]

Thanks for working through the design a bit!

I'd like to avoid putting any of the SVG-related logic in glyphon to keep it flexible. It seems like the API should allow the rasterized output + coordinates to be passed to glyphon instead of glyphon having optional dependencies.

[BillyDM]

Alright, I removed the SVG-related logic.

[BillyDM]

I realized I forgot to account for scale factor, so I added a quick fix for that.

[BillyDM]

Are you happy with this API, or do you want to workshop this some more?

[grovesNL]

@BillyDM Thanks! I'll try to take another detailed look soon.

[grovesNL]

I had some time to go through this in detail and it looks great so far. I had some ideas that might improve this a bit - please let me know if anything isn't clear.

It seems like it would be compatible with the ideas about inline boxes on the cosmic-text side, so that's really cool.

[grovesNL]

I'm not sure about relying on dummy() and the CacheKeyFlags representation. Maybe we could add an enum with something like enum CacheKey { Text(CacheKey), Custom(CustomGlyphCacheKey) }? The overhead might be 1 byte or so per entry, but that seems ok based on typical cache sizes.

[BillyDM]

Yeah, that seems cleaner and more flexible. I'll do that.

[BillyDM]

Ah, I also realized while doing this that I'm not currently getting the rasterized custom glyph data in InnerAtlas::grow() and this line will panic. https://github.com/grovesNL/glyphon/blob/main/src/text_atlas.rs#L160

I suppose I need to store the rasterized custom glyphs in a hash map.

[BillyDM]

Or wait, actually it seems like you are using SwashCache.get_image_uncached().

Does that mean that all glyphs should be re-rasterized whenever the atlas grows?

[BillyDM]

I just went ahead and did what glyphon does for text and re-rasterize the glyphs.

[grovesNL]

Ok great. Yeah we don't want to keep two versions around in memory but also don't want to try to pull existing images back out of the atlas during grow, so this is probably about as good as we could do it right now.

[grovesNL]

We could probably pass the original position down and let the rasterizer decide how they want to do binning. This way they could still apply cosmic-text's SubpixelBin if they want, but I'd guess a lot of the time for images they'd skip the binning and just snap to the nearest left/top pixel anyway

[BillyDM]

This won't work because you would need to have the position as part of the cache key, which would potentially lead to a whole lot of atlas entries being created.

I think the better solution is to add a flag to CustomGlyph saying whether or not you want to snap the glyph to the nearest whole pixel.

[grovesNL]

nit: CustomGlyphRasterizationRequest or something

[BillyDM]

How about just RasterizationRequest?

[grovesNL]

Sure, I'd just like to distinguish between rasterization requests for regular glyphs vs. custom glyphs somehow

[BillyDM]

Alright, I finished making improvements.

[grovesNL]

Alright this seems ready now, thank you!

[hecrj]

What is the rationale for glyphon to adopt this functionality?

It seems a bit strange to introduce the ability to draw absolutely positioned bitmaps through a text rendering crate.

I believe complexity would be greatly reduced if the library dropped this feature and instead exposed more parts of the TextAtlas (I think there is demand for a nice wgpu atlas crate). This way, a TextAtlas could be leveraged independently to render SVGs (and other cached primitives) in a different draw call, and externally to glyphon.

[grovesNL]

@hecrj my thought was that we’ll eventually have support for inline boxes in cosmic-text which would pair really nicely with this (automatic positions instead of the absolute coordinates).

I’d also be up for exposing the atlas and other pieces to do this externally, and/or provide helpers. Maybe there could be a nice hybrid that still allows for the inline boxes to be used with custom glyphs easily. If we had those parts exposed then we could probably change the API that we added for custom glyphs here. What would you think about something like that?

[hecrj]

@grovesNL I was just thinking that was problably the motivation.

However, I think for inline boxes I'd expect cosmic-text to tell me the box coordinates once laid out so that I can fill in the gaps; all externally to glyphon.

I think this is the only reasonable approach for GUI toolkits like iced, since sometimes we will want widgets themselves (and custom primitives) to be inlined with text and expecting glyphon to render these is clearly out of scope.

[grovesNL]

Yeah that makes sense too. I think I’d like to support both approaches – it seems common enough to want to insert a bitmap inline that it’s probably worthwhile to make it easier in glyphon instead of passing the position through. I’m not tied to any particular shape for the API though.

[hecrj]

Fair enough!

It blurs the boundaries of the library a bit. Up until now it was all glyph rendering; and thanks to that choosing etagere as an allocator made sense. If users can start uploading trivially sized bitmaps, the allocator may not perform as well.

In any case, I'll think about a potential shared atlas API. We use a couple atlases for raster and vector images in iced, although those leverage guillotiere.