Improvements to Glyph Rendering in SVGContext / CanvasContext

[ronyeh]

This thread has evolved into discussing ways to improve Glyph rendering. I'll keep this issue open for now, even though there is no bug to fix. If we prefer to move it to the GitHub discussions tab, I can close off this issue.

---

Original title:

Should OutlineCode enum (in glyph.ts) use ints other than 0, 1, 2, 3?

This isn't a bug, but I was reading glyph.ts and saw that GlyphOutline.parse() returns a number[] where the position of the number changes its meaning.

For example, Bach Demo produces an array like:
[0,0,60,3,140,180,0,135,62,180,3,425,60,268,180,425,62,3,285,180,425,134,367,180,3,0,60,127,180,0,63]

The first 0 is actually a "move to" instruction, and the second 0 is an x coordinate with value 0, and then comes a y coordinate with value 60. The 3 after that starts a bezier curve....

Then, Glyph.renderOutline() and Glyph.getOutlineBoundingBox() loop over these outline arrays to draw the glyphs to the context.

    while (!go.done()) {
      switch (go.next()) {
        case OutlineCode.MOVE: // => 0
          ctx.moveTo(go.nextX(), go.nextY());
          break;
        case OutlineCode.LINE: // => 1
          ctx.lineTo(go.nextX(), go.nextY());
          break;
          ...

So the switch statement is looking for numbers 0, 1, 2, 3.

Anyways, like I said, this isn't a bug because the three methods all follow the same convention (a number 0 implies an x, y and a number 3 is followed by 6 ints specifying the points of a cubic bezier curve).

I know it was part of @tommadams massive improvements on glyph performance & correctness in #1103 and #1077.

Obviously we don't want to go back to the old / slow way.

But do we have any ideas on if / how we could improve this, or make it potentially less brittle / future proof?

I can't think of anything off the top of my head at the moment, other than starting the enum at a really big number (that is still well under Number.MAX_SAFE_INTEGER), like:

export const enum OutlineCode {
  MOVE = 0xfffffffffff0,
  LINE = 0xfffffffffff1,
  QUADRATIC = 0xfffffffffff2,
  BEZIER = 0xfffffffffff3,
}

Anyways, can we improve this in any way? If not, I'll close as "not a bug."

[tommadams]

For what it's worth, the approach of packing commands and command arguments into a data stream is ubiquitous in space or time critical software: video games, computer graphics applications, audio & video codecs, etc. Somewhat more relevant to this discussion, a TrueType font rendering implementation must actually be Turing complete: TrueType font files contain a mix of both curve data and instructions for fairly complex stack machine.

Philosophically, one could take the view that GlyphOutline.parse is a kind of primitive compiler, which transforms source code (the string data stored in the glyph) into a an instruction stream to be interpreted, similar to, say, Java byte code.

I'm curious as to concretely what you think a problem might be? The one hypothetical case I can imagine is that someone writes a new function to interpret the glyph data and doesn't get the number of operands to one of the functions correct. I think it would be pretty obvious if that happened and the cause of the bug would be localised. It's unlikely that all the subsequent read values would correspond to valid op codes, so one way to improve the developer experience there might be to add a default clause to the switch statement that threw an exception to Glyph.renderOutline() and Glyph.getOutlineBoundingBox().

Changing the enum values might make things more obvious but I'd suggest using smaller constants. Even though the values you chose are smaller than Number.MAX_SAFE_INTEGER, they're too large to be represented as small integers in the V8 runtime (and possibly other JavaScript engines too, but I'm less familiar with those). Consequently, every one would get boxed and incur a heap allocation, rather than be packed into the engine's 64bit value representation.

[tommadams]

I will just say that after having worked on it a little, I personally think a more confusing and problematic issue with the glyph code is that the order of the values for the quadratic and cubic Bézier curve commands does not match the parameter order for any of the functions that they're passed to! ;)

[ronyeh]

Hello Tom!

Thanks for your insightful reply, and for your improvements to VexFlow.

I agree there is no bug and I'm fine leaving it as is. It reminds me of when I learned about the MIDI spec. At some point I was able to recognize the hex sysex messages that my devices were sending, haha.

Summary: I was using console.log() on the parsed glyph data and was surprised to see only numbers (I'm used to seeing SVG path strings). It felt wrong at first, because 0 could be an x or y coordinate, but it can also be a move-to command. Maybe we can just add a comment in glyph.ts that explains how the three switch() statements work in parallel.

---

LONG STORY: I was trying to learn how the glyph internals worked (how we go from an OTF file to a bravura_glyphs.ts or petaluma_glyphs.ts file to the render functions to the final canvas or svg output.

I was trying to debug all the ugly NOGLYPHs in the Petaluma font, so I threw together a font sample page that renders all the glyphs that VexFlow cares about. (See image at the bottom. The red glyphs are missing from the tested font, and are drawn using a fallback font.)

1. When we first read the OTF font files, we get something similar to SVG paths. Though curiously, VexFlow converts the C / [c]ubic bezier opcode to B for [b]ezier :-).
2. Then we parse these path strings into number arrays, with numeric opcodes instead of the original M | L | Q | B. Keeping it as all numbers was a great improvement, performance wise.
3. When it finally comes time to render the number[] onto a webpage, we pull the instructions and [x, y] points out of the array and call methods like bezierCurveTo().
4. The funny thing is, with the SVGContext, methods like bezierCurveTo() end up building a path string (using C again instead of B for the cubic bezier curves). So if we are using an SVGContext, we end up going from OTF => path strings => number arrays => moveTo / lineTo / bezierCurveTo / quadraticCurveTo methods => path strings.

And you are correct that the order of parameters is switched up. The SVG path spec for cubic bezier curves is:

C x1 y1, x2 y2, x y

VexFlow's smufl_fontgen.js writes the same command as:

b x y x1 y1 x2 y2

VexFlow's SVGContext and CanvasContext declare bezierCurveTo to be consistent with CanvasRenderingContext2D's bezierCurveTo():

bezierCurveTo(x1, y1, x2, y2, x, y)

I wonder if there's a more direct way to go from the OTF path strings to our SVG path strings? We might have to worry about the y axis. I see that smufl_fontgen.js inverts the y axis while it generates the original bravura_glyphs.ts file. (And our glyph.ts code also does some "inverting" since it subtracts the y value from the originY.)

Canvas can also accept SVG path strings via the Path2D API, so someday it would be nice if our bravura_glyphs.ts file contained paths that could be used directly (in SVG path, or Canvas Path2D).

Thanks for reading my brainstorming :-).

Font Glyph Table

I was just tired of seeing the NOGLYPH glyphs.... so I downloaded the most recent Petaluma font from Steinberg and generated a new petaluma_glyphs.ts file just to see if any glyphs were added.

Then I realized we didn't have a good way to visualize all the available (or missing) glyphs in our three fonts, so I hacked up a glyph table. On the test page, if I mouse over a glyph, it console.logs the glyph code, so I can see which exact glyph is missing.

At some point I'll clean this up... and maybe turn it into a PR.

[tommadams]

Thanks for digging into the glyph code, those missing glyphs have been bugging me too.

I agree that the string -> number -> string conversion is less than ideal. This is partly because the RenderContext abstraction is at slightly the wrong level in my opinion: because RenderContext exposes low level methods like moveTo and bezierCurveTo, the outline data has to be converted from strings to numbers.

Instead, if the RenderContext simply exposed a drawGlyph method, the SvgContext could embed each outline exactly once in the SVG using a <def> tag, then draw each glyph with <use> tags. So the SvgContext wouldn't have to process the outline at all and only the CanvasContext would have convert from string -> number. You'd still need to convert from string -> number once to calculate the glyph bounding boxes but I think you could gain substantial performance improvements.

I've been thinking about this idea for a few weeks now, maybe I'll actually finally try it out...

[ronyeh]

Wow if you investigate the svgcontext def/use, I'd definitely help with the code review (if it would help in any way). Maybe our CanvasContext could internally cache glyph outline strings, and then reuse them via Path2D every time drawGlyph() is called. This would parallel the SVG def/use approach.

…

On Fri, Sep 3, 2021, 7:22 AM Tom Madams ***@***.***> wrote: Thanks for digging into the glyph code, those missing glyphs have been bugging me too. I agree that the string -> number -> string conversion is less than ideal. This is partly because the RenderContext abstraction is at slightly the wrong level in my opinion: because RenderContext exposes low level methods like moveTo and bezierCurveTo, the outline data *has* to be converted from strings to numbers. Instead, if the RenderContext simply exposed a drawGlyph method, the SvgContext could embed each outline exactly once in the SVG using a <def> <https://developer.mozilla.org/en-US/docs/Web/SVG/Element/defs> tag, then draw each glyph with <use> <https://developer.mozilla.org/en-US/docs/Web/SVG/Element/use> tags. So the SvgContext wouldn't have to process the outline at all and only the CanvasContext would have convert from string -> number. You'd still need to convert from string -> number once to calculate the glyph bounding boxes but I think you could gain substantial performance improvements. I've been thinking about this idea for a few weeks now, maybe I'll actually finally try it out... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1127 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2MCJJOFW6YSC7QRHIPZDUADKZ5ANCNFSM5DKFWCHA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[ronyeh]

An alternative approach for caching glyphs in canvas would be to draw the glyph into a hidden buffer and then use the drawImage method to place the image data every time we need to render the glyph again. This would be useful for canvases with lots of reused glyphs, and maybe would help if vexflow ever decides to support animation (ver. 5 hehe).

[tommadams]

I spent a couple of hours hacking on it this morning and it looks like the approach could work. It seems like you have to use <symbol> instead of <defs> because elements in <defs> can't be rescaled.

Here's what I got so far:

Elements in red are drawn using <symbol> and <use>. They're all back to front, upside down and the wrong size but those are bugs that can be squashed.

[ronyeh]

Beautiful!

I'm actually curious about the ultimate performance. Will it be faster (or slower)? In the end, we are swapping a bunch of inline <path> elements for an equal number of <use> elements, and we add a <symbol> element for each unique glyph used by the score. It depends ultimately on how fast <use> duplicates the symbols.

[ronyeh]

The huge performance improvement will be that the SVGContext's drawGlyph() will just directly add another <use> element for the 2nd to nth time it draws a glyph. It won't have to rebuild the path string by looping over the moveTo / lineTo / etc instructions.

[tommadams]

Quick update.

I've got the glyphs drawn using <symbol> and <use> elements drawing correctly:

Only elements that are drawn via Glyph.renderGlyph are hitting this codepath for now, which is why things like the clefs and key signatures are still drawn in black.

I still don't what the performance is like yet; due to the way I've hacked this in, the rendering code is having to do a lot of unnecessary work for every glyph rendered. I haven't yet figured out a way to integrate this cleanly, I think it might require turning the glyph rendering completely inside out and replacing all the render methods on Glyph with methods on RenderContext. I was hoping to avoid that kind of invasive refactor though.

Unfortunately, the outline data still has to go through the string -> number -> string transformation because the y values all need to be negated since the <use> element can't scale by negative amounts. However, we only have to pay for this conversion one time for each glyph when adding the <symbol> element to the SVG.

I'll keep poking at it.

[ronyeh]

Cool!

I have some smaller ideas for improving the glyph pipeline. I think you've inspired me to look into them at some point.

For example, I think the bravura_glyphs.ts, petaluma_glyphs.ts, etc should be storing SVG paths. That means we'd need to:

• update smufl_fontgen.js to use C instead of B
• update glyph.ts's parse method to look for C instead of B
• make sure the outlines are the correct orientation, so that they aren't drawn upside down or mirrored
• make sure the order of coordinates is correct for the curve functions (e.g., C x1 y1, x2 y2, x y)

One benefit would be that I could copy the outline ("o") field from the bravura_glyphs.ts file and paste it into a SVG path element and see it show up in my browser immediately. That would enable easy debugging of the glyphs. Maybe it would also simplify your rendering code a tiny bit (can we avoid the negation of the y values if they are pre-negated in the bravura_glyphs.ts file?).

[tommadams]

Storing the SVG path in the *_glyphs.ts files would be great. And I am 100% behind the idea of reordering the coordinates to match what the RenderContexts expect!

To completely avoid any processing of the o field by the SVGContext, we'd also need to store the bounding box for every glyph too. This is because a <symbol> SVG element must have a viewBox attribute defined, otherwise the <use> can't resize the elements (see the note about width and height in the <use> documentation). I don't know whether the performance gains from avoiding parsing the o path string would outweigh the extra space required to store the view box data, so maybe leave that until later.

The other wrinkle is that the CanvasContext will have to be updated to parse the SVG path but that shouldn't be too difficult.

[tommadams]

Oh, and yes, avoiding the negation of the y values would definitely simplify the rendering code. I spent about an hour tracking down a bug where the dynamicPiano glyph wasn't being rendered correctly because I had messed up calculating the bounding box. The notehead glyphs only happened to draw correctly because their SVG paths were centered vertically.

[tommadams]

So I cleaned up the rendering code and removed all the extra unnecessary work and the performance is... terrible unfortunately.

When the test page is running, Chrome spends nearly all of its time stuck in "recalculate style" and "layout" :

Each of those blocks is around 150ms and there are thousands of them. After the tests had been running for about a minute, I tried pausing JavaScript execution and the Chrome process running the page crashed :(

I did a bit of digging and the "recalculate style" seem to be triggered by calls to measureText so I tried creating an offscreen SVG element and using that solely for text measurement but that didn't have any effect.

I'll try poking around some more but at first glance it seems like <use> may be unusable :(

[ronyeh]

Bummer.

I found an old discussion from 6+ years ago about performance with SVG use symbol. The comments suggested that symbol might be slower than just drawing tons of paths directly. See: stackoverflow: svg performance with symbols

• there has to be some minimal overhead to keep track of the connection beween a symbol and each <use>.
• and what happens if the original symbol changes? The browser's renderer has to propagate all changes to each <use> instance.

In measureText, there is a line:

    // Temporarily add it to the document for measurement.
    this.svg.appendChild(txt);

I guess that's why you tried the offscreen SVG approach, instead of modifying the SVGContext's internal svg element.

What if we just comment out the entire measureText() method and return a constant rectangle?

    const box = {
      x: 0,
      y: 0,
      width: 12,
      height: 15,
    };
    return box as SVGRect;

Does that have any effect?

In any case, please don't delete your branch and hard work, haha. Maybe we can look at your approach and learn something from it. If you don't mind sharing your code at some pont, I'd love to take a look.

[ronyeh]

Here's another almost 7 year old blog post about SVG performance, by a Khan Academy engineer. I thought it was an interesting read:

https://www.crmarsh.com/svg-performance/
"...applying linear transformations to SVG elements does trigger re-layout and re-painting"

Summary: at least 7 years ago, major browsers didn't seem to focus on SVG render performance. They were able to make huge gains in (animation) performance by putting a static SVG inside a DIV, and then moving that DIV around.

So it's possible that changes to the SVG element trigger lots of relayout and repainting. We could build a small svg symbol/use experiment and use Chrome's Rendering > Paint flashing checkbox to draw green rectangles every time a repaint happens.

What would happen if we just built a SVG string in memory, without ever calling this.element.appendChild(svg)? String building with <symbol> and <use> would be pretty fast. Then, after all the "rendering" (aka string building) is done, we finally display the finished SVG with a single update to this.element.innerHTML?

[tommadams]

Some good news, it turns out that the terrible performance was caused by multiple <symbol> elements in different SVGs sharing the same ID. I was simply assigning the glyph name to the symbol ID under the assumption that <use> references would be local to the SVG they were in but that turns out not to be the case.

After changing all the <symbol> IDs to be unique, running all test tests takes about 12 seconds on my machine, which is the same as before making this change.

Looking at a profile, it seems like measuring text still causes loads of "recalculate style" events inside Chrome but these complete in microseconds instead of milliseconds.

[rvilarl]

vexflow5 branch no longer renders glyphs in SVGContext/CanvasContext.