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

Issue 816763 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

SVG Text Animation

Reported by reliza1...@gmail.com, Feb 27 2018

Issue description

Steps to reproduce the problem:
1. Go to this text animation: https://codepen.io/supah/pen/vXyBza?editors=1111
2. View using chrome version 60.0.3112.90. You'll see no text is displayed originally in the console. Then text animates.
3. Use chrome version 64.0.3282.137. You'll see text is already displayed on load. It should be hidden at first.

What is the expected behavior?

What went wrong?
SVG Text animation is working incorrectly.

Did this work before? Yes 60.0.3112.90

Chrome version: 64.0.3282.137  Channel: n/a
OS Version: 7.0.0; SM-G955U Build/NRD90M
Flash Version: 

Thank you for the help!
 
Cc: sandeepkumars@chromium.org
Labels: Needs-triage-Mobile Triaged-Mobile Needs-Feedback
Tested the issue in Android and could not reproduce the issue. No text is seen.

Steps Followed:
1. Launch the Chrome Browser.
2. Navigated to https://codepen.io/supah/pen/vXyBza?editors=1111
3. No text is seen on first load

Chrome versions tested:
64.0.3282.137
OS:
Android 7.0.0

Android Devices:
Samsung J7

@marcin: Could you please confirm the expected behaviour and help us with a screencast for further triaging.

Thanks!!

Hello, attached are screenshots of what is occurring in the working chrome version, 60.0.3112.90, and the not working version, 64.0.3282.137. 

Sorry, I don't have the ability to screencast at the moment but please let me know if you need anything else. 

Thanks!
Chrome-version-6403282137-SVG-text-Animation-issue-with-screenshots.docx
1.3 MB Download
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 27 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Thanks for the reply.

Could you please attach the screenshots in accessible format as the above mentioned doc doesn't contain any screenshots in it.

Thanks!!
Hello, screenshots were in the doc. I'm attaching them all separately here, please let me know if you have further issues. 

Here's the link to the SVG Text animation if you need it again:
https://codepen.io/supah/pen/vXyBza?editors=1111

Images entitled "working chrome version" shows the animation working correctly from Chrome version 60.0.3112.90.

Images entitled "broken chrome version" shows the animation broken in the new Chrome version, 64.0.3282.137.

Thank you!
State-1-working-chrome-version.png
53.7 KB View Download
State-2-working-chrome-version.png
68.5 KB View Download
State-3-working-chrome-version.png
69.2 KB View Download
State-1-broken-chrome-version.png
288 KB View Download
State-2-broken-chrome-version.png
289 KB View Download
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 28 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Thanks for the response.

Still I'm having issue's with your screenshots as I didn't get the Actual and expected behaviours as the attached screenshots "State-3-working-chrome-version.png" and "State-2-broken-chrome-version.png" are looks to be same. (You have attached working screenshots from desktop and not working screenshots from mobile), can you please be more specific about it as we got confused in finding Expected and Actual behaviour with the attached screenshots.

Thanks!!

Comment 8 by alph@chromium.org, Mar 2 2018

Components: -Platform>DevTools Blink>SVG
Labels: -Needs-Feedback -Via-Wizard-DeveloperTools
I've confirmed, and note the correct behavior in Firefox. I'll bisect.
Standalone test file (still using a bunch of 3rd party stuff)
cr816763.html
4.4 KB View Download
Cc: bunge...@chromium.org
Components: -Blink>SVG Internals>Skia
Labels: FoundIn-66 RegressedIn-64 FoundIn-64 FoundIn-65 OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Untriaged (was: Unconfirmed)
Bisect goes to this range:
https://chromium.googlesource.com/chromium/src/+log/f1317f4bef330412662755fb32dd73634190ebec..4c6cc67b76861b1927bcad841b4eba11b779d1e8

Only the Skia roll seems like it could affect this:
https://chromium.googlesource.com/chromium/src/+/3b0cf8b82a0e9a4b58a80b697e50a5a5dabc2954

Which is this set of Skia changes:
https://skia.googlesource.com/skia.git/+log/9eccfff67472..065b41dd9078

No idea which of those might cause hidden text to start rendering but most obvious would be "Glyphs without paths don't draw with paths."
https://skia.googlesource.com/skia.git/+/1e26ba9e966b219005dc05e031f47457ae604cd3
Cc: -bunge...@chromium.org
Owner: bunge...@chromium.org
Status: Assigned (was: Untriaged)
I locally reverted https://skia.googlesource.com/skia.git/+/1e26ba9e966b219005dc05e031f47457ae604cd3 and that does change the behavior. Though at the moment I can't say why. I'll take a look.
Managed to remove all the javascript from the reproduction, attaching that. It appears that what may be happening is that this creates a stroke with a offset of -900 which is on for 900 and then off for 900. As a result, the initial state should be nothing drawn since the glyphs are being drawn with a dash which doesn't hit any of them.

The Skia change in question is being too aggressive. It should be checking if the glyphs themselves have a path, not the the resolved path to draw from is empty (after dashing).
cr816763_a.html
2.2 KB View Download
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 6 2018

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

commit ba8feb56c301563186852023eb531bb2076eaf9a
Author: Ben Wagner <bungeman@google.com>
Date: Tue Mar 06 16:07:56 2018

Draw glyphs from paths if they have an empty path.

This distuguishes between glyphs which do not have a path and glyphs
which have a path but that path resolves to the empty path.

BUG= chromium:816763 

Change-Id: Id6c7dd66cdad3868bf3fe15bcb6e5e6f2ca82405
Reviewed-on: https://skia-review.googlesource.com/112484
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

[modify] https://crrev.com/ba8feb56c301563186852023eb531bb2076eaf9a/tests/DrawTextTest.cpp
[modify] https://crrev.com/ba8feb56c301563186852023eb531bb2076eaf9a/src/core/SkScalerContext.cpp
[modify] https://crrev.com/ba8feb56c301563186852023eb531bb2076eaf9a/src/core/SkScalerContext.h

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 6 2018

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

commit 511e14c8a35a65d6c89d96efb870f84204092e92
Author: skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Mar 06 19:04:32 2018

Roll src/third_party/skia/ bd6525304..8bac92800 (9 commits)

https://skia.googlesource.com/skia.git/+log/bd6525304d44..8bac928009b8

$ git log bd6525304..8bac92800 --date=short --no-merges --format='%ad %ae %s'
2018-03-06 robertphillips Fix .fp files
2018-03-05 bungeman Draw glyphs from paths if they have an empty path.
2018-03-06 reed check for valid inputs to shadows
2018-03-05 herb Add GetTypefaceOrDefault to SkPaintPriv
2018-03-06 caryclark use points instead of verbs for interpolate
2018-03-01 hcm Update Skia milestone to 67
2018-03-05 reed make compute helper for blurs private
2018-03-06 robertphillips Move internal calls from GrContext to GrContextPriv
2018-03-05 scroggo Fix drawing SkAnimatedImages with transparency

Created with:
  roll-dep src/third_party/skia
BUG= 816763 


The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=fmalita@chromium.org

Change-Id: I9852eac848da96f9880bdbff53ac9e703af941fb
Reviewed-on: https://chromium-review.googlesource.com/951712
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#541166}
[modify] https://crrev.com/511e14c8a35a65d6c89d96efb870f84204092e92/DEPS

Labels: Merge-Request-66
This is a request to cherry-pick https://skia.googlesource.com/skia/+/ba8feb56c301563186852023eb531bb2076eaf9a to the https://skia.googlesource.com/skia/+/chrome/m66 branch. This change is in 67.0.3364.0 and is observed to fix the issue.
Project Member

Comment 17 by sheriffbot@chromium.org, Mar 7 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M66, could you pls confirm followings?


Is the change listed at #16 well baked/verified in Canary, having enough automation tests coverage and safe to merge?
Any other imp details to justify the merge.

I've been using this Canary all day now, everything seems fine. Part of this issue is that previously we didn't have a test for this case and this change adds a test (which is verified to have failed before the change and passes after). It's a fairly simple change, already cherry-picked cleanly locally, built and tested on the branch.

This change fixes a regression introduced in m64, so it would be good to fix this in m66. Since m65 has already been cut for stable, it probably doesn't make sense to go back that far.
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66 branch 3359 based on comment #19. Please merge ASAP so we can pick it up for tomorrow's M66 dev release. Thank you.
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 7 2018

Labels: merge-merged-m66
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/bf5c8ea2834ac1567cd0b0eab6b283cf1eb1ec37

commit bf5c8ea2834ac1567cd0b0eab6b283cf1eb1ec37
Author: Ben Wagner <bungeman@google.com>
Date: Wed Mar 07 22:36:24 2018

Draw glyphs from paths if they have an empty path.

This distuguishes between glyphs which do not have a path and glyphs
which have a path but that path resolves to the empty path.

BUG= chromium:816763 

Reviewed-on: https://skia-review.googlesource.com/112484
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

Change-Id: Iee95733eb31a4005c98ee8c230744ccec0016732
Cherry-Pick: ba8feb56c301563186852023eb531bb2076eaf9a
Approval:  https://crbug.com/816763#c20 
Reviewed-on: https://skia-review.googlesource.com/112902
Reviewed-by: Ben Wagner <bungeman@google.com>

[modify] https://crrev.com/bf5c8ea2834ac1567cd0b0eab6b283cf1eb1ec37/tests/DrawTextTest.cpp
[modify] https://crrev.com/bf5c8ea2834ac1567cd0b0eab6b283cf1eb1ec37/src/core/SkScalerContext.cpp
[modify] https://crrev.com/bf5c8ea2834ac1567cd0b0eab6b283cf1eb1ec37/src/core/SkScalerContext.h

Pls remove "Merge-Approved-66" label if nothing else is pending for M66.
Labels: -Merge-Approved-66
Status: Fixed (was: Assigned)

Sign in to add a comment