Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVGContext scaling issue in 4.2.2 #1587

Open
brunostuani opened this issue Jul 21, 2023 · 6 comments
Open

SVGContext scaling issue in 4.2.2 #1587

brunostuani opened this issue Jul 21, 2023 · 6 comments

Comments

@brunostuani
Copy link

brunostuani commented Jul 21, 2023

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
Copy link
Author

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}`);

@AaronDavidNewman
Copy link
Collaborator

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.

@rvilarl
Copy link
Collaborator

rvilarl commented Jul 23, 2023

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

@brunostuani
Copy link
Author

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

@AaronDavidNewman
Copy link
Collaborator

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

@newlandsvalley
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants