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

Issue 707298 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 706366



Sign in to add a comment

Non-antialiased WebGL frames don't update with CPU compositing

Project Member Reported by capn@chromium.org, Mar 31 2017

Issue description

When using --disable-gpu, WebGL canvases don't update when using antialias:false. This happens when SwiftShader is being used either as a component or using the now integrated version, but also when replacing the SwiftShader libraries with ANGLE's.

When using anti-aliasing, we're seeing glBlitFramebuffer calls to resolve the multisampling, followed by a glReadPixels. Without anti-aliasing, neither happens, but rendering continues in the background.

This appears to be caused by https://codereview.chromium.org/2510583004. In the anti-aliased case, DrawingBuffer::resolveMultisampleFramebufferInternal() gets called, which sets m_contentsChangeCommitted. Without anti-aliasing, m_contentsChangeCommitted is never set, which causes ebGLRenderingContextBase::markContextChanged() to return early.
 
I'm fine with reverting that patch or working on a test and fix. How impactful is this bug - is it a problem that this patch shipped to M57 (stable now)?

Comment 2 by capn@chromium.org, Mar 31 2017

Owner: capn@chromium.org
Status: Started (was: Available)
SwiftShader WebGL fallback is broken on Stable. See  Issue 706366 .

I'm working on a patch which shouldn't revert the optimization. I'll CC you on it.
Ah, this was the cause of that bug. Apologies for the breakage! Thank you for handling the patch.

Comment 5 by kont...@trzeci.eu, Apr 3 2017

Hi,
It there a build what contains this patch, so that we can check if it fixes #706366 ? 

Comment 6 by capn@chromium.org, Apr 3 2017

No, the patch hasn't landed yet. It fixes a minimalized repro that I found (attached). Unfortunately when I try to run the AngryBots game in a local Release build it doesn't load correctly, both with and without the patch. So I'm trying to check if that's a recent regression.

You'll get notified here when the patch lands. A little while after that you'll be able to try out the Canary build. Note that this patch is only intended to take care of the SwiftShader fallback. Others are looking into the AMD switchable GPU blacklisting issue.
aa.html
8.5 KB View Download
no-aa.html
8.5 KB View Download
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 3 2017

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

commit 5f8b467781b1b08d611e1d05f31c10dee5538edb
Author: capn <capn@chromium.org>
Date: Mon Apr 03 21:16:06 2017

Fix non-anti-aliased frame update.

m_contentsCangeCommitted (now renamed to m_contentsChangeResolved) was
not getting set on non-anti-aliased DrawingBuffers, causing us to always
skip setting m_animationFrameInProgress to true. This in turn skips
calling glReadPixels() for CPU-composited frames.

BUG= 707298 , 608873 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2793653002
Cr-Commit-Position: refs/heads/master@{#461542}

[modify] https://crrev.com/5f8b467781b1b08d611e1d05f31c10dee5538edb/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/5f8b467781b1b08d611e1d05f31c10dee5538edb/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Comment 8 by capn@chromium.org, Apr 4 2017

The fix is live in Canary 59.0.3062.0.

Still needs to be cherry-picked into Beta and Stable, and I'll write a unit test for it before closing.
Hi @capn,
I confirm that the latest Canary has fixed a problem described in: https://bugs.chromium.org/p/chromium/issues/detail?id=706366 

We're looking forward for a cherry pick.

Thank you,
Piotr
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 13 2017

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

commit a4e71644c28270d794f1413b69e5e0efdfe79ab6
Author: capn <capn@chromium.org>
Date: Thu Apr 13 10:41:59 2017

Add DrawingBuffer multisample resolve unit test.

BUG= 707298 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I171c9b733d735f8a035640fd96b8fda5dd7686ec

Review-Url: https://codereview.chromium.org/2812963003
Cr-Commit-Position: refs/heads/master@{#464370}

[modify] https://crrev.com/a4e71644c28270d794f1413b69e5e0efdfe79ab6/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp
[modify] https://crrev.com/a4e71644c28270d794f1413b69e5e0efdfe79ab6/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
[modify] https://crrev.com/a4e71644c28270d794f1413b69e5e0efdfe79ab6/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTestHelpers.h

Comment 11 by capn@chromium.org, Apr 13 2017

Labels: Merge-Request-58
Requesting merge of https://chromium.googlesource.com/chromium/src.git/+/5f8b467781b1b08d611e1d05f31c10dee5538edb to Beta (M58). It's been in Canary/Dev for 10 days, and we have a new unit test to prevent the regression from happening again.
Project Member

Comment 12 by sheriffbot@chromium.org, Apr 13 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 13 by capn@chromium.org, Apr 13 2017

Cc: gov...@chromium.org
Krishna, can we get this fix merged into Beta before it goes Stable?
Cc: abdulsyed@chromium.org
Please add applicable OSs.

Comment 15 by capn@chromium.org, Apr 13 2017

Labels: OS-Windows
Thank you capn@.

+abdulsyed@ for M58 merge review.
Labels: -Merge-Review-58 Merge-Approved-58
Approving for M58 merge for https://chromium.googlesource.com/chromium/src.git/+/5f8b467781b1b08d611e1d05f31c10dee5538edb, based on comment #11. Please merge ASAP. thanks!

Comment 18 by capn@chromium.org, Apr 14 2017

Owner: abdulsyed@chromium.org
https://codereview.chromium.org/2810333009 Please land it for me, I'm not a Committer. Thanks.

Comment 19 by kbr@chromium.org, Apr 14 2017

Owner: kbr@chromium.org
Nicolas, I'll help you land this. I happen to have an M58 workspace set up for doing so.

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 14 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c777beab66117a31006f039de191c9a9dccefc7

commit 3c777beab66117a31006f039de191c9a9dccefc7
Author: Kenneth Russell <kbr@chromium.org>
Date: Fri Apr 14 23:04:29 2017

Fix non-anti-aliased frame update.

m_contentsCangeCommitted (now renamed to m_contentsChangeResolved) was
not getting set on non-anti-aliased DrawingBuffers, causing us to always
skip setting m_animationFrameInProgress to true. This in turn skips
calling glReadPixels() for CPU-composited frames.

BUG= 707298 , 608873 
TBR=kbr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2793653002
Cr-Commit-Position: refs/heads/master@{#461542}
(cherry picked from commit 5f8b467781b1b08d611e1d05f31c10dee5538edb)

Review-Url: https://codereview.chromium.org/2810333009 .
Cr-Commit-Position: refs/branch-heads/3029@{#724}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/3c777beab66117a31006f039de191c9a9dccefc7/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/3c777beab66117a31006f039de191c9a9dccefc7/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

Comment 21 by kbr@chromium.org, Apr 14 2017

Owner: capn@chromium.org
Status: Fixed (was: Started)

Sign in to add a comment