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

Issue 602941 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
not on Chrome anymore
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



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 description

Chrome 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.

 

Comment 1 by vku...@etouch.net, Apr 13 2016

Labels: hasbisect
Owner: tapted@chromium.org
Status: Assigned (was: Unconfirmed)
Manual regression range:
Good Build: 49.0.2580.0 
Bad Build:  49.0.2581.0 

Narrow bisect:
https://chromium.googlesource.com/chromium/src/+log/252e4b70d06a3a469f5fb53db6ecf194be8c5751..d0a3dca9f0b56481771d0dae360e7ea9a44eea60?pretty=fuller&n=30

Suspecting: 362878?
Kindly help to re-assign, if your changes are not cause for this issue.

Note: Issue not seen on Mac & Linux OS.
Actual_Result.mp4
563 KB Download
Expected_Result.mp4
449 KB Download

Comment 2 by keishi@chromium.org, Apr 14 2016

Components: -Blink Blink>Paint
Since its a Windows only rendering artifact, filing under Blink>Paint.

Comment 3 by pdr@chromium.org, Apr 14 2016

Cc: ericrk@chromium.org
I haven't reproduced the bug, but the report sounds like it may be https://chromium.googlesource.com/chromium/src/+/859410a5e85488c0b79de51814351f30cf2814a5.

Comment 4 by tapted@chromium.org, Apr 14 2016

Cc: jbroman@chromium.org
Components: Internals>Compositing>Ubercomp
Summary: webkit-filter:blur plus layers can lead to ubercompositor tiling glitches (was: Regression:Weird patches are seen after clicking on espanol language.)
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..
blur-glitch.png
271 KB View Download
blur.html
359 bytes View Download

Comment 5 by danakj@chromium.org, Apr 14 2016

Cc: jbau...@chromium.org

Comment 6 by danakj@chromium.org, Apr 14 2016

Cc: senorblanco@chromium.org

Comment 7 by piman@chromium.org, Apr 14 2016

Cc: enne@chromium.org
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.

Comment 9 by tapted@chromium.org, 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).
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.
Labels: ReleaseBlock-Stable
Cc: rnimmagadda@chromium.org
Labels: Needs-Feedback
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.
602941.mp4
2.6 MB Download
Labels: -Needs-Feedback -ReleaseBlock-Stable -M-51 M-49
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.
blur-video.mov
3.7 MB Download
Cc: tapted@chromium.org
Owner: jbau...@chromium.org
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.
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.
blur2.html
792 bytes View Download
Project Member

Comment 16 by bugdroid1@chromium.org, 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

Project Member

Comment 17 by bugdroid1@chromium.org, 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

Project Member

Comment 18 by bugdroid1@chromium.org, 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

Comment 19 by piman@chromium.org, May 17 2016

Is this fixed?
Components: -Blink>Paint
Ping. Should this be Pri-1? If so please assign a milestone.
Status: Fixed (was: Assigned)

Sign in to add a comment