Bug 44083 – SVGLocatable.getScreenCTM ignores scrolling

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

[Mike Bostock]

Created attachment 64541 [details]
Test case.

Per SVG Tiny 1.2, Section A.8.11 SVGLocatable:

"""
getScreenCTM

Returns the transformation matrix from current user units to the initial viewport coordinate system. The clientX and clientY coordinates of a MouseEvent are in the initial viewport coordinate system. Note that null is returned if this element is not hooked into the document tree. This method would have been more aptly named as getClientCTM, but the name getScreenCTM is kept for historical reasons.
"""

In WebKit (and Safari, Chrome), the matrix returned by getScreenCTM considers the scrollTop and scrollLeft. Therefore, it is as if pageX and pageY were used instead of clientX and clientY, as described by the spec. Firefox and Opera exhibit the correct behavior, consistent with clientX and clientY.

[Mike Bostock]

Created attachment 64542 [details]
Expected output of test case.

[Mike Bostock]

Created attachment 64543 [details]
Actual output of test case.

[Mike Bostock]

Oops, clarification to my initial description: getScreenCTM ignores scrollTop and scrollLeft in WebKit, which is why it is inconsistent with clientX and clientY.

For example, in Firefox and Opera, if you scroll down by 200 pixels, svg.getScreenCTM().f will go down by 200 pixels (e.g., 92 -> -108). On the other hand, in WebKit, Safari and Chrome the value of svg.getScreenCTM().f will stay constant independent of scrolling.

[Eric Seidel (no email)]

http://trac.webkit.org/browser/trunk/WebCore/svg/SVGLocatable.cpp#L87

is the code.  Notice how it stops at the first non-SVG element.

I suspect that scrolling may be expressed in tx/ty anyway, I'm not sure.

[Mike Bostock]

(In reply to comment #4)
> Notice how it stops at the first non-SVG element.

That's true for mode == NearestViewportScope (getCTM), but not mode == ScreenScope (getScreenCTM), where stopAtElement is 0 and it goes all the way to the root element.

I think what's needed is something like this at the end of computeCTM:

    if (mode == ScreenScope) {
        DOMWindow* window = element->ownerDocument()->defaultView();
        ctm = ctm.translate(window->scrollX(), window->scrollY());
    }

    return ctm;

Although I haven't thought through whether it should be translate or translateRight, yet. Or maybe -scrollX and -scrollY.

[Mike Bostock]

(In reply to comment #4)
> Notice how it stops at the first non-SVG element.

Oops, right. I see the current->isSVGElement() now. Either way, that part looks fine. It's just not taking into account scrolling.

[Eric Seidel (no email)]

Maybe the solution is as simple as asking frame()->view() for the scrollOffset().

[Nikolas Zimmermann]

(In reply to comment #0)
> Created an attachment (id=64541) [details]
> Test case.
> 
> Per SVG Tiny 1.2, Section A.8.11 SVGLocatable:
> 
> """
> getScreenCTM
> 
> Returns the transformation matrix from current user units to the initial viewport coordinate system. The clientX and clientY coordinates of a MouseEvent are in the initial viewport coordinate system. Note that null is returned if this element is not hooked into the document tree. This method would have been more aptly named as getClientCTM, but the name getScreenCTM is kept for historical reasons.
> """

First of all, we focus on SVG 1.1 2nd Edition, not SVG Tiny 1.2.
I explicitely ignored the scroll offset when implementing these functions, because what SVG 1.1 says, is:
http://www.w3.org/TR/SVG/types.html#InterfaceSVGLocatable


Returns the transformation matrix from current user units (i.e., after application of the ‘transform’ attribute, if any) to the parent user agent's notice of a "pixel". For display devices, ideally this represents a physical screen pixel. For other devices or environments where physical pixel sizes are not known, then an algorithm similar to the CSS2 definition of a "pixel" can be used instead. Note that null is returned if this element is not hooked into the document tree. This method would have been more aptly named as getClientCTM, but the name getScreenCTM is kept for historical reasons.

I was not sure about scrolling, but it seems to me that the SVG Tiny 1.2 behaviour is more clear.
So, I'll have a look to fix it.

[Simon Fraser (smfr)]

(In reply to comment #7)
> Maybe the solution is as simple as asking frame()->view() for the scrollOffset().

What about SVG nested inside multiple overflow:scroll divs?

[Mike Bostock]

(In reply to comment #9)
> What about SVG nested inside multiple overflow:scroll divs?

That part's fine, as you can see from this test case:

http://graphics.stanford.edu/~mbostock/getScreenCTM-nested.html

The goal is for getScreenCTM to be consistent with the mouse event's clientX and clientY. The only discrepancy with the current implementation is scrolling on the parent frame or window; scrolled container elements appear to be handled correctly (and thus are consistent with pageX and pageY).

[Nikolas Zimmermann]

Created attachment 65686 [details]
Patch

Indeed, all we need to do is respect the FrameViews scrollOffset.
Nested divs with overflow="scroll" already work properly, as the localToAbsolute() call already takes care of handling those translations.

Added three new tests covering scrollable SVG areas, and a large SVG area contained in (nested) divs with overflow="scroll", to rule out Simons concerncs.

[Dirk Schulze]

Comment on attachment 65686 [details]
Patch

Great examples! r=me

[Nikolas Zimmermann]

Landed in r66187. Mike, please retry, with a nightly newer or equa to r66187 :-)

[Mike Bostock]

(In reply to comment #13)
> Landed in r66187. Mike, please retry, with a nightly newer or equa to r66187 :-)

Works great! Thanks for fixing this.

[Jason Davies]

Created attachment 233985 [details]
Test case for getScreenCTM and scrolling on iOS.

[Jason Davies]

Unfortunately this issue seems to be unresolved on Safari in iOS 7.1.1.  I’ve attached a modified version Mike’s test case that uses touchmove instead of mousemove.