Commit a28f6b88 authored by Brandon Jones's avatar Brandon Jones Committed by Commit Bot

Fix WebXR framebuffer discard/clear logic.

In cases when we explicitly resolved multisampled framebuffers the
resolved framebuffer was being discarded instead of the drawing
framebuffer. This likely had no ill effects but was not giving us the
benefit expected on those devices.

Additionally, framebuffers were not clearing prior to each new frame
(although discard would sometimes appear to have that effect), which
would result in data from previous frames being left behind in the
framebuffer in an unpredictable manner. This patch introduces a
consistent clear for predictability.

Bug: 863242
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib78c7d02812574a655220c6bdba50e02904dd483
Reviewed-on: https://chromium-review.googlesource.com/1135943Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Reviewed-by: default avatarKlaus Weidner <klausw@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575297}
parent 9c2b7814
...@@ -288,12 +288,13 @@ void XRWebGLDrawingBuffer::UseSharedBuffer( ...@@ -288,12 +288,13 @@ void XRWebGLDrawingBuffer::UseSharedBuffer(
framebuffer_complete_checked_for_sharedbuffer_ = true; framebuffer_complete_checked_for_sharedbuffer_ = true;
} }
if (discard_framebuffer_supported_) { if (WantExplicitResolve()) {
const GLenum kAttachments[3] = {GL_COLOR_ATTACHMENT0, GL_DEPTH_ATTACHMENT, // Bind the drawing framebuffer if it wasn't bound previously.
GL_STENCIL_ATTACHMENT}; gl->BindFramebuffer(GL_FRAMEBUFFER, framebuffer_);
gl->DiscardFramebufferEXT(GL_FRAMEBUFFER, 3, kAttachments);
} }
ClearBoundFramebuffer();
DrawingBuffer::Client* client = drawing_buffer_->client(); DrawingBuffer::Client* client = drawing_buffer_->client();
if (!client) if (!client)
return; return;
...@@ -306,19 +307,14 @@ void XRWebGLDrawingBuffer::DoneWithSharedBuffer() { ...@@ -306,19 +307,14 @@ void XRWebGLDrawingBuffer::DoneWithSharedBuffer() {
BindAndResolveDestinationFramebuffer(); BindAndResolveDestinationFramebuffer();
gpu::gles2::GLES2Interface* gl = drawing_buffer_->ContextGL(); gpu::gles2::GLES2Interface* gl = drawing_buffer_->ContextGL();
// Discard the depth and stencil attachments since we're done with them.
// Don't discard the color buffer, we do need this rendered into the
// shared buffer.
if (discard_framebuffer_supported_) { if (discard_framebuffer_supported_) {
// Discard the depth and stencil attachments since we're done with them. const GLenum kAttachments[3] = {GL_DEPTH_ATTACHMENT, GL_STENCIL_ATTACHMENT};
// Don't discard the color buffer, we do need this rendered into the gl->BindFramebuffer(GL_FRAMEBUFFER, framebuffer_);
// shared buffer. gl->DiscardFramebufferEXT(GL_FRAMEBUFFER, 2, kAttachments);
if (WantExplicitResolve()) {
gl->BindFramebuffer(GL_FRAMEBUFFER, resolved_framebuffer_);
} else {
gl->BindFramebuffer(GL_FRAMEBUFFER, framebuffer_);
}
const GLenum kAttachments[] = {GL_DEPTH_ATTACHMENT, GL_STENCIL_ATTACHMENT};
gl->DiscardFramebufferEXT(GL_FRAMEBUFFER,
sizeof(kAttachments) / sizeof(kAttachments[0]),
kAttachments);
} }
// Always bind to the default framebuffer as a hint to the GPU to start // Always bind to the default framebuffer as a hint to the GPU to start
...@@ -336,6 +332,37 @@ void XRWebGLDrawingBuffer::DoneWithSharedBuffer() { ...@@ -336,6 +332,37 @@ void XRWebGLDrawingBuffer::DoneWithSharedBuffer() {
client->DrawingBufferClientRestoreFramebufferBinding(); client->DrawingBufferClientRestoreFramebufferBinding();
} }
void XRWebGLDrawingBuffer::ClearBoundFramebuffer() {
gpu::gles2::GLES2Interface* gl = drawing_buffer_->ContextGL();
GLbitfield clear_bits = GL_COLOR_BUFFER_BIT;
gl->ColorMask(true, true, true, true);
gl->ClearColor(0.0f, 0.0f, 0.0f, 0.0f);
if (depth_) {
clear_bits |= GL_DEPTH_BUFFER_BIT;
gl->DepthMask(true);
gl->ClearDepthf(1.0f);
}
if (stencil_) {
clear_bits |= GL_STENCIL_BUFFER_BIT;
gl->StencilMaskSeparate(GL_FRONT, true);
gl->ClearStencil(0);
}
gl->Disable(GL_SCISSOR_TEST);
gl->Clear(clear_bits);
DrawingBuffer::Client* client = drawing_buffer_->client();
if (!client)
return;
client->DrawingBufferClientRestoreScissorTest();
client->DrawingBufferClientRestoreMaskAndClearValues();
}
void XRWebGLDrawingBuffer::Resize(const IntSize& new_size) { void XRWebGLDrawingBuffer::Resize(const IntSize& new_size) {
IntSize adjusted_size = AdjustSize(new_size); IntSize adjusted_size = AdjustSize(new_size);
...@@ -545,12 +572,13 @@ void XRWebGLDrawingBuffer::SwapColorBuffers() { ...@@ -545,12 +572,13 @@ void XRWebGLDrawingBuffer::SwapColorBuffers() {
framebuffer_complete_checked_for_swap_ = true; framebuffer_complete_checked_for_swap_ = true;
} }
if (discard_framebuffer_supported_) { if (WantExplicitResolve()) {
const GLenum kAttachments[3] = {GL_COLOR_ATTACHMENT0, GL_DEPTH_ATTACHMENT, // Bind the drawing framebuffer if it wasn't bound previously.
GL_STENCIL_ATTACHMENT}; gl->BindFramebuffer(GL_FRAMEBUFFER, framebuffer_);
gl->DiscardFramebufferEXT(GL_FRAMEBUFFER, 3, kAttachments);
} }
ClearBoundFramebuffer();
client->DrawingBufferClientRestoreFramebufferBinding(); client->DrawingBufferClientRestoreFramebufferBinding();
} }
......
...@@ -113,6 +113,8 @@ class PLATFORM_EXPORT XRWebGLDrawingBuffer ...@@ -113,6 +113,8 @@ class PLATFORM_EXPORT XRWebGLDrawingBuffer
void BindAndResolveDestinationFramebuffer(); void BindAndResolveDestinationFramebuffer();
void SwapColorBuffers(); void SwapColorBuffers();
void ClearBoundFramebuffer();
void MailboxReleased(scoped_refptr<ColorBuffer>, void MailboxReleased(scoped_refptr<ColorBuffer>,
const gpu::SyncToken&, const gpu::SyncToken&,
bool lost_resource); bool lost_resource);
......
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