vexflow icon indicating copy to clipboard operation
vexflow copied to clipboard

SVGContext scaling issue in 4.2.2

Open brunostuani opened this issue 2 years ago • 6 comments

Hi,

In older versions (4.1) this used to work, but I realized that the behavior of .scale() has changed in the latest 4.2 version. The behavior switched from scaling from an absolute number to a relative number. Which could be fine, but it broke other places, for example, with .clear() and .resize().

The scale() code will now apply the new value relative to the last scale used (code):

    this.state.scaleX = this.state.scaleX ? this.state.scaleX * x : x;
    this.state.scaleY = this.state.scaleY ? this.state.scaleY * y : y;

Meaning that if I set .scale() to 0.8, the first time the function runs, scale will be set to 0.8. All good. If later I call .clear(), it will internally reset the scale to the latest value 0.8 from the state, and the new scale will be set to 0.64, which is of course not desired.

The same thing will happen to .resize(), where it calls .scale() internally, resulting in the exponential scaling bug to happen.

This is not letting me scale to anything other than 1

brunostuani avatar Jul 21 '23 23:07 brunostuani

For now, I'm bypassing this limitation by not setting a new scale different than 1, and changing the viewBox manually

    // this.renderer.getContext().scale(scale, scale);
    this.musicDiv.nativeElement.getElementsByTagName('svg')
      .item(0)?.setAttribute('viewBox', `0 0 ${width / scale} ${height / scale}`);

brunostuani avatar Jul 21 '23 23:07 brunostuani

I think the original definition of 'scale' in the rendering context was ambiguous about what happens when you run it more than once. A lot of this is because the RC is supposed to be agnostic SVG vs. canvas, and the viewBox is very much an SVG thing.

You are on the right track about setting the scale in the viewBox directly. Then you always know what you're getting. This is what I do for Smoosic.

AaronDavidNewman avatar Jul 22 '23 22:07 AaronDavidNewman

The scale was changed to match the canvas approach. Probably the functions you mentioned have to be adjusted as well.

rvilarl avatar Jul 23 '23 11:07 rvilarl

You are on the right track about setting the scale in the viewBox directly. Then you always know what you're getting. This is what I do for Smoosic.

The SVG is created by the renderer, meaning I have to query for it after the renderer creates it. The creation is an implementation detail of the SVG backend, which means that changing attributes of their created objects (that are not even exposed by the Renderer), would be at least not expected, thus not being on the right track :) nothing guarantees that the renderer backend would not rewrite the viewport attribute (in fact they do, but earlier than us).

Although I understand why this led us to hack the created SVG manually. Hopefully the interfaces are improved in V5. One idea would be to let us give which SVG element we want the renderer to use, then yes, it would be expected that we control the viewport

brunostuani avatar Jul 25 '23 22:07 brunostuani

That's a great idea, please submit a PR if you can think of a good interface for this.

AaronDavidNewman avatar Jul 26 '23 00:07 AaronDavidNewman

I've been hit by this too. Surely it's not intended that, after you set the initial scale, any subsequent call to clear resets the scale?

newlandsvalley avatar Oct 17 '23 09:10 newlandsvalley