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

PNG dimension and resolution issues #75

Open
louh opened this issue Nov 8, 2019 · 5 comments
Open

PNG dimension and resolution issues #75

louh opened this issue Nov 8, 2019 · 5 comments

Comments

@louh
Copy link
Contributor

louh commented Nov 8, 2019

Hi! Thanks for putting this library together. We're using it over at https://github.com/streetmix/vehicle-profile-editor/.

I came across a couple of issues on our end with downloading PNGs and would like to contribute our patch back here, if you're open to it.

  1. If a SVG has an intrinsic dimension (e.g. viewbox), the SVG will be rendered on the canvas with its intrinsic dimensions, not its DOM bounding box (the dimensions obtained with getBoundingClientRect().) This will result in a PNG that is either too large (if the DOM element has been scaled up bigger than the SVG's intrinsic dimensions) or cropped (if the DOM element has been scaled down less than the SVG's intrinsic dimensions). Our solution is to use the SVG's intrinsic width and height values if provided; otherwise, it will continue to fall back to the DOM element dimensions.

  2. The SVG will be pixelated if downloaded on a device with a higher device pixel ratio screen (e.g. Macbook with Retina screens, which has a pixel ratio of 2:1). I think the user's expectation is to download the image at the same resolution that they're seeing on their screen. Our solution is to scale the canvas to match the device pixel ratio when rendering the SVG to a raster image.

I'll be updating my own fork soon with these changes and would be happy to provide a pull request if you're interested. Thanks!

EDIT: Here's our branch - https://github.com/louh/svg-crowbar/tree/louh/png-fixes . I'd love to update tests, but it's currently breaking, as the empty SVG throws an error when you attempt to read the value from baseVal. I'll investigate whether there's a good workaround for this.

@louh
Copy link
Contributor Author

louh commented May 26, 2020

Here's a demonstration of the first case in the CodeSandbox below:

https://codesandbox.io/s/svg-crowbar-test-s05fn?file=/index.html

Let me know if this helps, or if there's anything I can add to clarify the problem.

@louh
Copy link
Contributor Author

louh commented May 26, 2020

Just to describe the issue visually (this is the output of my test case above):

Source SVG should look like this:

png version of source image

If the SVG's parent element is smaller than the SVG's intrinsic dimensions, the saved image is cropped:

case 1

If the SVG's parent element is larger than the SVG's intrinsic dimensions, the saved image has extra empty transparent space (hard to see here because it's just the site's white background:

case 2

I also included a last case that both crops the right side and pads the bottom:

case 3

cy6erskunk added a commit that referenced this issue Dec 25, 2020
cy6erskunk added a commit that referenced this issue Dec 25, 2020
@richardwestenra
Copy link
Contributor

Hey @cy6erskunk I wasn't seeing this issue before (when using v0.6.0), but now as of v0.6.4 I'm seeing cropped PNGs as precisely as described by @louh above.

When exporting PNGs from the manual tests, the versions without a viewBox work fine, whereas versions with a viewBox get cropped:
sample-svg
kedro-pipeline (6).

Compare exports from my app using v0.6.0:
kedro-pipeline (8)
to exporting a PNG using v0.6.4:
kedro-pipeline (7)

Do you know which change is most likely to have caused the issue? I assume it's something related to the DPR value but I can't isolate the exact cause.

@cy6erskunk
Copy link
Owner

I believe an example with viewBox never worked correctly, but currently the latest version is worse. @richardwestenra, could you check how version 0.6.5-0 works for you?

@richardwestenra
Copy link
Contributor

richardwestenra commented Jan 5, 2021

@cy6erskunk 0.6.5-0 works perfectly in my test in my app! 👌 Haven't tried the manual demos but I assume those are fine too.

richardwestenra added a commit to kedro-org/kedro-viz that referenced this issue Jan 7, 2021
v0.6.2 introduced [a call stack size error](cy6erskunk/svg-crowbar#278) when exporting PNGs, which was resolved in v0.6.4.
However, v0.6.4 introduced [an issue with PNG cropping](cy6erskunk/svg-crowbar#75) that has been resolved in 0.6.5.
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

3 participants