New issue
Advanced search Search tips

Issue 905607 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

regression: svg fill url does not render correctly

Reported by joshung...@gmail.com, Nov 15

Issue description

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

Example URL:
https://www.rei.com/rei-garage/product/130332/salomon-pulse-mid-aero-jacket-mens

Steps to reproduce the problem:
1. Visit https://www.rei.com/rei-garage/product/130332/salomon-pulse-mid-aero-jacket-mens

2. Find the rating on the page.

What is the expected behavior?
The first four svg stars should be filled.

What went wrong?
The first four svg stars are not filled.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? N/A 

Did this work before? Yes 70.0.3538.102

Does this work in other browsers? N/A

Chrome version: 72.0.3610.0  Channel: n/a
OS Version: OS X 10.14.0
Flash Version:
 
Salomon_Pulse_Mid_Aero_Jacket_-_Men_s___REI_Outlet.png
81.2 KB View Download
Salomon_Pulse_Mid_Aero_Jacket_-_Men_s___REI_Outlet2.png
89.2 KB View Download
Bisect info: 607236 (good) - 607240 (bad)
https://chromium.googlesource.com/chromium/src/+log/a5d0ab06..2545c929?pretty=fuller
Suspecting r607240 = 2545c929927e71dacd6c6cc10a1318834b84d1bd SKIA roll
Landed in 72.0.3609.0

FWIW here's the isolated svg.
Expected: blue star
Observed: gray star
test.html
652 bytes View Download
Labels: Needs-Triage-M72 Needs-Bisect
Cc: susan.boorgula@chromium.org
Components: Internals>Skia Blink>SVG
Labels: -Pri-2 -Type-Compat -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable RegressedIn-72 Triaged-ET Target-72 M-72 FoundIn-72 OS-Linux OS-Windows Pri-1 Type-Bug-Regression
Status: Untriaged (was: Unconfirmed)
joshunger1@ Thanks for the update.

Able to reproduce this issue on Mac OS 10.13.6, Windows 10 and Ubuntu 17.10 on the reported version 72.0.3610.0 and latest Canary 72.0.3611.2.

Bisect Information:
===================
Good Build: 72.0.3608.0
Bad Build : 72.0.3610.0

By running per-revision script below is the ChangeLog URL 
https://chromium.googlesource.com/chromium/src/+log/affab0d0760b821a10df0a346a53c4fed069f060..2545c929927e71dacd6c6cc10a1318834b84d1bd

From the above Changelog, unable to find the right suspect.
Hence marking this issue as Untriaged and requesting the appropriate Dev team to look into the issue and help further.

Adding 'ReleaseBlock-Stable' for M-72 as this is a recent regression. Please feel free to remove if it is not applicable.

Thanks..
Cc: fmalita@chromium.org
Owner: michaelludwig@google.com
Thank you for the bisect and minimization woxxom!

This would seem to be:

https://skia.googlesource.com/skia.git/+/024072af62fcbca0163d27863ae2d0900d98f13d
Status: Assigned (was: Untriaged)
Definitely looks like that's the change. From the gradient definition in the SVG, it hits the newly defined rules for a "degenerate" gradient since the start and end points are so close together. But we can use a smaller threshold when detecting these cases so that gradients defined like this one don't fail. The .01% width of the gradient is still enough to provide an orientation for the gradient.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 16

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

commit d431c72395bd6cd53070f55c0003f478223e1227
Author: Michael Ludwig <michaelludwig@google.com>
Date: Fri Nov 16 16:14:59 2018

Update threshold for degenerate gradients

Bug:  chromium:905607 
Change-Id: I17aff1bffeb5bf3673568f57107b7014d46eb986
Reviewed-on: https://skia-review.googlesource.com/c/171521
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>

[modify] https://crrev.com/d431c72395bd6cd53070f55c0003f478223e1227/src/shaders/gradients/SkGradientShader.cpp

I confirmed locally that this Skia change will fix the issue, but we'll have to wait for it to roll into chromium as well now.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16

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

commit 0d1b4a26a90c22efdef05019ce37b284fd9fa091
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Nov 16 19:39:09 2018

Roll src/third_party/skia 5bf5a64fe7a9..07ee63d076e5 (14 commits)

https://skia.googlesource.com/skia.git/+log/5bf5a64fe7a9..07ee63d076e5


git log 5bf5a64fe7a9..07ee63d076e5 --date=short --no-merges --format='%ad %ae %s'
2018-11-16 csmartdalton@google.com sksl: Add builtinFMASupport to StandaloneShaderCaps
2018-11-16 brianosman@google.com Add SkPMColor4fFitsInBytes
2018-11-16 mtklein@google.com remove _DXDY_ sample procs
2018-11-16 jvanverth@google.com Remove use of integers for atlas indexing
2018-11-16 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader 000df8b42041..b16f9897a868 (4 commits)
2018-11-16 halcanary@google.com SkShaper: better error info.
2018-11-16 mtklein@google.com I think the _DXDY_ code may all be dead.
2018-11-16 michaelludwig@google.com Update threshold for degenerate gradients
2018-11-16 brianosman@google.com Use GrVertexWriter for GrRegionOp, add writeQuad()
2018-11-16 brianosman@google.com Do CCPR hairline coverage scaling in floats, rather than bytes
2018-11-16 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader 44994a88c9cc..000df8b42041 (1 commits)
2018-11-16 herb@google.com Fun with flags
2018-11-16 csmartdalton@google.com sksl: Polyfill fma() when not present in GLSL
2018-11-16 scroggo@google.com Add safety net logging for issue 118143775


Created with:
  gclient setdep -r src/third_party/skia@07ee63d076e5

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

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=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel

BUG= chromium:905607 ,chromium:b/118143775
TBR=rmistry@chromium.org

Change-Id: I8f2c6d04a79abcc003ca4c5c9c02fb4e71c05441
Reviewed-on: https://chromium-review.googlesource.com/c/1340386
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#608894}
[modify] https://crrev.com/0d1b4a26a90c22efdef05019ce37b284fd9fa091/DEPS

Status: Fixed (was: Assigned)

Sign in to add a comment