vexflow

vexflow copied to clipboard

• build e3d0eb8f7eab7900b645245adfa61a2a680fc015 and load (Reproduce in chrome, safari, and FX): {TabSlide › Simple TabSlide › SVG + Bravura} (Assume that all tests that use the MeasureTextCache are affected)

1. tests/flow.html#TabSlide.Simple_TabSlide.Bravura
2. tests/flow.html?module=TabSlide#TabSlide.Simple_TabSlide.Bravura

• some width or position attributes are different like follows:

---

with this dirty patch:

diff --git a/src/svgcontext.ts b/src/svgcontext.ts
index f600a6ea..45bf23a0 100644
--- a/src/svgcontext.ts
+++ b/src/svgcontext.ts
@@ -45,6 +45,11 @@ export interface State {
   lineWidth: number;
 }
 
+interface SVGTextMeasure extends TextMeasure {
+  svg: SVGSVGElement;
+  propsStr: string;
+}
+
 class MeasureTextCache {
   protected txt?: SVGTextElement;
 
@@ -70,6 +75,12 @@ class MeasureTextCache {
       entry = this.measureImpl(text, svg, attributes);
       entries[key] = entry;
     }
+
+    const entry1 = this.measureImpl(text, svg, attributes);
+
+    if (entry.width !== entry1.width) {
+      console.log(`entry: ${JSON.stringify(entry)}, entry1: ${JSON.stringify(entry1)},`);
+    }
     return entry;
   }
 
@@ -97,7 +108,15 @@ class MeasureTextCache {
     // CSS specifies dpi to be 96 and there are 72 points to an inch: 96/72 == 4/3.
     const height = Font.convertSizeToPixelValue(fontSizeInPt);
 
-    return { width: bbox.width, height: height };
+    const ret: SVGTextMeasure = {
+      width: bbox.width,
+      height: height,
+      svg: svg,
+      propsStr: JSON.stringify({
+        viewBox: svg.getAttributeNS(null, 'viewBox'),
+      }),
+    };
+    return ret;
   }
 }
 

multiple problems(entry.width and entry1.width should always be the same, right?) are detected, as follows:

entry: {"width":7.4166669845581055,"height":13.333333333333332,"svg":{},"propsStr":"{\"viewBox\":\"0 0 333.3333333333333 160\"}"},
entry1: {"width":7.415865421295166,"height":13.333333333333332,"svg":{},"propsStr":"{\"viewBox\":\"0 0 384.6153846153846 184.6153846153846\"}"},

yikes, thanks for catching this @h-sug1no!

I'll take a look this week. I'm not familiar with the measure text cache but can investigate it.

How did you notice this bug, by the way?

Have you also been running the command line tests in parallel? We should get more of us testing the parallel visual diffs, and then promote it to be the default!

How did you notice this bug, by the way?

Ah...though my puppeteer visual-regression-test branch is still on going, but it can generate svg(and png (generated by UA from svg data)) files. I found this problem when I was testing my branch.

you can test my branch as follows:

clone and checkout: https://github.com/h-sug1no/vexflow/tree/vrtest-puppeteer-backend-1

git checkout vrtest-puppeteer-backend-1
npm install
npm audit fix
npm install --save-dev puppeteer
npx grunt && npm run reference
VF_GENERATE_OPTIONS="--backend=pptr" npm run generate:current
VF_GENERATE_OPTIONS="--backend=pptr --parallel=1" npm run generate:reference
npm run diff:reference

then you can get the following errors:

# In the case of macOS 11.6, results may be different on the other platforms...
You have        7 fail(s):
pptr-Bend_-_Double_Bends_-_SVG_%2B_Petaluma.svg 1.37663
pptr-Vibrato_-_Vibrato_with_Bend_-_SVG_%2B_Bravura.svg 0.339537
pptr-Bend_-_Double_Bends_-_SVG_%2B_Bravura.svg 0.230638
pptr-Bend_-_Double_Bends_With_Release_-_SVG_%2B_Bravura.svg 0.221603
pptr-Bend_-_Double_Bends_With_Release_-_SVG_%2B_Gonville.svg 0.221384
pptr-Bend_-_Double_Bends_With_Release_-_SVG_%2B_Petaluma.svg 0.218442
pptr-TabTie_-_Pulloffs_-_SVG_%2B_Petaluma.svg 0.0415295

I have no time to fix my branch until the end of next month....

But if you need to fix, Please feel free to fork my branch.

I investigated a bit. It might be because some of the test cases call ctx.scale(1.5, 1.5).

Currently, when we scale a SVG context by 1.5X, we modify the viewBox attribute. I think the reasoning is that it should operate similarly to CanvasContext. If the SVG is scaled up, its containing box will be the same size, but we "zoom into" the image.

A different approach would change the SVG element's transform: transform="scale(1.5 1.5)"

But if we wanted to preserve the cropping behavior of the CanvasContext, we would need to wrap our SVG element in a parent div tag with overflow:hidden.

Here's an example where you can play around with the SVG's viewBox, transform, and parent DIV:

https://jsfiddle.net/om72h39k/

As you modify the viewBox or scale transform, you'll see that the reported width of the ✕ symbol changes slightly. I think it's just regular floating point error.

SVGRect {x: 11, y: 5, width: 7.622656345367432, height: 11.5}
SVGRect {x: 11, y: 5, width: 7.625, height: 11.5}
SVGRect {x: 11, y: 5, width: 7.622109413146973, height: 11.5}
SVGRect {x: 11, y: 5, width: 7.622265815734863, height: 11.5}

See the test cases in: tests/flow.html?module=Bend

Even when I round the widths to 1 decimal place, I get discrepancies like this:

OLD
{"text":"Monstrous","width":62.3,"height":13.3,"svg":{},"family":"Arial, sans-serif","size":"10pt","style":"normal","weight":"normal"}

NEW
{"text":"Monstrous","width":62.2,"height":13.3,"svg":{},"family":"Arial, sans-serif","size":"10pt","style":"normal","weight":"normal"}

This is because some test cases set the scale to 1.5x, and then others set the scale to 1.0x. When we ask the browser to measure the text, the floating point error shows up.

This means that if we change the ordering of test cases, or if we run tests in parallel, the exact widths won't match up.

What should we do?

1. Nothing. The floating point error is small and doesn't noticeably affect visual output.
2. Round widths to 1 decimal point. This fixes most, but not all of the discrepancies.
3. Round widths to nearest integer. Maybe this will introduce small errors with layout? Or maybe it's fine?
4. Drop the text measuring cache? I think we had a good reason to implement it, as @tommadams made some performance improvements several months ago.

Can you verify that the failing test cases are when:

• two test cases use the same text (e.g., "Monstrous" in bend_tests.ts)
• the first test case sets scale(x1, y1)
• the second / failing test sets scale(x2, y2)
• x1 != x2 and/or y1 != y2

Sorry I didn't see this issue earlier, I've not had much time for personal projects recently.

I added the glyph measurement cache because it speeded up the unit tests by about 20%. Without the cache, every single call to measureText on a CanvasContext will cause a document reflow.

I'm all for deleting the cache if its buggy. I actually have a branch that I'm working on that factors texture measurement out into a separate class. That way, we can use a hidden (or offscreen) canvas to measure text even when the actual rendering is done using SVG.

I can send a PR deleting the cache later in the week.

I don't think we need to delete the cache just yet. The measurement numbers are basically the same. :-) It only affects the visual regression tests if we run them out of order (or in parallel).

On a given score in the real world, no one will notice if something is off by 0.1 pixels. Plus, this only happens if someone has two scores at different context scales rendering the same text.

I like your idea of using a offscreen canvas to measure text. That way, the helper canvas can always be the same scale, even if we scale the output canvas or SVG.

Just a quick idea: Can't we save the pixel value for 1.0 scale or something and restore it later? Whenever you have an out of order testing bug, usually you can either reset the values to a default before the test or store and restore values that tests modify (like we do with context). This may not work or be helpful, but I thought I'd mention it.