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

Issue 849080 link

Starred by 17 users

Issue metadata

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



Sign in to add a comment

SVG vector-effect="non-scaling-stroke" is not rendering properly

Reported by kiru...@cadonix.com, Jun 3 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.62 Safari/537.36

Example URL:
https://jsfiddle.net/zd0g4zbj/

Steps to reproduce the problem:
1. Zoom In and Zoom Out to see the difference in the enclosed URL for circle (Red Dot) and Rectangle path
2. 
3. 

What is the expected behavior?
It was working fine in Old version of Chrome

What went wrong?
The Stroke Width is being rendered wrongly during Non-Scaling Stroke

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? N/A

Chrome version: 67.0.3396.62  Channel: stable
OS Version: 14.04.5 LTS
Flash Version:
 

Comment 1 by tkent@chromium.org, Jun 3 2018

Components: Blink>SVG
Labels: Needs-Triage-M67
Cc: pbomm...@chromium.org

Comment 4 by f...@opera.com, Jun 4 2018

Could have a similar cause to  issue 848234 .
Labels: -Pri-2 -Type-Compat hasbisect-per-revision ReleaseBlock-Stable Triaged-ET RegressedIn-67 M-67 Target-67 FoundIn-67 Target-68 Target-69 FoundIn-69 FoundIn-68 OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported chrome version 67.0.3396.62 and on latest chrome# 69.0.3448.0 using Windows-10, Mac 10.12.6 & Ubuntu 14.04  hence providing Bisect Info
Bisect Info:
================
Good build: 67.0.3371.0
Bad build: 67.0.3372.0

You are probably looking for a change made after 543290 (known good), but no later than 543291 (first known bad).
https://chromium.googlesource.com/chromium/src/+log/63667eca2a1d86f5de61f29c7c5c6417cc351757..3a3c78a924a686ed0d3f90d765b00cdd78453e11
Reviewed-on: https://chromium-review.googlesource.com/923572

@Xianzhu Wang: Please confirm the issue and help in re-assigning if it is not related to your change.
Adding ReleaseBlock-Stable as it is seems a recent break, feel free to remove it if not applicable.

Thanks!
Cc: pdr@chromium.org
 Issue 848234  has been merged into this issue.
Cc: trchen@chromium.org
Labels: -M-67 -Target-67
Let's target this to M-68.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 5 2018

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

commit 85b96ea98619ea7d775e1277ab859cea6d613dbb
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Tue Jun 05 18:20:32 2018

[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform

In Pre-SPv175 we forced subtree paint invalidation on non-composited
transform change. SPv175 no longer does that, causing
NonScalingStrokeTransform not updated on ancestor transform change.

We also had a non-obvious bug that LayoutSVGShape::StrokeBoundingBox
didn't get updated on ancestor transform change.

Now always explicitly update non-scaling-stroke data during layout.

Bug:  849080 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia61eb94f43e53a71a80e1102e4d605e4331f44b1
Reviewed-on: https://chromium-review.googlesource.com/1086715
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#564584}
[add] https://crrev.com/85b96ea98619ea7d775e1277ab859cea6d613dbb/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform-expected.html
[add] https://crrev.com/85b96ea98619ea7d775e1277ab859cea6d613dbb/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform.html
[modify] https://crrev.com/85b96ea98619ea7d775e1277ab859cea6d613dbb/third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
[modify] https://crrev.com/85b96ea98619ea7d775e1277ab859cea6d613dbb/third_party/blink/renderer/core/layout/svg/layout_svg_shape.h
[modify] https://crrev.com/85b96ea98619ea7d775e1277ab859cea6d613dbb/third_party/blink/renderer/core/paint/svg_shape_painter.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 6 2018

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

commit e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Wed Jun 06 05:24:49 2018

Revert "[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform"

This reverts commit 85b96ea98619ea7d775e1277ab859cea6d613dbb.

Reason for revert:  crbug.com/849968 

Original change's description:
> [PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform
> 
> In Pre-SPv175 we forced subtree paint invalidation on non-composited
> transform change. SPv175 no longer does that, causing
> NonScalingStrokeTransform not updated on ancestor transform change.
> 
> We also had a non-obvious bug that LayoutSVGShape::StrokeBoundingBox
> didn't get updated on ancestor transform change.
> 
> Now always explicitly update non-scaling-stroke data during layout.
> 
> Bug:  849080 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ia61eb94f43e53a71a80e1102e4d605e4331f44b1
> Reviewed-on: https://chromium-review.googlesource.com/1086715
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Fredrik Söderquist <fs@opera.com>
> Cr-Commit-Position: refs/heads/master@{#564584}

TBR=wangxianzhu@chromium.org,fs@opera.com

Change-Id: Ifbdd23b74dc45b5c8bc66c3d64bff580d5306f78
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  849080 ,  849968 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1087332
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564793}
[delete] https://crrev.com/eeb362f202da18d3eb477b2ab5e09b271a91d248/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform-expected.html
[delete] https://crrev.com/eeb362f202da18d3eb477b2ab5e09b271a91d248/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform.html
[modify] https://crrev.com/e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd/third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
[modify] https://crrev.com/e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd/third_party/blink/renderer/core/layout/svg/layout_svg_shape.h
[modify] https://crrev.com/e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd/third_party/blink/renderer/core/paint/svg_shape_painter.cc

Project Member

Comment 10 by sheriffbot@chromium.org, Jun 6 2018

Cc: viswa.karala@chromium.org
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-68
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 6 2018

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

commit 7eec667b47b35671945e6d9b20238636c303e50c
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Wed Jun 06 22:01:27 2018

Reland "[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform"

This reverts commit e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd.

Reason for revert: Reland with the null pointer issue fixed.

LayoutSVGEllipse and LayoutSVGRect override LayoutSVGShape methods not
to create paths in case that optimized algorithm can be used.
However, the original condition in their ShapeDependentStrokeContains()
might call LayoutSVGShape::ShapeDependentStrokeContains() without
use_path_fallback_ in some cases (e.g. when the shape is invalid).

Now ensure ShapeDependentStrokeContains() is called only if
use_path_fallback_ is set. Also ensure that use_path_fallback_ is set
whenever we need it.

Original change's description:
> Revert "[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform"
>
> This reverts commit 85b96ea98619ea7d775e1277ab859cea6d613dbb.
>
> Reason for revert:  crbug.com/849968 
>
> Original change's description:
> > [PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform
> >
> > In Pre-SPv175 we forced subtree paint invalidation on non-composited
> > transform change. SPv175 no longer does that, causing
> > NonScalingStrokeTransform not updated on ancestor transform change.
> >
> > We also had a non-obvious bug that LayoutSVGShape::StrokeBoundingBox
> > didn't get updated on ancestor transform change.
> >
> > Now always explicitly update non-scaling-stroke data during layout.
> >
> > Bug:  849080 
> > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > Change-Id: Ia61eb94f43e53a71a80e1102e4d605e4331f44b1
> > Reviewed-on: https://chromium-review.googlesource.com/1086715
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Reviewed-by: Fredrik Söderquist <fs@opera.com>
> > Cr-Commit-Position: refs/heads/master@{#564584}
>
> Change-Id: Ifbdd23b74dc45b5c8bc66c3d64bff580d5306f78
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  849080 ,  849968 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Reviewed-on: https://chromium-review.googlesource.com/1087332
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#564793}

Bug:  849080 ,  849968 
Change-Id: I15c2f80a2f80d11ccf356328ad41e8ab9d8de72f
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1089090
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#565050}
[add] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform-expected.html
[add] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform.html
[add] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/WebKit/LayoutTests/svg/stroke/isPointInStroke-non-scaling-stroke-invalid-shape-crash.html
[modify] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/blink/renderer/core/layout/svg/layout_svg_ellipse.cc
[modify] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/blink/renderer/core/layout/svg/layout_svg_rect.cc
[modify] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
[modify] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/blink/renderer/core/layout/svg/layout_svg_shape.h
[modify] https://crrev.com/7eec667b47b35671945e6d9b20238636c303e50c/third_party/blink/renderer/core/paint/svg_shape_painter.cc

Request to merge #c12 CL into M68. Verified on latest canary and ToT.
Labels: Merge-Request-68
Forgot to add Merge-Request-68 label.
Project Member

Comment 15 by sheriffbot@chromium.org, Jun 11 2018

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-68 Merge-Approved-68
Approved - branch:3440
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 11 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/004fdd2c991083ce71b559cc20acc358cf82350d

commit 004fdd2c991083ce71b559cc20acc358cf82350d
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Mon Jun 11 19:31:37 2018

Reland "[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform"

This reverts commit e2d97e7a96b86e8dca294ce59c3bf04a1c3c1cdd.

Reason for revert: Reland with the null pointer issue fixed.

LayoutSVGEllipse and LayoutSVGRect override LayoutSVGShape methods not
to create paths in case that optimized algorithm can be used.
However, the original condition in their ShapeDependentStrokeContains()
might call LayoutSVGShape::ShapeDependentStrokeContains() without
use_path_fallback_ in some cases (e.g. when the shape is invalid).

Now ensure ShapeDependentStrokeContains() is called only if
use_path_fallback_ is set. Also ensure that use_path_fallback_ is set
whenever we need it.

Original change's description:
> Revert "[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform"
>
> This reverts commit 85b96ea98619ea7d775e1277ab859cea6d613dbb.
>
> Reason for revert:  crbug.com/849968 
>
> Original change's description:
> > [PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform
> >
> > In Pre-SPv175 we forced subtree paint invalidation on non-composited
> > transform change. SPv175 no longer does that, causing
> > NonScalingStrokeTransform not updated on ancestor transform change.
> >
> > We also had a non-obvious bug that LayoutSVGShape::StrokeBoundingBox
> > didn't get updated on ancestor transform change.
> >
> > Now always explicitly update non-scaling-stroke data during layout.
> >
> > Bug:  849080 
> > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > Change-Id: Ia61eb94f43e53a71a80e1102e4d605e4331f44b1
> > Reviewed-on: https://chromium-review.googlesource.com/1086715
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Reviewed-by: Fredrik Söderquist <fs@opera.com>
> > Cr-Commit-Position: refs/heads/master@{#564584}
>
> Change-Id: Ifbdd23b74dc45b5c8bc66c3d64bff580d5306f78
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  849080 ,  849968 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Reviewed-on: https://chromium-review.googlesource.com/1087332
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#564793}

TBR=wangxianzhu@chromium.org

(cherry picked from commit 7eec667b47b35671945e6d9b20238636c303e50c)

Bug:  849080 ,  849968 
Change-Id: I15c2f80a2f80d11ccf356328ad41e8ab9d8de72f
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1089090
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Original-Commit-Position: refs/heads/master@{#565050}
Reviewed-on: https://chromium-review.googlesource.com/1096026
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#282}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[add] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform-expected.html
[add] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/WebKit/LayoutTests/paint/invalidation/svg/non-scaling-stroke-change-container-transform.html
[add] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/WebKit/LayoutTests/svg/stroke/isPointInStroke-non-scaling-stroke-invalid-shape-crash.html
[modify] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/blink/renderer/core/layout/svg/layout_svg_ellipse.cc
[modify] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/blink/renderer/core/layout/svg/layout_svg_rect.cc
[modify] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc
[modify] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/blink/renderer/core/layout/svg/layout_svg_shape.h
[modify] https://crrev.com/004fdd2c991083ce71b559cc20acc358cf82350d/third_party/blink/renderer/core/paint/svg_shape_painter.cc

Status: Fixed (was: Assigned)
Note that I also merged the CL for https://bugs.chromium.org/p/chromium/issues/detail?id=850659#c11 which is needed to fix a NULL pointer issue of the #c12 CL.
Labels: TE-Verified-M68 TE-Verified-68.0.3440.25
Able to reproduce the issue on chrome reported version 67.0.3396.62
Verified the fix on Mac 10.12.6, Windows-10 & Ubuntu 14.04 on Chrome version #68.0.3440.25 as per the comment#0
Attaching screen cast for reference.
Observed "Stroke Width got rendered without any issues"
Hence, the fix is working as expected.
Adding the verified label.

Thanks!
849080.mp4
1.2 MB View Download

Comment 20 by f...@opera.com, Jun 15 2018

 Issue 852805  has been merged into this issue.

Comment 21 by f...@opera.com, Jun 21 2018

Cc: schenney@chromium.org
 Issue 854170  has been merged into this issue.

Comment 22 by f...@opera.com, Jun 27 2018

 Issue 857147  has been merged into this issue.

Comment 23 by f...@opera.com, Jun 29 2018

 Issue 858934  has been merged into this issue.
 Issue 863793  has been merged into this issue.

Sign in to add a comment