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 <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

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));