opentype.js icon indicating copy to clipboard operation
opentype.js copied to clipboard

feat: add SVG table support

Open kinolaev opened this issue 1 year ago • 5 comments

Description

This PR adds support for 'SVG ' table.

Motivation and Context

Fixes #106, #193

How Has This Been Tested?

I added tests to test/svg.js file. These tests parse and encode Colortube font.

Screenshots (if appropriate):

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x] I did npm run test and all tests passed green (including code styling checks).
  • [x] I have added tests to cover my changes.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the README accordingly.
  • [ ] I have read the CONTRIBUTING document.

kinolaev avatar Jan 31 '24 19:01 kinolaev

Is this to implement apple-style emoji fonts? I'll pull in connum on this one as I'm a little out of the loop but this looks good to go.

ILOVEPIE avatar Feb 10 '24 01:02 ILOVEPIE

One issue I do notice for this is the lack of integration with the drawing code.

ILOVEPIE avatar Feb 10 '24 01:02 ILOVEPIE

Hello @ILOVEPIE! It is to parse and make color fonts like Colortube or Fattern where SVG documents are embedded directly into a font file instead of being converted into glyphs. By drawing code you mean Font.prototype.draw method, right? In my opinion parsing and making SVG tables are useful features even without the ability to draw SVGs on canvas. So maybe we could leave the drawing part out of this PR? By the way SVG fonts usually have glyphs as fallback. I'll look into it and try to implement drawing SVGs on canvas in a separate PR if I have time.

kinolaev avatar Feb 10 '24 09:02 kinolaev

The purpose of the library is to parse, generate and draw fonts. This PR is incomplete imho.

ILOVEPIE avatar Feb 10 '24 11:02 ILOVEPIE

Ok, I see your point and will try to find time to implement drawing as part of this PR.

kinolaev avatar Feb 10 '24 14:02 kinolaev

The purpose of the library is to parse, generate and draw fonts. This PR is incomplete imho.

IMHO it's OK to implement parsing and writing first and drawing with an additional PR. We have other features where parsing works but writing doesn't, so here we even have parsing and writing in one go.

Connum avatar Mar 22 '24 10:03 Connum

But what I am missing is a method to retrieve the SVG document in plaintext, using extDecoder() as is done for the tests. This should be done in a deferred way using defineProperty() and getters/setters, so we only have to do that calculation when the data is needed, like we already do in other places like glyph paths.

Connum avatar Mar 22 '24 17:03 Connum

Nice! However, rendering of the Colortube-lWAd.otf is working in the font inspector and the two top canvases on the main page, but not the glyph inspector (or the glyphs on the main page) for me. Can you look into this?

Two optimizations we should implement for performance:

  • Only de-gzip and build the image when it is requested for a given glyph
  • Use the native DecompressionStream instead of tiny inflate if available (recent browser and node versions)

We can implement the second one independently from this PR at a later time, but it would be great if we could have the lazy loading from the start.

Connum avatar Mar 24 '24 12:03 Connum

There also seem to be scaling issues when rendering SVG data in the following fonts from https://github.com/unicode-org/text-rendering-tests/tree/main/fonts

Connum avatar Mar 24 '24 12:03 Connum

I've added the ability to draw SVGs. Before drawing SVG images must be prepared by calling await font.prepareSVGImages(). It will render SVGs as HTMLImageElements and cache them. It's also possible to pass an array of SVGDocumentRecord to that function to prepare images only for specific records.

I've exported decodeDocument function to retrieve the SVG document in plaintext. As SVGs can be gzipped, I've also added utility functions to determine and uncompress a gzip buffer (using tiny-inflate).

@Connum, you wanted to have a getter svg on Glyph, is it right? The problem is that one SVG document can contain multiple glyphs. If you want to get an SVG document for a specific glyph, it must also be transformed after decoding (see makeGlyphSvgTemplate function). Transformed SVG documents in plaintext are available in data URL of prepared images (font.tables.svg.images.get(glyphId).image.src). Could you please explain a bit further, where you want me to add a getter and what exactly it should return?

kinolaev avatar Mar 24 '24 12:03 kinolaev

When getting the path for a glyph, we do this via a getter that will calculate the path once when it is requested for the first time, and then returns a cached version of it. We should do the same for the SVGs per glyph instead of preparing all of them at once, which could take quite some time depending on how large and complex the SVG font is.

Connum avatar Mar 24 '24 12:03 Connum

For example, NotoColorEmoji-SVG.otf is freezing the browser for a considerable amount of time.

Connum avatar Mar 24 '24 12:03 Connum

Thank you @Connum for the feedback! It's difficult to prepare images lazily, because draw is synchronous and prepareSvgImages is asynchronous... But the problem is clear to me now and I'll try to figure it out.

kinolaev avatar Mar 24 '24 13:03 kinolaev

I'm currently working on COLR/CPAL font rendering (so far we only support parsing and writing the tables), where I will also implement lazy loading of the layers. I hope to have this ready in the next days, then you could take some inspiration from it.

Connum avatar Mar 25 '24 22:03 Connum

It will be interesting to see your lazy loading implementation. But before I've read your message I've already found a way to implement it.

It is now possible to add onload callback to an SVG table that will be called with a glyphId when an image for the corresponding glyph is ready. In that callback you can decide whether you need to redraw anything. The image loading only runs when svg.getImage is called. The result is cached. Does it looks about right to you?

I haven't solved the scaling problem with the fonts you mentioned above yet. I'll look at it later.

kinolaev avatar Mar 26 '24 15:03 kinolaev

I hope I'll finish the cpal/colr overhaul today or tomorrow. Maybe it will help you fix the performance issue.

Connum avatar Mar 26 '24 15:03 Connum

Haven't created a PR yet, but you can already take a look at my current work in progress here: https://github.com/Connum/opentype.js-next/tree/feature/colr-cpal-rendering

regarding the lazy loading, especially: https://github.com/Connum/opentype.js-next/blob/feature/colr-cpal-rendering/src/glyphset.js#L136

Connum avatar Mar 27 '24 02:03 Connum

Thank you, @Connum , I've added svgImage getter. Does it look good to you? I've also implemented decompression using native DecompressionStream when it's available.

kinolaev avatar Mar 27 '24 18:03 kinolaev

We should definitely support all of the different methods of doing emoji that have been implemented into fonts, although I will say that only Apple supports the SVG table currently.

ILOVEPIE avatar Mar 27 '24 20:03 ILOVEPIE

We should definitely support all of the different methods of doing emoji that have been implemented into fonts, although I will say that only Apple supports the SVG table currently.

I'll have a PR for CPAL/COLR v0 rendering ready by (hopefully) tomorrow. v1 is quite complex though.

Connum avatar Mar 27 '24 20:03 Connum

There also seem to be scaling issues when rendering SVG data in the following fonts from https://github.com/unicode-org/text-rendering-tests/tree/main/fonts

Scaling issues have been resolved. Is there anything anything else you'd like me to improve?

kinolaev avatar Mar 28 '24 08:03 kinolaev

Can you please resolve the conflicts with the current master in order to do a rebase? Then we can have another look at it! Thanks!

Connum avatar Apr 08 '24 20:04 Connum

The conflicts are resolved. I took inspiration from your work and made three changes:

  • added drawSVG font render option
  • added getSvgImage method to Glyph class
  • moved all svg image creation code to SVGImageManager class

kinolaev avatar Apr 09 '24 17:04 kinolaev

Thank you! I already spotted a few things, but I'll do a full review soon, hopefully in the course of this week.

Connum avatar Apr 09 '24 22:04 Connum

Thanks for the detailed review! I'll try to address all the issues you mentioned by the end of next week.

kinolaev avatar Apr 10 '24 12:04 kinolaev

TODO and warning are added to Path.prototype.toSVG method 2d56eb26a1e99f8ca79138210bef8559e8f9141e. I think I've answered all your comments. Please let me know if I've missed anything.

kinolaev avatar Apr 12 '24 13:04 kinolaev

Thanks again for your work and your patience. I'd love to see more contributions in the future!

Connum avatar Apr 12 '24 15:04 Connum

Thank you for your time and attention. It was a pleasure to collaborate with you on this task!

kinolaev avatar Apr 12 '24 15:04 kinolaev