Issue metadata
Sign in to add a comment
|
webkit-filter:blur plus layers can lead to ubercompositor tiling glitches
Reported by
vku...@etouch.net,
Apr 13 2016
|
||||||||||||||||||||
Issue descriptionChrome Version:50.0.2661.75 (Official Build) ffad3c52452e415ab24c9cad1bae22eeb7428092-refs/branch-heads/2661@{#573} 32/64 Bit. OS:Windows What steps will reproduce the problem? 1.Launch chrome and navigate to https://espanol.southwest.com/ 2.Click on language section from top R.H.S and click on 'espanol' language and observe the page. Actual: Weird patches are seen after clicking on espanol language. Expected: Weird patches should not be seen and page should be loaded properly. This is a regression issue broken in 'M49' and will soon update other info.
,
Apr 14 2016
Since its a Windows only rendering artifact, filing under Blink>Paint.
,
Apr 14 2016
I haven't reproduced the bug, but the report sounds like it may be https://chromium.googlesource.com/chromium/src/+/859410a5e85488c0b79de51814351f30cf2814a5.
,
Apr 14 2016
r362878 does seem to be what triggers this, but adding a ui::Layer shouldn't cause quirks in the composited tiles in the renderer. I think this is tickling an existing bug The site uses .target > * { -moz-filter: blur(23px); -o-filter: blur(23px); -ms-filter: blur(23px); filter: url("data:image/svg+xml;utf8,<svg version='1.1' xmlns='http://www.w3.org/2000/svg'><filter id='blur'><feGaussianBlur stdDeviation='23'/></filter></svg>#blur"); filter: blur(23px); -webkit-filter: blur(23px); color: transparent\0/; text-shadow: 0 0 3px #000000, 3px 0 3px #000000, 0 5px 5px #000000, -6px 0 6px #000000, 0 -7px 7px #000000, 0 -8px 8px #000000, 0 -9px 9px #000000, 0 -10px 10px #000000, 0 -11px 11px #000000, 0 -12px 12px #000000, 0 -13px 13px #000000, 0 -13px 13px #000000, 0 -14px 14px #000000, 0 -15px 15px #000000;\0/; opacity: 0.01\0/; } Minimizing that I was able to reproduce with <html> <head> <style> .target > * { -webkit-filter: blur(23px); } </style> </head> <body class="target"> <div style="position:fixed; top: 100px; left: 100px; transform: translate3d(0, 0, 0);"> Text. </div> <div class="target" style="background: blue; font-size: 100pt"> Lorem ipsum dolor sit amet, consectetur adipiscing elit, </div> </body> To reproduce, just have a loading spinner on *any* tab, e.g. in the background. That adds an extra layer for the ubercompositor to deal with. There's probably some way to get the same effect by adding a layer to the webcontent instead. I'm more a UI person than a compositor person, so I'm a bit out of my depth here..
,
Apr 14 2016
,
Apr 14 2016
,
Apr 14 2016
,
Apr 14 2016
Thanks for the reduction! This kind of bug usually means something is defeating Skia's request for pixels outside the clip rect, in order to correctly blur at the clip edges. Unfortunately, I can't repro #4 in 52.0.2707.0 (Official Build) canary (32-bit) or 49.0.2623.110 (Official Build) m (32-bit) on Windows or a local build of r386978 on Linux64.
,
Apr 15 2016
The screenshot in #4 is the 52.0.2707.0 canary on Windows. You might need to wave the cursor over some of the blurry text, or try selecting some of it (while ensuring the tab-loading spinner is still running in the background). Confirmed it still happens in 52.0.2708.0 on Windows. (and it does stop happening as soon as the tab-loading spinner in the background tab stops updating).
,
Apr 15 2016
and I wasn't able to repro in 51.0.2704.7 dev on Linux - either blur.html or the original site - so this does seem to be Windows-specific.
,
Apr 23 2016
,
Apr 25 2016
Unable to repro this issue on Windows 7 for Google Chrome Canary Version - 52.0.2715.0 Screen-recording is attached. @vkupte: Could you please re-test the same on Google Chrome Canary Version - 52.0.2715.0 and update the thread accordingly. Thank you.
,
Apr 26 2016
It still repros for me in 52.0.2714.0 and 52.0.2716.0. It doesn't repro in #c12 because switching from Spanish to Spanish doesn't require the page to reload, so no loading spinner appears. Removing RBS since this regressed in m49 - we can't block stable release there. Also, reverting r362878 won't really fix the root cause of the glitch.
,
Apr 26 2016
tapted@: I'll take this if you don't mind. I've been looking at this a bit, and I think this is caused because the DirectRenderer::ComputeScissorRectForRenderPass is incorrect if a renderpassdrawquad that references a pass uses a filter, or if the pass itself has a RenderPassDrawQuad with a background filter (not sure about that). This only happens with the tab-loading spinner, because on frames where only the tab spinner is damaged the SurfaceAggregator won't aggregate the web contents, so all its renderpasses are destroyed. When they're recreated they're empty. Otherwise they'd at least be full of stale contents, which would look mostly correct. I think to fix this we'd need to make ComputeScissorRectForRenderPass aware of the entire tree of renderpassdrawquads in the frame. It might be best to move it into the SurfaceAggregator, because that already has to walk over all the quads anyway. I also need to fix the SurfaceAggregator to correctly handle choosing what quads to aggregate in this situation.
,
Apr 27 2016
Here's another test case that doesn't need the spinner. In the first frame the renderpass is occluded by two black layers so it's not drawn. After 2 seconds, it's partially unoccluded, but not enough is drawn for the filter.
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250 commit 575a9b96a2b2776a6a4ae81f2e20c2bf3e975250 Author: jbauman <jbauman@chromium.org> Date: Thu May 05 02:19:08 2016 Correctly handle damage involving filters in SurfaceAggregator There are two main issues with the handling of damage of contents affected by RenderPassDrawQuad filters: 1) Damage of a small part of a source Surface can affect a large part of the destination and 2) To draw damage on the destination, an unexpectedly large portion of the source must be used. To deal with these, the SurfaceAggregator must keep track of all output render passes affected by filters. Any Surface in an affected RenderPass will cause its parent Surface to be completely damaged (handling #1 above). To handle #2, affected RenderPasses will have their damage rects set to contain the entire output_rect. Then DirectRenderer can use that damage rect to determine what to draw. CopyRequests can also cause areas outside the damage rect to be read, so any RenderPass inside a CopyRequest (directly or indirectly) will have its damage rect set to the output rect. This system is very pessimistic about what should be drawn, but in general it shouldn't affect that many cases and should always be correct. BUG= 602941 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1927413002 Cr-Commit-Position: refs/heads/master@{#391733} [modify] https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250/cc/output/direct_renderer.cc [modify] https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250/cc/surfaces/surface_aggregator.cc [modify] https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250/cc/surfaces/surface_aggregator.h [modify] https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250/cc/surfaces/surface_aggregator_unittest.cc [modify] https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250/cc/trees/layer_tree_host_impl.cc
,
May 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fef422a0e8492b766d0713d9a6f5e3298c563081 commit fef422a0e8492b766d0713d9a6f5e3298c563081 Author: jbauman <jbauman@chromium.org> Date: Mon May 09 22:17:45 2016 Revert of Correctly handle damage involving filters in SurfaceAggregator (patchset #6 id:100001 of https://codereview.chromium.org/1927413002/ ) Reason for revert: Seems to cause rendering issues on mac. BUG= 610181 Original issue's description: > Correctly handle damage involving filters in SurfaceAggregator > > There are two main issues with the handling of damage of contents > affected by RenderPassDrawQuad filters: > 1) Damage of a small part of a source Surface can affect a large part > of the destination and > 2) To draw damage on the destination, an unexpectedly large portion > of the source must be used. > > To deal with these, the SurfaceAggregator must keep track of all output > render passes affected by filters. Any Surface in an affected RenderPass > will cause its parent Surface to be completely damaged (handling #1 > above). > > To handle #2, affected RenderPasses will have their damage rects set to > contain the entire output_rect. Then DirectRenderer can use that damage > rect to determine what to draw. > > CopyRequests can also cause areas outside the damage rect to be read, so > any RenderPass inside a CopyRequest (directly or indirectly) will have > its damage rect set to the output rect. > > This system is very pessimistic about what should be drawn, but in > general it shouldn't affect that many cases and should always be > correct. > > BUG= 602941 > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel > > Committed: https://crrev.com/575a9b96a2b2776a6a4ae81f2e20c2bf3e975250 > Cr-Commit-Position: refs/heads/master@{#391733} TBR=piman@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 602941 Review-Url: https://codereview.chromium.org/1963693003 Cr-Commit-Position: refs/heads/master@{#392445} [modify] https://crrev.com/fef422a0e8492b766d0713d9a6f5e3298c563081/cc/output/direct_renderer.cc [modify] https://crrev.com/fef422a0e8492b766d0713d9a6f5e3298c563081/cc/surfaces/surface_aggregator.cc [modify] https://crrev.com/fef422a0e8492b766d0713d9a6f5e3298c563081/cc/surfaces/surface_aggregator.h [modify] https://crrev.com/fef422a0e8492b766d0713d9a6f5e3298c563081/cc/surfaces/surface_aggregator_unittest.cc [modify] https://crrev.com/fef422a0e8492b766d0713d9a6f5e3298c563081/cc/trees/layer_tree_host_impl.cc
,
May 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe19524b2c646dc6b7b8b4849e9139216805c0a7 commit fe19524b2c646dc6b7b8b4849e9139216805c0a7 Author: jbauman <jbauman@chromium.org> Date: Wed May 11 22:06:12 2016 Reland: Correctly handle damage involving filters in SurfaceAggregator There are two main issues with the handling of damage of contents affected by RenderPassDrawQuad filters: 1) Damage of a small part of a source Surface can affect a large part of the destination and 2) To draw damage on the destination, an unexpectedly large portion of the source must be used. To deal with these, the SurfaceAggregator must keep track of all output render passes affected by filters. Any Surface in an affected RenderPass will cause its parent Surface to be completely damaged (handling #1 above). To handle #2, affected RenderPasses will have their damage rects set to contain the entire output_rect. Then DirectRenderer can use that damage rect to determine what to draw. In the case that overlays are used the root damage rect can temporarily be expanded for the frame, so in that case the entire child renderpass needs to be drawn. CopyRequests can also cause areas outside the damage rect to be read, so any RenderPass inside a CopyRequest (directly or indirectly) will have its damage rect set to the output rect. This system is very pessimistic about what should be drawn, but in general it shouldn't affect that many cases and should always be correct. BUG= 602941 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1966073002 Cr-Commit-Position: refs/heads/master@{#393084} [modify] https://crrev.com/fe19524b2c646dc6b7b8b4849e9139216805c0a7/cc/output/direct_renderer.cc [modify] https://crrev.com/fe19524b2c646dc6b7b8b4849e9139216805c0a7/cc/surfaces/surface_aggregator.cc [modify] https://crrev.com/fe19524b2c646dc6b7b8b4849e9139216805c0a7/cc/surfaces/surface_aggregator.h [modify] https://crrev.com/fe19524b2c646dc6b7b8b4849e9139216805c0a7/cc/surfaces/surface_aggregator_unittest.cc [modify] https://crrev.com/fe19524b2c646dc6b7b8b4849e9139216805c0a7/cc/trees/layer_tree_host_impl.cc
,
May 17 2016
Is this fixed?
,
Jun 3 2016
,
Jun 17 2016
Ping. Should this be Pri-1? If so please assign a milestone.
,
Jun 17 2016
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by vku...@etouch.net
, Apr 13 2016Owner: tapted@chromium.org
Status: Assigned (was: Unconfirmed)
563 KB
563 KB Download
449 KB
449 KB Download