New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 747708 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

when animating SVGs with non-scaling-stroke in 3D space, compositing seems to fail

Reported by tombi...@wix.com, Jul 23 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
1. Add an SVG with a <path> element with a stroke and a "vector-effect: non-scaling-stroke" style attribute
2. animate the transform3D style of the <svg> element with Javascript

I've created a demo in Codepen using GSAP for convenience 
https://codepen.io/tombigel/pen/dzbEMJ

What is the expected behavior?
The svg element should animate without glitches, same as it animates without the vector effect

What went wrong?
while animating the stroke does not paint correctly and for complex paths it even hangs the browser tab

Did this work before? N/A 

Chrome version: 59.0.3071.115  Channel: stable
OS Version: OS X 10.12.5
Flash Version: 

With CSS animations the stroke is just not being painted while animating.

In Safari this works fine
 

Comment 1 by woxxom@gmail.com, Jul 23 2017

I see the bug with "non-scaling stroke" checkbox enabled in Windows too.
I had to click the "With 3D (Bug is Here!)" button a couple of times, though.

Clip301-fs8.png
15.8 KB View Download

Comment 2 by kojii@chromium.org, Jul 24 2017

Components: -Blink Blink>SVG
Labels: OS-Windows
Status: Untriaged (was: Unconfirmed)
Cc: pdr@chromium.org brajkumar@chromium.org
Labels: -Type-Bug -Pri-2 hasbisect-per-revision M-62 OS-Linux Pri-1 Type-Bug-Regression
Owner: f...@opera.com
Status: Assigned (was: Untriaged)
Able to reproduce on Windows-10, Ubuntu 14.04 and Mac OS 10.11.5 using chrome latest stable M59-59.0.3071.115. 

Bisect Information:
--------------------
Good build: 58.0.3020.0
Bad Build : 59.0.3022.0

You are probably looking for a change made after 451937 (known good), but no later than 451938 (first known bad).

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/c5b606c3188dac60dd3f8e29bafbb90d95089c3e..2ad2a9bf7205ec77843cf27a7964fda92df386b2

From the above change log suspecting below change
Review URL: https://codereview.chromium.org/2711503002

fs@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Adding RB-stable for M-60 please feel free to edit if this is not the case.

Thanks!

Correction on build numbers:
-------------------------------
Good build: 58.0.3020.0
Bad Build : 58.0.3022.0

Comment 5 by f...@opera.com, Jul 24 2017

That CL made us actually consider the scale imposed by the transform on the <svg>, so it quite likely that it exposed something (possibly fixed the "JS animation / no 3D" case too, I didn't bother getting an appropriate build to verify that.)
What I think is happen (in the "JS anim / 3D" case) is that we paint when the scale is a small value > 0, and then raster that at various scales while keeping the "old" host-CTM, or rather its inverse, and thus ending up scaling by a large factor. This can be observed in the "CSS anim / *" cases as well by nudging the starting scale factor from 0 to something slightly above 0 (say 0.0001.) So what "prevents" the effect during the (original) CSS animation is that painting happens at scale=0, which yields a degenerate host-CTM, causing us to skip the stroke entirely (easily seen in the testcase.)
So the bug here is either that we don't repaint "when we should", or that we should use a different definition of "host coordinate space" [1]. There are downsides to both of those though...

Simplified the TC a bit to https://jsfiddle.net/fncywtfm/.

(@brajkumar, you said "Adding RB-stable for M-60", but you didn't add labels per that I think? I'm not quite sure this is that high prio, but I'd like to hear other opinions. Regardless, I'm not going to do anything about this for the next three weeks...)

[1] The spec for non-scaling-stroke is not 100% clear but alludes to host==screen - which is actually what we do after my change... (https://svgwg.org/svg2-draft/painting.html#non-scaling-stroke)
Cc: chrishtr@chromium.org
This is probably a broken use case for our changes to re-rastering semantics. chrishtr@, danajk@, is that your understanding too?

Seems like we would have to special case cases where we start with very small scales, or maybe we have that and are missing it for the SVG case.
Labels: BugSource-User PaintTeamTriaged-20170724

Comment 8 by f...@opera.com, Jul 24 2017

The "small scale" is not a problem in and of itself, but problem is rather that it's "baked" into the display list.
For the testcase reported in comment 0, I was also able to see a difference
in the second example, when clicking "3D" on Chrome 61, but not Chrome 59.
On Chrome 59, it displays as expected; on Chrome 61, the content disappears until
the end of the animation.  When choosing the "non-scaling stroke" option in the
second example, Chrome 59 also makes the content disappear during animation.

I bisected this difference to
https://chromium.googlesource.com/chromium/src/+/af0dcd17a4c16c8de78e2021d505cf1dbf212a95
(don't draw with a singular screen space transform).

This agrees with fs@opera's analysis that the problem is degenerate matrices.
I don't think there is a rasterization issue. If it was raster, we would
observe blurriness, not strange artifacts.

Comment 10 by f...@opera.com, Jul 26 2017

Cc: f...@opera.com schenney@chromium.org ajha@chromium.org
 Issue 748875  has been merged into this issue.

Comment 11 by f...@opera.com, Jul 26 2017

The merged dupe ( issue 748875 ) is an illustration sans the "3D".
Does the fix remain scheduled for M62 despite the merged case?
What's the current situation here?

Comment 14 by f...@opera.com, Oct 2 2017

Haven't managed to decide in which direction to take this. Leaning towards changing the condition for when the non-scaling-stroke transform is considered well-conditioned.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 3 2017

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

commit 4740f488f57876dbc84d9a3c54890c6b814f325f
Author: Fredrik Söderquist <fs@opera.com>
Date: Tue Oct 03 18:01:52 2017

Revert behavior for non-scaling-stroke transform

https://codereview.chromium.org/2711503002 fixed getScreenCTM by
including the actual CTM from the element to the "screen" ("host"
coordinate space.) This of course also included the scale factor
contributed by any ancestor transforms. Including these transforms
however made evaluation of the non-scaling-stroke transform (the host
transform) unreliable, because of how that transform is "baked" into
the display list. (So if the host transform is sampled when it is
degenerate, all rasterization using that display list will produce
"weird" results - which will remain until an new display list is
recorded.)
Revert the computation of the non-scaling-stroke transform back
something akin to what it was previously until we can figure out a
better way to deal with this situation.

Bug:  747708 
Change-Id: I8fdd19829c44f88d4d5e0bbbe4dec3ff2bee52a4
Reviewed-on: https://chromium-review.googlesource.com/697809
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#506099}
[add] https://crrev.com/4740f488f57876dbc84d9a3c54890c6b814f325f/third_party/WebKit/LayoutTests/svg/stroke/non-scaling-stroke-ancestor-transform-3d-expected.html
[add] https://crrev.com/4740f488f57876dbc84d9a3c54890c6b814f325f/third_party/WebKit/LayoutTests/svg/stroke/non-scaling-stroke-ancestor-transform-3d.html
[modify] https://crrev.com/4740f488f57876dbc84d9a3c54890c6b814f325f/third_party/WebKit/Source/core/layout/svg/LayoutSVGShape.cpp

Comment 16 by f...@opera.com, Oct 4 2017

Status: Fixed (was: Assigned)
Worth merging to M-62?

Comment 18 by f...@opera.com, Oct 4 2017

The risk should be low, but I'm not sure if the value is high enough?

Sign in to add a comment