Commit b242637d authored by Brian Ho's avatar Brian Ho Committed by Chromium LUCI CQ

viz: Wait for fullscreen damage after reallocating buffers

A page flip can result in a SWAP_NAK_RECREATE_BUFFERS signal,
indicating that the backing buffers must be recreated. Since the
buffers are entirely uninitialized, we should damage the entire
screen for the next draw [1].

Unfortunately, this doesn't work in SkiaRenderer because the GPU
thread performs the actual reallocation [2] and then posts a task to
the Viz thread to update the damage. The task taces with the next
invocation of DirectRenderer::DrawFrame, so there's no guarantee
that the new damage will be picked up in time for the next frame.
If it isn't, we will present a buffer with uninitialized data.

This CL fixes this issue by pausing SwapBuffer invocations after
reallocating buffers until we receive a frame with full screen
damage (e.g. the Viz thread catches up with the GPU thread).

[1] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/gl_output_surface_buffer_queue.cc;l=263;drc=3c0194e7917daed5debe0d0772393e5784367e01
[2] https://source.chromium.org/chromium/chromium/src/+/master:components/viz/service/display_embedder/skia_output_device_buffer_queue.cc;l=458;drc=7ccffaf0933ccc647c744bf66971bcf5f33a676a

Bug: 1156182
Change-Id: Ibf0bf3d6903c4c4f2dadcfc71dfda36a03b513d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630227Reviewed-by: default avatarVasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Brian Ho <hob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844167}
parent eff385e1
......@@ -901,6 +901,7 @@ void SkiaOutputSurfaceImpl::DidSwapBuffersComplete(
// Reset |damage_of_buffers_|, if buffers are new created.
if (params.swap_response.result ==
gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
client_->SetNeedsRedrawRect(gfx::Rect(size_));
for (auto& damage : damage_of_buffers_)
damage = gfx::Rect(size_);
}
......
......@@ -1450,6 +1450,20 @@ void SkiaOutputSurfaceImplOnGpu::PostSubmit(
scoped_output_device_paint_.reset();
if (frame) {
if (waiting_for_full_damage_) {
// If we're using partial swap, we need to check whether the sub-buffer
// rect is actually the entire screen, but otherwise, the damage is
// always the full surface.
if (frame->sub_buffer_rect &&
capabilities().supports_post_sub_buffer &&
frame->sub_buffer_rect->size() != size_) {
output_surface_plane_.reset();
destroy_after_swap_.clear();
return;
}
waiting_for_full_damage_ = false;
}
if (output_surface_plane_)
DCHECK(output_device_->IsPrimaryPlaneOverlay());
output_device_->SchedulePrimaryPlane(output_surface_plane_);
......@@ -1540,6 +1554,16 @@ void SkiaOutputSurfaceImplOnGpu::DidSwapBuffersCompleteInternal(
// Mark the context lost if not already lost.
MarkContextLost(ContextLostReason::CONTEXT_LOST_SWAP_FAILED);
}
} else if (params.swap_response.result ==
gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
// We shouldn't present newly reallocated buffers until we have fully
// initialized their contents. SWAP_NAK_RECREAETE_BUFFERS should trigger a
// full-screen damage in DirectRenderer, but there is no guarantee that it
// will happen immediately since the SwapBuffersComplete task gets posted
// back to the Viz thread and will race with the next invocation of
// DrawFrame. To ensure we do not display uninitialized memory, we hold
// off on submitting new frames until we have received a full damage.
waiting_for_full_damage_ = true;
}
#if defined(OS_APPLE)
......
......@@ -343,6 +343,8 @@ class SkiaOutputSurfaceImplOnGpu
// Micro-optimization to get to issuing GPU SwapBuffers as soon as possible.
std::vector<sk_sp<SkDeferredDisplayList>> destroy_after_swap_;
bool waiting_for_full_damage_ = false;
int num_readbacks_pending_ = 0;
bool readback_poll_pending_ = false;
......
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