Commit 844bebe5 authored by Sunny Sachanandani's avatar Sunny Sachanandani Committed by Commit Bot

webgl: Use correct format for exported low latency canvas resource.

SkiaRenderer is strict about ensuring that the resource format passed in
to the display compositor via TransferableResource, which is used to
create the GrSurfaceProxy, matches the underlying SharedImageBacking's
format which is the GrSurfaceBackend.

In the context of DrawingBuffer this means that for low latency mode,
the ExternalCanvasResource must have the same format as the shared image
or swap chain. One problematic case is RGBA8 vs BGRA8 because the
default CanvasColorParams() used when creating the resource uses the
platform specific default for Skia, which is based on endianness, to
determine the format passed to the display compositor.

To solve this problem, this CL adds the shared image resource format to
ColorBuffer, and makes it the source of truth for all places that need
the format in DrawingBuffer. In particular, the CanvasColorParams is
constructed to match the underlying resource format instead of relying
on the defaults.

Bug: 1122638
Change-Id: I8c4e93d7f08566e695a3948505f98541d55b7216
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391410
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Reviewed-by: default avatarKai Ninomiya <kainino@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805027}
parent 030f8a68
<!DOCTYPE HTML>
<html>
<head>
<title>Low Latency WebGL Canvas with Rounded Corners Test</title>
<style type="text/css">
.nomargin {
margin: 0px auto;
}
.rounded-corner {
border-radius: 20px;
}
</style>
<script>
let g_swapsBeforeAck = 15;
function waitForFinish()
{
if (g_swapsBeforeAck == 0) {
domAutomationController.send("SUCCESS");
} else {
g_swapsBeforeAck--;
window.requestAnimationFrame(waitForFinish);
}
}
function main()
{
let canvas_gl = document.getElementById("c3d");
let gl = canvas_gl.getContext("webgl", {"desynchronized": true});
let width = canvas_gl.width;
let height = canvas_gl.height;
gl.enable(gl.SCISSOR_TEST);
gl.scissor(0, 0, width, height);
gl.clearColor(1, 0, 0, 1);
gl.clear(gl.COLOR_BUFFER_BIT);
gl.scissor(0, 0, width / 2, height / 2);
gl.clearColor(0, 1, 0, 1);
gl.clear(gl.COLOR_BUFFER_BIT);
gl.scissor(width / 2, height / 2, width / 2, height / 2);
gl.clearColor(0, 0, 1, 1);
gl.clear(gl.COLOR_BUFFER_BIT);
waitForFinish();
}
</script>
</head>
<body onload="main()">
<div id="container" style="position:absolute; top:0px; left:0px">
<canvas id="c3d" width="100" height="100" class="nomargin rounded-corner"></canvas>
</div>
</body>
</html>
......@@ -485,6 +485,10 @@ class PixelTestPages(object):
PixelTestPage('pixel_canvas_low_latency_2d_image_data.html',
base_name + '_CanvasLowLatency2DImageData',
test_rect=[0, 0, 200, 100]),
PixelTestPage('pixel_canvas_low_latency_webgl_rounded_corners.html',
base_name + '_CanvasLowLatencyWebGLRoundedCorners',
test_rect=[0, 0, 100, 100],
other_args={'no_overlay': True}),
]
# Only add these tests on platforms where SwiftShader is enabled.
......
......@@ -84,6 +84,7 @@ crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP9_DXVA [ Skip ]
# Fuchsia issues
crbug.com/1115673 [ fuchsia no-use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ]
crbug.com/1115673 [ fuchsia ] Pixel_CanvasLowLatencyWebGLDrawImage [ Skip ]
crbug.com/1115673 [ fuchsia ] Pixel_CanvasLowLatencyWebGLRoundedCorners [ Skip ]
crbug.com/1096746 [ fuchsia no-use-skia-dawn ] Pixel_WebGL2_BlitFramebuffer_Result_Displayed [ Skip ]
crbug.com/1096746 [ fuchsia no-use-skia-dawn ] Pixel_WebGL2_ClearBufferfv_Result_Displayed [ Skip ]
crbug.com/1096746 [ fuchsia no-use-skia-dawn ] Pixel_CanvasLowLatencyWebGLAlphaFalse [ Skip ]
......@@ -285,6 +286,7 @@ crbug.com/1021566 [ linux use-skia-dawn ] Pixel_CanvasLowLatency2DImageData [ Sk
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_CanvasLowLatencyWebGL [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_CanvasLowLatencyWebGLAlphaFalse [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_CanvasLowLatencyWebGLDrawImage [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_CanvasLowLatencyWebGLRoundedCorners [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_OffscreenCanvas2DResizeOnWorker [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_OffscreenCanvasTransferBeforeStyleResize [ Skip ]
crbug.com/1021566 [ linux use-skia-dawn ] Pixel_OffscreenCanvasWebGLPaintAfterResize [ Skip ]
......@@ -304,6 +306,7 @@ crbug.com/1021566 [ win use-skia-dawn ] Pixel_CanvasLowLatency2DImageData [ Skip
crbug.com/1021566 [ win use-skia-dawn ] Pixel_CanvasLowLatencyWebGL [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_CanvasLowLatencyWebGLAlphaFalse [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_CanvasLowLatencyWebGLDrawImage [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_CanvasLowLatencyWebGLRoundedCorners [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_OffscreenCanvas2DResizeOnWorker [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_OffscreenCanvasTransferBeforeStyleResize [ Skip ]
crbug.com/1021566 [ win use-skia-dawn ] Pixel_OffscreenCanvasWebGLPaintAfterResize [ Skip ]
......
......@@ -436,8 +436,7 @@ class TraceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
def _EvaluateSuccess_CheckSwapChainPath(self, category, event_iterator,
other_args):
"""Verified that swap chains were used for low latency canvas."""
del other_args # Unused in this particular success evaluation.
"""Verifies that swap chains are used as expected for low latency canvas."""
os_name = self.browser.platform.GetOSName()
assert os_name and os_name.lower() == 'win'
......@@ -446,6 +445,10 @@ class TraceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
self.fail('Overlay bot config can not be determined')
assert overlay_bot_config.get('direct_composition', False)
expect_no_overlay = other_args and other_args.get('no_overlay', False)
expect_overlay = not expect_no_overlay
found_overlay = False
# Verify expectations through captured trace events.
for event in event_iterator:
if event.category != category:
......@@ -454,10 +457,16 @@ class TraceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
continue
presentation_mode = event.args.get('image_type', None)
if presentation_mode == 'swap chain':
found_overlay = True
break
else:
self.fail('Events with name %s were not found' %
_PRESENT_TO_SWAP_CHAIN_EVENT_NAME)
if expect_overlay and not found_overlay:
self.fail(
'Overlay expected but not found: matching %s events were not found' %
_PRESENT_TO_SWAP_CHAIN_EVENT_NAME)
elif expect_no_overlay and found_overlay:
self.fail(
'Overlay not expected but found: matching %s events were found' %
_PRESENT_TO_SWAP_CHAIN_EVENT_NAME)
def _EvaluateSuccess_CheckMainSwapChainPath(self, category, event_iterator,
other_args):
......
......@@ -313,11 +313,19 @@ DrawingBuffer::RegisteredBitmap DrawingBuffer::CreateOrRecycleBitmap(
}
viz::SharedBitmapId id = viz::SharedBitmap::GenerateId();
// TODO(sunnyps): Software compositor only supports RGBA_8888 format, and
// AllocateSharedBitmap has a DCHECK for that, but we still allocate an F16
// buffer (of twice the size) so that there's no invalid memory access at
// runtime in ReadBackFramebuffer in release mode. Fixing this is non-trivial,
// so it will be done in a follow-up CL.
viz::ResourceFormat format = viz::RGBA_8888;
if (use_half_float_storage_)
format = viz::RGBA_F16;
base::MappedReadOnlyRegion shm = viz::bitmap_allocation::AllocateSharedBitmap(
static_cast<gfx::Size>(size_), format);
auto bitmap = base::MakeRefCounted<cc::CrossThreadSharedBitmap>(
id, std::move(shm), static_cast<gfx::Size>(size_), format);
RegisteredBitmap registered = {
......@@ -399,9 +407,15 @@ bool DrawingBuffer::FinishPrepareTransferableResourceSoftware(
op);
}
// TODO(sunnyps): Software compositor only supports RGBA_8888 format, and
// AllocateSharedBitmap has a DCHECK for that, but we still allocate an F16
// buffer (of twice the size) so that there's no invalid memory access at
// runtime in ReadBackFramebuffer in release mode. Fixing this is non-trivial,
// so it will be done in a follow-up CL.
viz::ResourceFormat format = viz::RGBA_8888;
if (use_half_float_storage_)
format = viz::RGBA_F16;
*out_resource = viz::TransferableResource::MakeSoftware(
registered.bitmap->id(), static_cast<gfx::Size>(size_), format);
out_resource->color_space = storage_color_space_;
......@@ -509,16 +523,7 @@ bool DrawingBuffer::FinishPrepareTransferableResourceGpu(
color_buffer_for_mailbox->produce_sync_token, gfx::Size(size_),
is_overlay_candidate);
out_resource->color_space = sampler_color_space_;
if (allocate_alpha_channel_) {
if (use_half_float_storage_)
out_resource->format = viz::RGBA_F16;
else
out_resource->format = viz::RGBA_8888;
} else {
DCHECK(!use_half_float_storage_);
out_resource->format = viz::RGBX_8888;
}
out_resource->format = color_buffer_for_mailbox->format;
// This holds a ref on the DrawingBuffer that will keep it alive until the
// mailbox is released (and while the release callback is running).
auto func = base::BindOnce(&DrawingBuffer::NotifyMailboxReleasedGpu,
......@@ -677,9 +682,23 @@ scoped_refptr<CanvasResource> DrawingBuffer::AsCanvasResource(
scoped_refptr<ColorBuffer> canvas_resource_buffer =
UsingSwapChain() ? front_color_buffer_ : back_color_buffer_;
CanvasColorParams color_params;
switch (canvas_resource_buffer->format) {
case viz::RGBA_8888:
case viz::RGBX_8888:
color_params.SetCanvasPixelFormat(CanvasPixelFormat::kRGBA8);
break;
case viz::RGBA_F16:
color_params.SetCanvasPixelFormat(CanvasPixelFormat::kF16);
break;
default:
NOTREACHED();
break;
}
return ExternalCanvasResource::Create(
canvas_resource_buffer->mailbox, canvas_resource_buffer->size,
texture_target_, CanvasColorParams(), context_provider_->GetWeakPtr(),
texture_target_, color_params, context_provider_->GetWeakPtr(),
resource_provider, kLow_SkFilterQuality,
/*is_origin_top_left=*/opengl_flip_y_extension_);
}
......@@ -687,12 +706,14 @@ scoped_refptr<CanvasResource> DrawingBuffer::AsCanvasResource(
DrawingBuffer::ColorBuffer::ColorBuffer(
base::WeakPtr<DrawingBuffer> drawing_buffer,
const IntSize& size,
viz::ResourceFormat format,
GLuint texture_id,
std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer,
gpu::Mailbox mailbox)
: owning_thread_ref(base::PlatformThread::CurrentRef()),
drawing_buffer(std::move(drawing_buffer)),
size(size),
format(format),
texture_id(texture_id),
gpu_memory_buffer(std::move(gpu_memory_buffer)),
mailbox(mailbox) {}
......@@ -1092,14 +1113,9 @@ bool DrawingBuffer::ResizeDefaultFramebuffer(const IntSize& size) {
premultiplied_alpha_false_mailbox_.SetZero();
premultiplied_alpha_false_texture_ = 0;
}
viz::ResourceFormat format;
if (use_half_float_storage_)
format = viz::RGBA_F16;
else
format = viz::RGBA_8888;
premultiplied_alpha_false_mailbox_ = sii->CreateSharedImage(
format, static_cast<gfx::Size>(size), storage_color_space_,
kTopLeft_GrSurfaceOrigin, kUnpremul_SkAlphaType,
back_color_buffer_->format, static_cast<gfx::Size>(size),
storage_color_space_, kTopLeft_GrSurfaceOrigin, kUnpremul_SkAlphaType,
gpu::SHARED_IMAGE_USAGE_GLES2 |
gpu::SHARED_IMAGE_USAGE_GLES2_FRAMEBUFFER_HINT |
gpu::SHARED_IMAGE_USAGE_RASTER,
......@@ -1675,7 +1691,7 @@ scoped_refptr<DrawingBuffer::ColorBuffer> DrawingBuffer::CreateColorBuffer(
texture_id = gl_->CreateAndTexStorage2DSharedImageCHROMIUM(
front_buffer_mailbox.name);
front_color_buffer_ = base::MakeRefCounted<ColorBuffer>(
weak_factory_.GetWeakPtr(), size, texture_id, nullptr,
weak_factory_.GetWeakPtr(), size, format, texture_id, nullptr,
front_buffer_mailbox);
}
// Import the backbuffer of swap chain or allocated SharedImage into GL.
......@@ -1704,7 +1720,7 @@ scoped_refptr<DrawingBuffer::ColorBuffer> DrawingBuffer::CreateColorBuffer(
}
return base::MakeRefCounted<ColorBuffer>(
weak_factory_.GetWeakPtr(), size, texture_id,
weak_factory_.GetWeakPtr(), size, format, texture_id,
std::move(gpu_memory_buffer), back_buffer_mailbox);
}
......
......@@ -361,6 +361,7 @@ class PLATFORM_EXPORT DrawingBuffer : public cc::TextureLayerClient,
struct ColorBuffer : public base::RefCountedThreadSafe<ColorBuffer> {
ColorBuffer(base::WeakPtr<DrawingBuffer> drawing_buffer,
const IntSize&,
viz::ResourceFormat,
GLuint texture_id,
std::unique_ptr<gfx::GpuMemoryBuffer>,
gpu::Mailbox mailbox);
......@@ -375,6 +376,7 @@ class PLATFORM_EXPORT DrawingBuffer : public cc::TextureLayerClient,
// ColorBuffers.
base::WeakPtr<DrawingBuffer> drawing_buffer;
const IntSize size;
const viz::ResourceFormat format;
const GLuint texture_id = 0;
std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer;
......
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