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

Issue 591993 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Compat



Sign in to add a comment

Initial incorrect rendering of SVG when using dasharray and dashoffset

Reported by paul.cu...@buzzfeed.com, Mar 4 2016

Issue description

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

Example URL:
http://codepen.io/anon/pen/MyapXG

Steps to reproduce the problem:
1. Visit the Codepen. You may need to download the sample or resize your screen so that you can see the initial SVG rendering area before the trigger-point is fired. 
2. Observe two dots (the start of the two SVG paths) rendered before you go past the scroll trigger point. These shouldn't be drawn.
2. Scroll past the end trigger point to draw in the magic word and draw the dot.
3. Scroll back up the page, and note that the two initially rendered dots are no longer visible.

What is the expected behavior?
A blank initial SVG area.

What went wrong?
The two initially rendered dots should not be on screen. stroke-dasharray and stroke-dashoffset are both set to the path "getTotalLength" by the JS. Other browsers (Safari, Firefox) correctly do not render the dots.

The fact that on scrolling back up (which restores the same SVG state as the initial rendering) DOES hide the dots may indicate this is a browser issue.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? N/A 

Did this work before? N/A 

Does this work in other browsers? Yes 

Chrome version: 49.0.2623.75  Channel: n/a
OS Version: OS X 10.10.5
Flash Version: Shockwave Flash 20.0 r0

The state of the SVG before the scroll effect is applied appears to be the same as after it is wound back, but the rendering is different.
 
Screen Shot 2016-03-04 at 14.35.02.png
26.5 KB View Download
Components: Blink>SVG

Comment 2 by f...@opera.com, Mar 4 2016

Cc: caryclark@chromium.org
Components: Internals>Skia
Labels: -OS-Mac OS-All
Status: Available (was: Unconfirmed)
It's an odd effect for an "open" path maybe, but I'm guessing that it's because we get a zero-length dash at the start (and end) of the path. I'll try to get a reduction done when I have the time.

Comment 3 by f...@opera.com, Mar 4 2016

I guess it could also be due to discrepancy between the two path length computations...

Comment 4 by f...@opera.com, Mar 7 2016

Reduced: https://jsfiddle.net/e2jyt3y3/2/ (black line kept for illustrative purposes)
(I think) this skia fiddle also shows the bug: https://fiddle.skia.org/c/8e0fc1db43826b4764bb7dcacef4f488
Cc: -caryclark@chromium.org
Owner: caryclark@chromium.org

Comment 7 by f...@opera.com, Mar 8 2016

Status: Assigned (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 9 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/18bbd00190623fb6cdb119df4a118ac3c1aed52a

commit 18bbd00190623fb6cdb119df4a118ac3c1aed52a
Author: caryclark <caryclark@google.com>
Date: Wed Mar 09 13:55:53 2016

don't create zero length intervals

Dashing a pattern without zero-length intervals should
not create them if the end of the on interval coincides
with the beginning of the initial dash offset.

R=reed@google.com
BUG= 591993 
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1766243004

Review URL: https://codereview.chromium.org/1766243004

[modify] https://crrev.com/18bbd00190623fb6cdb119df4a118ac3c1aed52a/gm/bug530095.cpp
[modify] https://crrev.com/18bbd00190623fb6cdb119df4a118ac3c1aed52a/src/utils/SkDashPath.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 9 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/7f229ed827d1dc5897acfa87d84b642ce34b678f

commit 7f229ed827d1dc5897acfa87d84b642ce34b678f
Author: robertphillips <robertphillips@google.com>
Date: Wed Mar 09 16:37:23 2016

Revert of don't create zero length intervals (patchset #1 id:1 of https://codereview.chromium.org/1766243004/ )

Reason for revert:
This may, or may not, be blocking the DEPS roll with:

svg/W3C-SVG-1.1/painting-stroke-04-t.svg

-      LayoutSVGPath {path} at (50,127) size 380x26 [stroke={[type=SOLID] [color=#000000] [stroke width=25.00] [dash offset=10.00] [dash array={10.00, 10.00}]}] [data="M 50 140 L 430 140"]
+      LayoutSVGPath {path} at (60,127) size 370x26 [stroke={[type=SOLID] [color=#000000] [stroke width=25.00] [dash offset=10.00] [dash array={10.00, 10.00}]}] [data="M 50 140 L 430 140"]

Original issue's description:
> don't create zero length intervals
>
> Dashing a pattern without zero-length intervals should
> not create them if the end of the on interval coincides
> with the beginning of the initial dash offset.
>
> R=reed@google.com
> BUG= 591993 
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1766243004
>
> Committed: https://skia.googlesource.com/skia/+/18bbd00190623fb6cdb119df4a118ac3c1aed52a

TBR=reed@google.com,fmalita@chromium.org,caryclark@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 591993 

Review URL: https://codereview.chromium.org/1779803002

[modify] https://crrev.com/7f229ed827d1dc5897acfa87d84b642ce34b678f/gm/bug530095.cpp
[modify] https://crrev.com/7f229ed827d1dc5897acfa87d84b642ce34b678f/src/utils/SkDashPath.cpp

Cc: robertphillips@chromium.org fmalita@chromium.org f...@opera.com
The CL above breaks svg/W3C-SVG-1.1/painting-stroke-04-t.svg and causes this diff:

10       LayoutSVGPath {path} at (50,127) size 380x26 [stroke={[type=SOLID] [color=#000000] [stroke width=25.00] [dash offset=10.00] [dash array={10.00, 10.00}]}] [data="M 50 140 L 430 140"]
10       LayoutSVGPath {path} at (60,127) size 370x26 [stroke={[type=SOLID] [color=#000000] [stroke width=25.00] [dash offset=10.00] [dash array={10.00, 10.00}]}] [data="M 50 140 L 430 140"]
                                  ^             ^
Need some SVG expertise to determine if this is expected. The Skia change has been rolled back for now.

Comment 11 by f...@opera.com, Mar 9 2016

That looks like an expected change to me - the black dashed "line" would previously start at x=50 with a "zero length interval" like the one we aim to remove here. It's not visibly noticeable because it uses 'butt' linecaps (switching to 'round' makes it more obvious.)

I guess we need to Skip+Rebaseline that across the DEPS roll.
FWIW this is a regression, we used to get this right.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 16 2016

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

commit d886bedb1e41ad07d2f5028c614bb70c88f5c5a0
Author: caryclark <caryclark@google.com>
Date: Wed Mar 16 21:17:45 2016

suppress test for rebaseline

Fixing bug referred to in  crbug.com/591993  correctly changes
the output of svg/W3C-SVG-1.1/painting-stroke-04-t.svg

Suppress the test failure until it can be rebaselined.

R=fmalita@chromium.org
BUG= 591993 

Review URL: https://codereview.chromium.org/1812623002

Cr-Commit-Position: refs/heads/master@{#381547}

[modify] https://crrev.com/d886bedb1e41ad07d2f5028c614bb70c88f5c5a0/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 17 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/d3cfd94228e6e58bb43dc4f799b4e443fba027a3

commit d3cfd94228e6e58bb43dc4f799b4e443fba027a3
Author: caryclark <caryclark@google.com>
Date: Thu Mar 17 12:33:28 2016

don't create zero length intervals

Dashing a pattern without zero-length intervals should
not create them if the end of the on interval coincides
with the beginning of the initial dash offset.

R=reed@google.com
BUG= 591993 
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1766243004

Committed: https://skia.googlesource.com/skia/+/18bbd00190623fb6cdb119df4a118ac3c1aed52a

Review URL: https://codereview.chromium.org/1766243004

[modify] https://crrev.com/d3cfd94228e6e58bb43dc4f799b4e443fba027a3/gm/bug530095.cpp
[modify] https://crrev.com/d3cfd94228e6e58bb43dc4f799b4e443fba027a3/src/utils/SkDashPath.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 21 2016

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

commit 7b7c6041ba1bdcf39ca2078ad864ded906209f5a
Author: caryclark <caryclark@google.com>
Date: Mon Mar 21 21:04:24 2016

fix svg test expectation

Change SVG test to values generated after fixing the
generation of zero length dash intervals.

The SVG test
  svg/W3C-SVG-1.1/painting-stroke-04-t.svg
was suppressed in commit
   d886bedb1e41ad07d2f5028c614bb70c88f5c5a0t

R=fmalita@chromium.org
BUG= 591993 

Review URL: https://codereview.chromium.org/1820893003

Cr-Commit-Position: refs/heads/master@{#382386}

[modify] https://crrev.com/7b7c6041ba1bdcf39ca2078ad864ded906209f5a/third_party/WebKit/LayoutTests/TestExpectations

Cc: ranjitkan@chromium.org
Labels: TE-Verified-M51 TE-Verified-51.0.2687.0
Rechecked this on chrome version 51.0.2687.0 on Windows 7, MAC 10.11.4, Ubuntu 14.04. fix is working as intended. No dots are displayed on screen.

Attached screen shot for the same. Adding TE-Verified labels.
Blank SVG.png
14.0 KB View Download
Status: Fixed (was: Assigned)

Sign in to add a comment