From 6898b35826476b9c841897b34190ef448a897e8e Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 10 Nov 2016 11:41:15 -0500 Subject: [PATCH] FramebufferGL: Fix blit workaround corner case The SRGB blit workaround had to wrong assumptions: - SRGB blits can have a multisample source. - The woarkound is needed even when the filter is GL_LINEAR in the case where we are doing a RGB -> SRGB or RGB -> SRGB blit. BUG=angleproject:1492 BUG=chromium:658898 Change-Id: I1d89572565a4e23c1c97bdf985bb21a445e898b7 Reviewed-on: https://chromium-review.googlesource.com/409540 Reviewed-by: Jamie Madill Reviewed-by: Geoff Lang Commit-Queue: Corentin Wallez --- src/libANGLE/renderer/gl/FramebufferGL.cpp | 77 +++++++++++----------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/src/libANGLE/renderer/gl/FramebufferGL.cpp b/src/libANGLE/renderer/gl/FramebufferGL.cpp index e66905d5d..7e300b6ea 100644 --- a/src/libANGLE/renderer/gl/FramebufferGL.cpp +++ b/src/libANGLE/renderer/gl/FramebufferGL.cpp @@ -283,59 +283,58 @@ Error FramebufferGL::blit(ContextImpl *context, const Framebuffer *sourceFramebuffer = context->getGLState().getReadFramebuffer(); const Framebuffer *destFramebuffer = context->getGLState().getDrawFramebuffer(); + const FramebufferAttachment *colorReadAttachment = sourceFramebuffer->getReadColorbuffer(); + GLsizei readAttachmentSamples = colorReadAttachment->getSamples(); + bool needManualColorBlit = false; - // The manual SRGB blit is only needed to perform correct linear interpolation. We don't - // need to make sure there is SRGB conversion for NEAREST as the values will be copied. - if (filter != GL_NEAREST) + // TODO(cwallez) when the filter is LINEAR and both source and destination are SRGB, we + // could avoid doing a manual blit. + + // Prior to OpenGL 4.4 BlitFramebuffer (section 18.3.1 of GL 4.3 core profile) reads: + // When values are taken from the read buffer, no linearization is performed, even + // if the format of the buffer is SRGB. + // Starting from OpenGL 4.4 (section 18.3.1) it reads: + // When values are taken from the read buffer, if FRAMEBUFFER_SRGB is enabled and the + // value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer attachment + // corresponding to the read buffer is SRGB, the red, green, and blue components are + // converted from the non-linear sRGB color space according [...]. { + bool sourceSRGB = colorReadAttachment != nullptr && + colorReadAttachment->getColorEncoding() == GL_SRGB; + needManualColorBlit = + needManualColorBlit || (sourceSRGB && mFunctions->isAtMostGL(gl::Version(4, 3))); + } - // Prior to OpenGL 4.4 BlitFramebuffer (section 18.3.1 of GL 4.3 core profile) reads: - // When values are taken from the read buffer, no linearization is performed, even - // if the format of the buffer is SRGB. - // Starting from OpenGL 4.4 (section 18.3.1) it reads: - // When values are taken from the read buffer, if FRAMEBUFFER_SRGB is enabled and the - // value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer attachment - // corresponding to the read buffer is SRGB, the red, green, and blue components are - // converted from the non-linear sRGB color space according [...]. + // Prior to OpenGL 4.2 BlitFramebuffer (section 4.3.2 of GL 4.1 core profile) reads: + // Blit operations bypass the fragment pipeline. The only fragment operations which + // affect a blit are the pixel ownership test and scissor test. + // Starting from OpenGL 4.2 (section 4.3.2) it reads: + // When values are written to the draw buffers, blit operations bypass the fragment + // pipeline. The only fragment operations which affect a blit are the pixel ownership + // test, the scissor test and sRGB conversion. + if (!needManualColorBlit) + { + bool destSRGB = false; + for (size_t i = 0; i < destFramebuffer->getDrawbufferStateCount(); ++i) { - const FramebufferAttachment *readAttachment = sourceFramebuffer->getReadColorbuffer(); - bool sourceSRGB = - readAttachment != nullptr && readAttachment->getColorEncoding() == GL_SRGB; - needManualColorBlit = - needManualColorBlit || (sourceSRGB && mFunctions->isAtMostGL(gl::Version(4, 3))); - } - - // Prior to OpenGL 4.2 BlitFramebuffer (section 4.3.2 of GL 4.1 core profile) reads: - // Blit operations bypass the fragment pipeline. The only fragment operations which - // affect a blit are the pixel ownership test and scissor test. - // Starting from OpenGL 4.2 (section 4.3.2) it reads: - // When values are written to the draw buffers, blit operations bypass the fragment - // pipeline. The only fragment operations which affect a blit are the pixel ownership - // test, the scissor test and sRGB conversion. - if (!needManualColorBlit) - { - bool destSRGB = false; - for (size_t i = 0; i < destFramebuffer->getDrawbufferStateCount(); ++i) + const FramebufferAttachment *attachment = destFramebuffer->getDrawBuffer(i); + if (attachment && attachment->getColorEncoding() == GL_SRGB) { - const FramebufferAttachment *attachment = destFramebuffer->getDrawBuffer(i); - if (attachment && attachment->getColorEncoding() == GL_SRGB) - { - destSRGB = true; - break; - } + destSRGB = true; + break; } - - needManualColorBlit = - needManualColorBlit || (destSRGB && mFunctions->isAtMostGL(gl::Version(4, 1))); } + + needManualColorBlit = + needManualColorBlit || (destSRGB && mFunctions->isAtMostGL(gl::Version(4, 1))); } // Enable FRAMEBUFFER_SRGB if needed mStateManager->setFramebufferSRGBEnabledForFramebuffer(true, this); GLenum blitMask = mask; - if (needManualColorBlit && (mask & GL_COLOR_BUFFER_BIT)) + if (needManualColorBlit && (mask & GL_COLOR_BUFFER_BIT) && readAttachmentSamples <= 1) { ANGLE_TRY(mBlitter->blitColorBufferWithShader(sourceFramebuffer, destFramebuffer, sourceArea, destArea, filter));