Skip to content

GitLab

T
tangled
Commit af59acd7 authored by Sunny Sachanandani's avatar Sunny Sachanandani Committed by Commit Bot
Browse files
Options

webgl: Fix incorrect redraw due to read pixels with software compositing 

The issue is that we don't reset the contents_changed_ flag to false at
the end of DrawingBuffer::FinishPrepareTransferableResourceSoftware.
Here's the problem:
1) We draw something into webgl canvas (DrawingBuffer) and export it as
   a software resource (FinishPrepareTransferableResourceSoftware).
2) Later we do a readPixels on the canvas (which is
   preserveDrawingBuffer: false).  The readPixels will clear the canvas
   to match spec behavior
   (WebGLRenderingContextBase::ClearIfComposited).
3) The next time blink document lifecycle happens (rAF, resize, etc.)
   DrawingBuffer thinks it needs to export the back buffer to compositor
   again because contents_changed_ is still true.
4) DrawingBuffer exports the cleared back buffer to compositor.

This CL includes a pixel test that simulates the above case by switching
tabs (and calling readPixels) to force a document lifecycle update. The
included test fails before this fix and passes afterwards.

Bug: 1047829
Change-Id: I1496919b974d84e975db28eafd9b8e6ffc71d91e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2040732Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739141}
parent b739e83f
Hide whitespace changes
Inline Side-by-side
Showing with 150 additions and 0 deletions
content/test/data/gpu/pixel_webgl_read_pixels_tab_switch.html 0 → 100644
View file @ af59acd7
<!DOCTYPE HTML>
<!-- READ BEFORE UPDATING:
If this test is updated make sure to increment the "revision" value of the
associated test in content/test/gpu/page_sets/pixel_tests.py. This will ensure
that the baseline images are regenerated on the next run.
-->
<html>
<head>
<meta name="viewport" content="initial-scale=1">
<title>WebGL Canvas Read Pixels And Tab Switch Test</title>
<style type="text/css">
.nomargin {
margin: 0px auto;
}
</style>
<script>
let g_swapsBeforeAck = 15;
let g_canvas;
let g_gl;
function sendResult(status) {
if (domAutomationController) {
domAutomationController.send(status);
} else {
console.log(status);
}
}
function waitForFinish() {
if (g_swapsBeforeAck == 0) {
sendResult("SUCCESS");
} else {
g_swapsBeforeAck--;
window.requestAnimationFrame(waitForFinish);
}
}
function handleVisibilityChange() {
if (document.visibilityState == "visible") {
waitForFinish();
} else {
// Read pixels will cause drawing buffer to be cleared, but it shouldn't
// cause it to be recomposited after the tab switches back.
let pixels = new Uint8Array(4);
g_gl.readPixels(0, 0, 1, 1, g_gl.RGBA, g_gl.UNSIGNED_BYTE, pixels);
}
}
function main() {
document.addEventListener("visibilitychange", handleVisibilityChange, false);
g_canvas = document.getElementById("c");
g_gl = g_canvas.getContext("webgl");
let width = g_canvas.width;
let height = g_canvas.height;
g_gl.enable(g_gl.SCISSOR_TEST);
g_gl.scissor(0, 0, width, height);
g_gl.clearColor(1, 0, 0, 1);
g_gl.clear(g_gl.COLOR_BUFFER_BIT);
g_gl.scissor(0, 0, width / 2, height / 2);
g_gl.clearColor(0, 1, 0, 1);
g_gl.clear(g_gl.COLOR_BUFFER_BIT);
g_gl.scissor(width / 2, height / 2, width / 2, height / 2);
g_gl.clearColor(0, 0, 1, 1);
g_gl.clear(g_gl.COLOR_BUFFER_BIT);
sendResult("READY");
}
</script>
</head>
<body onload="main()" class="nomargin">
<canvas id="c" width="100" height="100" class="nomargin" style="background-color:black"></canvas>
</body>
</html>
content/test/gpu/gpu_tests/pixel_test_pages.py
View file @ af59acd7
......@@ -690,6 +690,73 @@ class PixelTestPages(object):
},
]),
PixelTestPage(
'pixel_webgl_read_pixels_tab_switch.html',
base_name + '_WebGLReadPixelsTabSwitch',
test_rect=[0, 0, 100, 100],
optional_action='SwitchTabs',
tolerance=3,
expected_colors=[
{
'comment': 'top left, red',
'location': [5, 5],
'size': [40, 40],
'color': [255, 0, 0],
},
{
'comment': 'bottom right, red',
'location': [55, 55],
'size': [40, 40],
'color': [255, 0, 0],
},
{
'comment': 'top right, blue',
'location': [55, 5],
'size': [40, 40],
'color': [0, 0, 255],
},
{
'comment': 'bottom left, green',
'location': [5, 55],
'size': [40, 40],
'color': [0, 255, 0],
},
]),
PixelTestPage(
'pixel_webgl_read_pixels_tab_switch.html',
base_name + '_WebGLReadPixelsTabSwitch_SoftwareCompositing',
test_rect=[0, 0, 100, 100],
browser_args=sw_compositing_args,
optional_action='SwitchTabs',
tolerance=3,
expected_colors=[
{
'comment': 'top left, red',
'location': [5, 5],
'size': [40, 40],
'color': [255, 0, 0],
},
{
'comment': 'bottom right, red',
'location': [55, 55],
'size': [40, 40],
'color': [255, 0, 0],
},
{
'comment': 'top right, blue',
'location': [55, 5],
'size': [40, 40],
'color': [0, 0, 255],
},
{
'comment': 'bottom left, green',
'location': [5, 55],
'size': [40, 40],
'color': [0, 255, 0],
},
]),
]
......
content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
View file @ af59acd7
......@@ -28,9 +28,11 @@ crbug.com/653538 [ win amd-0x6613 ] Pixel_GpuRasterization_BlueBox [ RetryOnFail
[ android ] Pixel_CanvasUnacceleratedLowLatency2D [ Skip ]
[ android ] Pixel_RepeatedWebGLTo2D_SoftwareCompositing [ Skip ]
[ android ] Pixel_Canvas2DTabSwitch_SoftwareCompositing [ Skip ]
[ android ] Pixel_WebGLReadPixelsTabSwitch_SoftwareCompositing [ Skip ]
# Skip tab switching tests on Android webview since it doesn't have tabs
[ android android-webview-instrumentation ] Pixel_Canvas2DTabSwitch [ Skip ]
[ android android-webview-instrumentation ] Pixel_WebGLReadPixelsTabSwitch [ Skip ]
# Tests running with SwiftShader are skipped on platforms where SwiftShader
# isn't supported.
......
third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc
View file @ af59acd7
......@@ -397,6 +397,7 @@ bool DrawingBuffer::FinishPrepareTransferableResourceSoftware(
WTF::Passed(std::move(registered)));
*out_release_callback = viz::SingleReleaseCallback::Create(std::move(func));
contents_changed_ = false;
ResetBuffersToAutoClear();
return true;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment