Bug 194903 – Transform-origin on SVG breaks when zoom in or out

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Kristina Kovach]

In the following codepen example there is an inline SVG spinner animated with CSS. The spinner has two animated parts: <g> and <circle>. Both parts have `transform-origin: center` and `transform-box: fill-box` set. When zooming page in or out these parts are no longer rotated at their center point despite transform origin property.

[Simon Fraser (smfr)]

By "zoom" you mean Command-+ zooming?

[Kristina Kovach]

Yes

[Simon Fraser (smfr)]

In SVGGraphicsElement::animatedLocalTransform(), objectBoundingBox() is being affected by effectiveZoom() but transformOriginX() is not, so they are mismatched.

https://trac.webkit.org/r166967 tried to fix this but I'm not sure it's correct.

[Simon Fraser (smfr)]

Created attachment 363945 [details]
Testcase

[Simon Fraser (smfr)]

Transforms and zoom are just broken in SVG in general. You don't need a rotate or transform-origin to have them go wrong.

[Simon Fraser (smfr)]

Created attachment 363949 [details]
Test with translate

[Simon Fraser (smfr)]

Reverting r166967 fixes this for me.

[Simon Fraser (smfr)]

Created attachment 363972 [details]
Patch

[EWS Watchlist]

Comment on attachment 363972 [details]
Patch

Attachment 363972 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11422213

New failing tests:
svg/zoom/page/transform-origin-and-zoom.html

[EWS Watchlist]

Created attachment 363975 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363972 [details]
Patch

Attachment 363972 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11422223

New failing tests:
svg/zoom/page/transform-origin-and-zoom.html

[EWS Watchlist]

Created attachment 363977 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363972 [details]
Patch

Attachment 363972 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11422355

New failing tests:
svg/zoom/page/transform-origin-and-zoom.html

[EWS Watchlist]

Created attachment 363980 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363972 [details]
Patch

Attachment 363972 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11422382

New failing tests:
svg/zoom/page/transform-origin-and-zoom.html

[EWS Watchlist]

Created attachment 363982 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Said Abou-Hallawa]

Comment on attachment 363972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363972&action=review

> Source/WebCore/svg/SVGGraphicsElement.cpp:-107
> -        // CSS bakes the zoom factor into lengths, including translation components.
> -        // In order to align CSS & SVG transforms, we need to invert this operation.
> -        float zoom = style->effectiveZoom();
> -        if (zoom != 1) {
> -            matrix.setE(matrix.e() / zoom);
> -            matrix.setF(matrix.f() / zoom);
> -        }
> -

I do.not think this change is correct. With this change if you open the file  LayoutTests/svg/zoom/page/zoom-css-transforms.svg and you manually Command-+, the display will be wrong. I think the above if-stament should only be executed if (style->transformBox() != TransformBox::FillBox). In case of TransformBox::FillBox, boundingBox is the logical bounds of the renderer. Otherwise, boundingBox Is multiplied by the zooming factor. So we need to remove this scaling somehow from the matrix.

[EWS Watchlist]

Comment on attachment 363972 [details]
Patch

Attachment 363972 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11463175

New failing tests:
svg/zoom/page/transform-origin-and-zoom.html

[EWS Watchlist]

Created attachment 364322 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Joseph Pecoraro]

*** Bug 195434 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Someone else should be free to take this.

[Joseph Pecoraro]

Comment on attachment 363972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363972&action=review

>> Source/WebCore/svg/SVGGraphicsElement.cpp:-107
>> -
> 
> I do.not think this change is correct. With this change if you open the file  LayoutTests/svg/zoom/page/zoom-css-transforms.svg and you manually Command-+, the display will be wrong. I think the above if-stament should only be executed if (style->transformBox() != TransformBox::FillBox). In case of TransformBox::FillBox, boundingBox is the logical bounds of the renderer. Otherwise, boundingBox Is multiplied by the zooming factor. So we need to remove this scaling somehow from the matrix.

If I put this inside of an `if (style->transformBox() != TransformBox::FillBox)` block then this no longer solves duplicate 195434.

[Joseph Pecoraro]

Comment on attachment 363972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363972&action=review

> LayoutTests/ChangeLog:8
> +        Test still needs some work to avoid antialiasing diffs.

To avoid antialiasing diffs you could use SVG transforms:

    <!DOCTYPE html>
    <html>
    <head>
        <style>
            body {
                margin: 0;
            }
            rect {
                transform-box: view-box;
            }
        </style>
        <script>
            function doTest()
            {
                const zoomCount = 2;
                for (let i = 0; i < zoomCount; ++i)
                    eventSender.zoomPageIn();
            }
            window.addEventListener('load', doTest, false);
        </script>
    </head>
    <body>
        <svg width="500" height="400" viewBox="0 0 500 400" xmlns="http://www.w3.org/2000/svg">
            <rect x="50" y="50" width="80" height="80" fill="gray"/>
            <rect x="50" y="50" width="80" height="80" fill="green" transform="rotate(90 130 130)"/>

            <rect x="250" y="100" width="80" height="80" fill="gray"/>
            <rect x="210" y="60" width="120" height="120" fill="green" transform="rotate(90 330 180)"/>

            <rect x="50" y="250" width="80" height="80" fill="gray"/>
            <rect x="50" y="250" width="80" height="80" fill="green" transform="translate(80 0)"/>
        </svg>
    </body>
    </html>

But I'm not sure if this will be a great test then, because we'd like to compare to CSS transforms.

[Radar WebKit Bug Importer]

<rdar://problem/96318505>

[Simon Fraser (smfr)]

*** Bug 258615 has been marked as a duplicate of this bug. ***

[Karl Dubost]

*** Bug 263219 has been marked as a duplicate of this bug. ***

[Fujii Hironori]

*** Bug 273307 has been marked as a duplicate of this bug. ***

[Fujii Hironori]

Created attachment 471211 [details]
test case 2

[Fujii Hironori]

Created attachment 471940 [details]
WIP patch

[Fujii Hironori]

Pull request: https://github.com/WebKit/WebKit/pull/31095

[EWS]

Committed 281265@main (c78d507eeee2): <https://commits.webkit.org/281265@main>

Reviewed commits have been landed. Closing PR #31095 and removing active labels.

[Ahmad Saleem]

*** Bug 218300 has been marked as a duplicate of this bug. ***

[Fujii Hironori]

*** Bug 133150 has been marked as a duplicate of this bug. ***