New issue
Advanced search Search tips

Issue 678167 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 709001



Sign in to add a comment

SVG: getScreenCTM() ancestor element transforms not applied correctly

Reported by paul.leb...@gmail.com, Jan 4 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce the problem:
1. Visit https://jsfiddle.net/r8887x05/1/ or try the code below:

<div id="mydiv" style="transform: translate(1px,0) scale(2)">
  <svg>
    <circle id="mycircle" cx="50" cy="50" r="10"/>
  </svg>
</div>

var div = document.getElementById('mydiv');
var circle = document.getElementById('mycircle');
console.log("before=", circle.getScreenCTM());
div.setAttribute("style", "transform: translate3D(1px,0,0) scale(2)");
console.log("after=", circle.getScreenCTM());

What is the expected behavior?
This code outputs:

before= SVGMatrix { a: 1, b: 0, c: 0, d: 1, e: -335.5, f: -69 }
after= SVGMatrix { a: 1, b: 0, c: 0, d: 1, e: -335.5, f:-69 }

It appears that the "scale(2)" portion of the transform has affected the 'e' and 'f' fields of the matrix, but not the 'a' and 'd' fields.  I would have expected a result more like:

before= SVGMatrix { a: 2, b: 0, c: 0, d: 2, e: -335.5, f: -69 }
after= SVGMatrix { a: 2, b: 0, c: 0, d: 2, e: -335.5, f:-69 }

What went wrong?
Incorrect matrix.

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 55.0.2883.87  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 24.0 r0

http://stackoverflow.com/questions/41452295/zooming-an-image-results-in-issues-with-pagex-pagey-to-svg-conversions
 

Comment 1 by f...@opera.com, Jan 4 2017

Labels: -OS-Windows
Status: Available (was: Unconfirmed)
There's a FIXME in SVGSVGElement::localCoordinateSpaceTransform:

// FIXME: This doesn't work correctly with CSS transforms.

which I think answers the "why". What's happening right now is that we're computing the location of the upper-left corner of the CSS layout box of the <svg> and then adding (prepending) that to the transform, "missing" the scale. Hopefully it's as easy as replacing localToAbsolute with localToAbsoluteTransform. (Probably flattening any 3d-components, at least at first, because there's some impedance mismatch here until we can return DOMMatrix from getScreenCTM.)

I'm including the spec link here for easy reference:
https://svgwg.org/svg2-draft/types.html#__svg__SVGGraphicsElement__getScreenCTM
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0286404a0fdb943d9e69ea7af0ffc85e0435f87

commit d0286404a0fdb943d9e69ea7af0ffc85e0435f87
Author: fs <fs@opera.com>
Date: Mon Feb 20 20:01:26 2017

Rewrite svg/zoom/page/zoom-get-screen-ctm.html to use testharness

In preparation for some future tweaks to this test.

BUG= 678167 

Review-Url: https://codereview.chromium.org/2708493003
Cr-Commit-Position: refs/heads/master@{#451657}

[delete] https://crrev.com/60698ac0ece866407aafdab0a71d18068ff111bc/third_party/WebKit/LayoutTests/svg/zoom/page/zoom-get-screen-ctm-expected.txt
[modify] https://crrev.com/d0286404a0fdb943d9e69ea7af0ffc85e0435f87/third_party/WebKit/LayoutTests/svg/zoom/page/zoom-get-screen-ctm.html

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ad2a9bf7205ec77843cf27a7964fda92df386b2

commit 2ad2a9bf7205ec77843cf27a7964fda92df386b2
Author: fs <fs@opera.com>
Date: Wed Feb 22 08:50:27 2017

Compute a more correct "screen scope" transform for SVGSVGElement

For getScreenCTM, only the position (translation) of the outermost svg
element was computed - any additional transform data was dropped.

Use LayoutObject::localToAbsoluteTransform to compute the full transform
rather than just the position of the layout box. Since using this method
works for any (attached) element, implement getScreenCTM without using
computeCTM, and get rid of the ScreenScope variant of the latter. This
also allows us to simplify SVGSVGElement::localCoordinateSpaceTransform
a bit, and drop the CTMScope argument from the
localCoordinateSpaceTransform declaration(s).

It's not clear from [1] how elements which are not in the rendering tree
(i.e has 'display: none' or similar) should be handled. With this
implementation we will return an identity matrix in those cases, which
doesn't seem unreasonable. (The option would be to return 'null', which
is how elements not in the document should be treated, but we don't have
that semantic implemented yet.)

[1] https://svgwg.org/svg2-draft/types.html#__svg__SVGGraphicsElement__getScreenCTM

BUG= 678167 

Review-Url: https://codereview.chromium.org/2711503002
Cr-Commit-Position: refs/heads/master@{#451938}

[add] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/LayoutTests/svg/dom/getScreenCTM-ancestor-transform.html
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/LayoutTests/svg/zoom/page/zoom-get-screen-ctm.html
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGElement.h
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGGraphicsElement.cpp
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGGraphicsElement.h
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGPatternElement.cpp
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGPatternElement.h
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGSVGElement.cpp
[modify] https://crrev.com/2ad2a9bf7205ec77843cf27a7964fda92df386b2/third_party/WebKit/Source/core/svg/SVGSVGElement.h

Comment 4 Deleted

Should we raise a spec issue to get a clarification on the display:none question Fredrik?

Comment 7 by creis@chromium.org, Feb 28 2017

Blockedon: 697111
Note that the test from r451938 is failing with --site-per-process (i.e., when using out-of-process iframes for cross-site URLs).  Please see  issue 697111 .
Labels: PaintTeamTriaged-20170104 BugSource-User

Comment 9 by f...@opera.com, May 3 2017

Blockedon: -697111

Comment 10 by f...@opera.com, May 3 2017

Blockedon: 709001
Project Member

Comment 11 by sheriffbot@chromium.org, May 3 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 12 by f...@opera.com, May 3 2018

Status: Fixed (was: Untriaged)
I'm not sure it's worthwhile to keep this around while waiting for WG resolution and DOMMatrix to be usable here (i.e issue 709001.) I even STR that we have another bug for the display:none behavior. Shout if you disagree Paul.
If the origin issue has been resolved (your patch has been merged right?). Then I am good.


Sign in to add a comment