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

Wrong transform is added for SVG with viewbox origin != (0,0) #236

Open
robinwersich opened this issue May 15, 2023 · 2 comments
Open

Wrong transform is added for SVG with viewbox origin != (0,0) #236

robinwersich opened this issue May 15, 2023 · 2 comments

Comments

@robinwersich
Copy link

When reading and writing an SVG with viewbox not starting at (0,0), a wrong transform is added:

Input:

<svg height="20" version="1.1" width="20" viewBox="10 10 20 20" xmlns="http://www.w3.org/2000/svg" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xlink="http://www.w3.org/1999/xlink">
  <rect x="10" y="10" width="20" height="20"/>
</svg>

Output:

<svg height="20.0" version="1.1" width="20.0" viewBox="10 10 20 20" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:ev="http://www.w3.org/2001/xml-events">
  <rect x="10" y="10" width="20.0" height="20.0" transform="matrix(1.000000, 0.000000, 0.000000, 1.000000, 10.000000, 10.000000)" stroke="none" stroke-width="1.0" fill="#000000" />
</svg>

Expected:

<svg height="20.0" version="1.1" width="20.0" viewBox="10 10 20 20" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:ev="http://www.w3.org/2001/xml-events">
  <rect x="10.0" y="10.0" width="20.0" height="20.0" stroke="none" stroke-width="1.0" fill="#000000" />
</svg>

I suppose this is because the original x and y attribute is just written-as is, because the decimal point is missing and querying the x position of the parsed Rect, it correctly returns 0.0. The transform is then probably added to move the rect back to (10, 10), ignoring the fact that the Rect is already placed at (10, 10) with x and y.
This issue may also apply to other shapes.

@tatarize
Copy link
Member

Tested. This is accurate. The problem isn't that the transform is added, the transform is technically valid, it's that if we're writing a viewbox we actually generate a counter-transform by doing that even if the viewbox is technically identical to the one we started with.

@tatarize
Copy link
Member

This is actually because the export write done in #228 doesn't account for the fact that the viewbox will technically cause a transform and that transform should actually be inverted and applied to the shapes via the viewbox-to-transform algorithm.

Basically what happened is that the when we parsed we got rid of the viewbox by turning it into a transform. We applied that to all of the geometric shapes. Then we just sort of write the viewbox back to the design without taking into account that any viewbox would technically actually cause our shapes to invert.

We either need to counter-transform all the shapes or we need to modify the viewbox between loading and saving.

When you have a viewbox, the parsing actually scales our geometric objects up to make that viewbox equal to effectively an identity matrix.

On the write, we apply the viewbox again, but we didn't reverse the effects on all the geometric objects when we did that.


Also, we may actually want to change our viewbox and we should probably account for that. I'm unsure whether this might better fall into the svgio to-do list. Technically we could set the viewbox to always start at 0,0 and technically that would be accurate.

The viewbox is just where you're viewing the objects. When loaded the viewbox was applied to the geometric data since this is sort of expected. But, technically the viewbox doesn't scale up the geometry it just makes it look bigger because your view is smaller.

So if our viewbox is really 1mm x 1mm and we're using it to view a circle located at 15mm, 15mm with a 1mm diameter giving us a viewbox of 15mm 15mm 16mm 16mm the program will go ahead and apply the transform as a matrix so that it's identical to viewing it at 0 0 1mm 1mm . That's fine and sort of what we expect, however if then save with the original viewport of 15mm 15mm 16mm 16mm we're actually looking at a region of space containing no geometric objects at all.

In a very real sense we deleted the viewbox and should have no viewbox or we should make a viewbox that is always located at (0,0) and is the size of our design (so it produces an identity matrix).

tatarize added a commit that referenced this issue Aug 17, 2023
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

2 participants