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

SVG view box should not be used for its aspect ratio #731

Closed
zhanghai opened this issue Apr 20, 2021 · 1 comment
Closed

SVG view box should not be used for its aspect ratio #731

zhanghai opened this issue Apr 20, 2021 · 1 comment

Comments

@zhanghai
Copy link
Contributor

Describe the bug

(Copying discussion from #688 becuase it's missing follow-up probably because it's a PR instead of an issue.)

I see, thanks for the clarification about using view box for fitting the target size. However, it still seems to me the original behavior is correct.

For example, if we have an SVG that is 100x200, but the view box is only 100x100, the image actually only has a 1x1 aspect ratio (everything outside of the view box isn't rendered).

This doesn't sound correct according to my understanding of SVG.

  1. Although the view box may be smaller, it's still perfectly valid for content to be drawn outside the view box and be shown in the final SVG, because:

    1. the viewBox attribute is essentially just a shorthand for a transformation matrix:

      The effect of the ‘viewBox’ attribute is that the user agent automatically supplies the appropriate transformation matrix to map the specified rectangle in user space to the bounds of a designated region (often, the viewport).

      https://www.w3.org/TR/SVG11/coords.html#ViewBoxAttribute

    2. the default value for preserveAspectRatio (when viewBox is present) is xMidYMid instead of none, so that uniform scaling is performed and the view box is centered, leaving additional space for drawing.

      • xMidYMid (the default) - Force uniform scaling.
        Align the midpoint X value of the element's ‘viewBox’ with the midpoint X value of the viewport.
        Align the midpoint Y value of the element's ‘viewBox’ with the midpoint Y value of the viewport.

      https://www.w3.org/TR/SVG11/coords.html#PreserveAspectRatioAttribute

      (One potential source of confusion is that for Android VectorDrawable XML files, there is no preserveAspectRatio equivalent and android:viewportWidth & android:viewportHeight behaves as if preserveAspectRatio is none. But Android VectorDrawable XML isn't SVG.)

    3. The SVG file in my previous comment is an example of this (It's the Material Design icon for "image", and obviously draws outside its view box), and I've confirmed in EOG and Inkscape that they treat the SVG file as if the view box is just a transformation matrix, and nothing else (size, aspect ratio, zoom) is affected by the view box.

      <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 6 24" version="1.1">
          <path fill="#000000" d="M 10,5 V 19 H -4 V 5 H 10 M 10,3 H -4 c -1.1,0 -2,0.9 -2,2 v 14 c 0,1.1 0.9,2 2,2 h 14 c 1.1,0 2,-0.9 2,-2 V 5 C 12,3.9 11.1,3 10,3 Z m -4.86,8.86 -3,3.87 L 0,13.14 -3,17 H 9 Z" />
      </svg>
  2. Even if no content is drawn outside of the view box, it is still perfectly valid for an SVG file to utilize the view box for transformation, and it should be handled as if the transformation were applied at a root group instead of as the view box.

    (i.e. I didn't find anything in the specification that says the viewBox attribute should be the viewport of the user agent, and on the contrary, the specification seems to suggest the viewBox attribute is just another way to specify a transformation. The value for viewBox are the coordinates pre-tranform, but it's the post-tranform coordinates that should be rendered and respected.)

    Take this SVG file for an ellipse as an example:

        <svg xmlns="http://www.w3.org/2000/svg" width="48" height="24" viewBox="0 0 24 24" preserveAspectRatio="none" version="1.1">
            <circle cx="12" cy="12" fill="#000000" r="12" />
        </svg>

    It should be handled exactly like the following equivalent SVG file:

        <svg xmlns="http://www.w3.org/2000/svg" width="48" height="24" version="1.1">
            <g transform="scale(2, 1)">
                <circle cx="12" cy="12" fill="#000000" r="12" />
            </g>
        </svg>

    I've confirmed that both EOG and Inkscape treats the two SVG files in exactly the same way.

    This is actually also covered similarly in the specification's discussion of how the user agent may implement viewBox with transform:

    ... The effect is equivalent to having a viewport of size 300px by 200px and the following supplemental transformation in the document, as follows:

    <?xml version="1.0" standalone="no"?>
    <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" 
      "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
    <svg width="300px" height="200px" version="1.1"
         xmlns="http://www.w3.org/2000/svg">
      <g transform="scale(0.2)">
        <!-- Rest of document goes here -->
      </g>
    </svg>
    

    https://www.w3.org/TR/SVG11/coords.html#ViewBoxAttribute

  3. Even if some non-standard handling of viewBox for fitting the target size is desired for some reason, it should actually respect the value of preserveAspectRatio to be a correct implementation.

    The current implementation would return an incorrect result if preserveAspectRatio is set to none, in which case the aspect ratio from the width and height attributes should definitely be used instead of the one from viewBox.

    I still don't see why we need non-standard handling of viewBox for fitting the target size though.

Expected behavior

SVG view box should not be used for its aspect ratio.

To Reproduce

Read changelog or #688.

Logs/Screenshots

N/A

Version

1.2.0

@zhanghai zhanghai added the bug Something isn't working label Apr 20, 2021
@colinrtwhite colinrtwhite removed the bug Something isn't working label Dec 12, 2021
@colinrtwhite
Copy link
Member

Sorry for the long delay on this! I've avoided this issue for a while since I don't think I have enough understanding of the SVG spec to say what the correct behaviour should be here. It's possible Coil is implementing logic out of line with the spec, however overall I lean towards keeping the behaviour as is since there haven't been any follow up issue reports related to SVG rendering and this behaviour.

Additionally, I think changing the default behaviour at this point would be more detrimental to end users even if the behaviour is incorrect. Also it's possible to avoid the default behaviour by setting useViewBoundsAsIntrinsicSize = false.

@colinrtwhite colinrtwhite closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 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