Issue metadata
Sign in to add a comment
|
rx attribute for svg rect breaks rendering when visualizing large numbers of rectangles
Reported by
o...@signalfx.com,
Jan 23 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36 Steps to reproduce the problem: 1. Open reduced test case. 2. Both SVGs do not render same number of boxes, even though they should. Only difference is the red boxes do not have an "rx" attribute. What is the expected behavior? Both SVGs should render the same number of boxes. What went wrong? The SVG with blue boxes experiences a rendering error manifesting in white areas where the rect elements aren't rendered. These areas of white seem to vary with viewport size. We've been able to reproduce it whenever viewport size is larger than the svg width (700px). Did this work before? N/A Does this work in other browsers? Yes Chrome version: 55.0.2883.95 Channel: stable OS Version: OS X 10.12.0 Flash Version: Shockwave Flash 24.0 r0 Reproduced on a few different machines: Chrome 55.0.2883.95 on OS X 10.11.6 (El Capitan) Chrome 55.0.2883.95 on OS X 10.12 (Sierra) Chrome 55.0.2883.95 on OS X 10.12.2 (Sierra) JSFiddle of test case: https://jsfiddle.net/r0qbteuv/3/
,
Jan 24 2017
pdr: Not sure, but for what it's worth, I'm not able to repro https://crbug.com/675499 on Chrome 55.0.2883.95 on OS X 10.12.
,
Jan 24 2017
,
Jan 24 2017
Able to reproduce the issue on 55.0.2883.95 on MAC 12.2. Below are the bisect details for the same: Bisect Info: ============ 55.0.2860.0 - Good Build 55.0.2864.0 - Bad build Change Log: https://chromium.googlesource.com/chromium/src/+log/db00688793f63e1f3ccd71f883ec3129bd169abe..846a59b93f9911517c7aeb05b47af9b4c0cad175 Suspecting the below change could be a possible culprit: https://chromium.googlesource.com/skia.git/+/ceb93abddc81ccc0b4f93d958135632ed4cebfd4 @ fmalita: Assigning to you, kindly have a look into it. Please help us to find an owner if not with respect to your change. P.S: Issue is not observed on Windows and Linux OS. Also hasbisect per revision bisect script invoked builds where chrome was showing network error beteween build 55.0.2862.0 and 55.0.2863.0
,
Jan 24 2017
The suspected CL in c#4 doesn't touch code used in Chromium. Looks like a bad bisect. I've retried, and got You are probably looking for a change made after 415138 (known good), but no later than 415162 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/712fc6049b32036a00ee1684bc1db726c969db59..72b7c4e88a9543a11a649fadc2ac5c2eed0e15f9 (note: the repro requires GPU rasterization) Skia roll in that range: https://chromium.googlesource.com/skia.git/+log/839d3ad854c0..6f5df6acb7de Most likely culprit: https://chromium.googlesource.com/skia.git/+/84839f6fb3d0de9088be3d53e81df195331ed1c9
,
Jan 24 2017
,
Feb 9 2017
The following revision refers to this bug: https://skia.googlesource.com/skia/+/8cefe40ab094bfbea532761dad1a857eb3d4b831 commit 8cefe40ab094bfbea532761dad1a857eb3d4b831 Author: Jim Van Verth <jvanverth@google.com> Date: Thu Feb 09 17:29:31 2017 Don't batch circles and circular rrects beyond index limit BUG= skia:6158 , chromium:690144 , chromium:688582, chromium:684112 Change-Id: I7a6d1fb73cbe6cb4328848acd153ff2505b5fea2 Reviewed-on: https://skia-review.googlesource.com/8256 Reviewed-by: Florin Malita <fmalita@chromium.org> Reviewed-by: Robert Phillips <robertphillips@google.com> Commit-Queue: Jim Van Verth <jvanverth@google.com> [modify] https://crrev.com/8cefe40ab094bfbea532761dad1a857eb3d4b831/src/gpu/ops/GrOvalOpFactory.cpp
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7273c6c6210628e39f02afa086ed025413cf8611 commit 7273c6c6210628e39f02afa086ed025413cf8611 Author: skia-deps-roller <skia-deps-roller@chromium.org> Date: Thu Feb 09 23:24:58 2017 Roll src/third_party/skia/ 1df161ab8..2c49a4185 (15 commits). https://skia.googlesource.com/skia.git/+log/1df161ab8a6a..2c49a4185865 $ git log 1df161ab8..2c49a4185 --date=short --no-merges --format='%ad %ae %s' 2017-02-09 bsalomon Remove inner/outer threshold restriction on SkAlphaThresholdFilter. 2017-02-09 kjlubick Prevent waiting for NexusPlayers indefinitely 2017-02-09 fmalita [4fGradient] Relax interval checks for SkGradientShaderBase also 2017-02-09 mtklein exclude _none.cpp in G3 build 2017-02-09 ethannicholas re-land of skslc type constructor cleanups 2017-02-09 bsalomon Temporarily don't mark alpha threshold fp as modulating 2017-02-09 mtklein Don't build sksl if we're not building Ganesh. 2017-02-09 msarett Pixel conversion refactors: use raster pipeline for 565 and gray 2017-02-09 mtklein Make src/effects explicitly optional. 2017-02-09 ethannicholas added support for sk_ClipDistance 2017-02-09 bsalomon Re-enable processor optimization test with some fixes. 2017-02-09 fmalita [4fGradient] Relax interval checks 2017-02-09 herb Remove last use of ktx.h 2017-02-09 robertphillips Fix simple-magnification GM in "--preAbandonGpuContext" mode 2017-02-09 jvanverth Don't batch circles and circular rrects beyond index limit Created with: roll-dep src/third_party/skia BUG= 690144 ,688582, 684112 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel TBR=robertphillips@google.com Review-Url: https://codereview.chromium.org/2680673006 Cr-Commit-Position: refs/heads/master@{#449461} [modify] https://crrev.com/7273c6c6210628e39f02afa086ed025413cf8611/DEPS
,
Feb 13 2017
Looks to be working in current Chrome Canary
,
Feb 14 2017
Verified the issue on Mac Retina 10.12.2 using Chrome version 58.0.3012.0 as per the comment #0. Observed that the fix is working as expected. Attaching the screen cast for reference. Hence, adding the verified labels. Note: Issue is specific to Mac Retina. Issue is not observed on Windows-7 and Linux Ubuntu-14.04 and MacBookAir-10.12.2. Thanks.
,
Feb 21 2017
,
Feb 21 2017
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 21 2017
This bug exists since M55 and marked as P2. Can this wait until M58 as we're getting closer to M57 Stable promotion?
,
Feb 21 2017
Sure, no great hurry.
,
Feb 21 2017
Actually, this is related to chromium:688582, which is marked Priority 1.
,
Feb 21 2017
Thank you jvanverth@. Bug 688582 marked as P1 but also exists on current M56 Stable. Could you please confirm this change will be a safe merge to M57?
,
Feb 22 2017
There are reports that it is causing a perf slowdown, so no, at this point I can't say that it's a safe merge.
,
Feb 22 2017
Rejecting merge to M57 based on comment #17. Please let me know if there is any concern here. Thank you.
,
Mar 7 2017
If some change completely broke rendering and the fix makes it so that we render correctly, why isn't the thing reverted completely and the revert merged? As-is, we'll end up in the slower-but-correct rendering mode in m58, and we'll have busted rendering in m57.
,
Mar 7 2017
I think the fix should be merged. The root cause of the performance regression is yet to be determined but we know the fix for this bug is not the cause.
,
Mar 7 2017
Flipping back to requested then. Please seem comment 19 and 20 -- we're fixing a very visible drawing regression, and the perf regression is by now verified to be caused by something else.
,
Mar 7 2017
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 7 2017
This bug exists on M55 and M56 and is P2. Plan is to cut M57 Stable RC today for Desktop. Will this be a safe merge to M57 (no functional, perf or stability regression)?
,
Mar 7 2017
Yes, this is a safe change.
,
Mar 7 2017
Approving merge to M57 branch 2987 based on comment #21 and #24. Please merge ASAP. Thank you.
,
Mar 7 2017
The following revision refers to this bug: https://skia.googlesource.com/skia/+/ae9cc5d3588d52f4b371b55845704b25d88cf06d commit ae9cc5d3588d52f4b371b55845704b25d88cf06d Author: Brian Salomon <bsalomon@google.com> Date: Tue Mar 07 16:20:54 2017 Don't batch circles and circular rrects beyond index limit M57 Cherry pick BUG= skia:6158 , chromium:690144 , chromium:688582, chromium:684112 Change-Id: I7a6d1fb73cbe6cb4328848acd153ff2505b5fea2 Reviewed-on: https://skia-review.googlesource.com/8256 Reviewed-by: Florin Malita <fmalita@chromium.org> Reviewed-by: Robert Phillips <robertphillips@google.com> Commit-Queue: Jim Van Verth <jvanverth@google.com> Reviewed-on: https://skia-review.googlesource.com/9383 Reviewed-by: Brian Salomon <bsalomon@google.com> [modify] https://crrev.com/ae9cc5d3588d52f4b371b55845704b25d88cf06d/src/gpu/ops/GrOvalOpFactory.cpp
,
Mar 7 2017
This is already merged to M57 per comment #26. So removing "Merge-Approved-57" label. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by pdr@chromium.org
, Jan 23 2017