Fix BlitFramebuffer validation for BGRA sources and targets.

src/libANGLE/formatutils.cpp

@@ -458,6 +458,24 @@ bool Format::SameSized(const Format &a, const Format &b)
     return a.info->sizedInternalFormat == b.info->sizedInternalFormat;
 }
 
+static GLenum EquivalentBlitInternalFormat(GLenum internalformat)
+{
+    // BlitFramebuffer works if the color channels are identically
+    // sized, even if there is a swizzle (for example, blitting from a
+    // multisampled RGBA8 renderbuffer to a BGRA8 texture). This could
+    // be expanded and/or autogenerated if that is found necessary.
+    if (internalformat == GL_BGRA8_EXT)
+        return GL_RGBA8;
+    return internalformat;
+}
+
+// static
+bool Format::EquivalentForBlit(const Format &a, const Format &b)
+{
+    return (EquivalentBlitInternalFormat(a.info->sizedInternalFormat) ==
+            EquivalentBlitInternalFormat(b.info->sizedInternalFormat));
+}
+
 // static
 Format Format::Invalid()
 {

src/libANGLE/formatutils.h

@@ -150,6 +150,7 @@ struct Format
 
     static Format Invalid();
     static bool SameSized(const Format &a, const Format &b);
+    static bool EquivalentForBlit(const Format &a, const Format &b);
 
     friend std::ostream &operator<<(std::ostream &os, const Format &fmt);
 

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -133,6 +133,10 @@ struct WorkaroundsGL
     // On some NVIDIA drivers the point size range reported from the API is inconsistent with the
     // actual behavior. Clamp the point size to the value from the API to fix this.
     bool clampPointSize = false;
+
+    // Calling getInternalformativ for GL_NUM_SAMPLES and the internal
+    // format GL_BGRA8_EXT generates a GL_INVALID_ENUM error on macOS.
+    bool avoidInternalFormativForBGRA8 = false;
 };
 }  // namespace rx
 

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -89,8 +89,12 @@ static bool MeetsRequirements(const FunctionsGL *functions, const nativegl::Supp
     }
 }
 
-static gl::TextureCaps GenerateTextureFormatCaps(const FunctionsGL *functions, GLenum internalFormat)
+static gl::TextureCaps GenerateTextureFormatCaps(const FunctionsGL *functions,
+                                                 const WorkaroundsGL &workarounds,
+                                                 GLenum internalFormat)
 {
+    ASSERT(functions->getError() == GL_NO_ERROR);
+
     gl::TextureCaps textureCaps;
 
     const nativegl::InternalFormat &formatInfo = nativegl::GetInternalFormatInfo(internalFormat, functions->standard);
@@ -102,16 +106,27 @@ static gl::TextureCaps GenerateTextureFormatCaps(const FunctionsGL *functions, G
     // extension GL_ARB_internalformat_query
     if (textureCaps.renderable && functions->getInternalformativ)
     {
+        GLenum queryInternalFormat = internalFormat;
+
+        if (internalFormat == GL_BGRA8_EXT && workarounds.avoidInternalFormativForBGRA8)
+        {
+            // Querying GL_NUM_SAMPLE_COUNTS for GL_BGRA8_EXT generates an INVALID_ENUM error on
+            // macOS' Core Profile OpenGL driver. It seems however that allocating a multisampled
+            // renderbuffer of this format succeeds. To avoid breaking multisampling for this
+            // format, query the supported sample counts for GL_RGBA8 instead.
+            queryInternalFormat = GL_RGBA8;
+        }
+
         GLint numSamples = 0;
-        functions->getInternalformativ(GL_RENDERBUFFER, internalFormat, GL_NUM_SAMPLE_COUNTS, 1, &numSamples);
+        functions->getInternalformativ(GL_RENDERBUFFER, queryInternalFormat, GL_NUM_SAMPLE_COUNTS,
+                                       1, &numSamples);
 
         if (numSamples > 0)
         {
             std::vector<GLint> samples(numSamples);
-            functions->getInternalformativ(GL_RENDERBUFFER, internalFormat, GL_SAMPLES,
+            functions->getInternalformativ(GL_RENDERBUFFER, queryInternalFormat, GL_SAMPLES,
                                            static_cast<GLsizei>(samples.size()), &samples[0]);
 
-            GLenum queryInternalFormat = internalFormat;
             if (internalFormat == GL_STENCIL_INDEX8)
             {
                 // The query below does generates an error with STENCIL_INDEX8 on NVIDIA driver
@@ -149,6 +164,7 @@ static gl::TextureCaps GenerateTextureFormatCaps(const FunctionsGL *functions, G
         }
     }
 
+    ASSERT(functions->getError() == GL_NO_ERROR);
     return textureCaps;
 }
 
@@ -229,7 +245,8 @@ void GenerateCaps(const FunctionsGL *functions,
     const gl::FormatSet &allFormats = gl::GetAllSizedInternalFormats();
     for (GLenum internalFormat : allFormats)
     {
-        gl::TextureCaps textureCaps = GenerateTextureFormatCaps(functions, internalFormat);
+        gl::TextureCaps textureCaps =
+            GenerateTextureFormatCaps(functions, workarounds, internalFormat);
         textureCapsMap->insert(internalFormat, textureCaps);
 
         if (gl::GetSizedInternalFormatInfo(internalFormat).compressed)
@@ -539,9 +556,14 @@ void GenerateCaps(const FunctionsGL *functions,
         LimitVersion(maxSupportedESVersion, gl::Version(2, 0));
     }
 
-    if (functions->isAtLeastGL(gl::Version(3, 0)) ||
-        functions->hasGLExtension("GL_ARB_ES2_compatibility") ||
-        functions->isAtLeastGLES(gl::Version(2, 0)))
+    if (functions->isAtLeastGL(gl::Version(3, 2)) &&
+        (functions->profile & GL_CONTEXT_CORE_PROFILE_BIT) != 0)
+    {
+        caps->maxVaryingComponents = QuerySingleGLInt(functions, GL_MAX_VERTEX_OUTPUT_COMPONENTS);
+    }
+    else if (functions->isAtLeastGL(gl::Version(3, 0)) ||
+             functions->hasGLExtension("GL_ARB_ES2_compatibility") ||
+             functions->isAtLeastGLES(gl::Version(2, 0)))
     {
         caps->maxVaryingComponents = QuerySingleGLInt(functions, GL_MAX_VARYING_COMPONENTS);
     }
@@ -1061,6 +1083,12 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
 
     workarounds->clampPointSize = true;
 #endif
+
+#if defined(ANGLE_PLATFORM_APPLE)
+    workarounds->avoidInternalFormativForBGRA8 = true;
+#else
+    workarounds->avoidInternalFormativForBGRA8 = IsAMD(vendor);
+#endif
 }
 
 void ApplyWorkarounds(const FunctionsGL *functions, gl::Workarounds *workarounds)

src/libANGLE/validationES.cpp

@@ -1727,7 +1727,7 @@ bool ValidateBlitFramebufferParameters(ValidationContext *context,
                     }
 
                     if (readColorBuffer->getSamples() > 0 &&
-                        (!Format::SameSized(readFormat, drawFormat) || !sameBounds))
+                        (!Format::EquivalentForBlit(readFormat, drawFormat) || !sameBounds))
                     {
                         context->handleError(InvalidOperation());
                         return false;
@@ -1778,7 +1778,7 @@ bool ValidateBlitFramebufferParameters(ValidationContext *context,
 
             if (readBuffer && drawBuffer)
             {
-                if (!Format::SameSized(readBuffer->getFormat(), drawBuffer->getFormat()))
+                if (!Format::EquivalentForBlit(readBuffer->getFormat(), drawBuffer->getFormat()))
                 {
                     context->handleError(InvalidOperation());
                     return false;

src/libANGLE/validationES2.cpp

@@ -2350,8 +2350,8 @@ bool ValidateBlitFramebufferANGLE(Context *context,
                     }
 
                     // Return an error if the destination formats do not match
-                    if (!Format::SameSized(attachment->getFormat(),
-                                           readColorAttachment->getFormat()))
+                    if (!Format::EquivalentForBlit(attachment->getFormat(),
+                                                   readColorAttachment->getFormat()))
                     {
                         context->handleError(InvalidOperation());
                         return false;

src/tests/gl_tests/BlitFramebufferANGLETest.cpp

@@ -52,7 +52,11 @@ class BlitFramebufferANGLETest : public ANGLETest
 
         mRGBAColorbuffer = 0;
         mRGBAFBO = 0;
+        mRGBAMultisampledRenderbuffer = 0;
+        mRGBAMultisampledFBO          = 0;
 
+        mBGRAColorbuffer              = 0;
+        mBGRAFBO                      = 0;
         mBGRAMultisampledRenderbuffer = 0;
         mBGRAMultisampledFBO = 0;
     }
@@ -205,9 +209,10 @@ class BlitFramebufferANGLETest : public ANGLETest
             ASSERT_GL_NO_ERROR();
         }
 
-        if (extensionEnabled("GL_ANGLE_framebuffer_multisample"))
+        if (extensionEnabled("GL_ANGLE_framebuffer_multisample") &&
+            extensionEnabled("GL_OES_rgb8_rgba8"))
         {
-            // Test blit between RGBA and multisampled BGRA
+            // RGBA single-sampled framebuffer
             glGenTextures(1, &mRGBAColorbuffer);
             glBindTexture(GL_TEXTURE_2D, mRGBAColorbuffer);
             glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, getWindowWidth(), getWindowHeight(), 0, GL_RGBA,
@@ -220,16 +225,50 @@ class BlitFramebufferANGLETest : public ANGLETest
             ASSERT_GL_NO_ERROR();
             ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
 
-            glGenRenderbuffers(1, &mBGRAMultisampledRenderbuffer);
-            glBindRenderbuffer(GL_RENDERBUFFER, mBGRAMultisampledRenderbuffer);
-            glRenderbufferStorageMultisampleANGLE(GL_RENDERBUFFER, 1, GL_BGRA8_EXT, getWindowWidth(), getWindowHeight());
+            // RGBA multisampled framebuffer
+            glGenRenderbuffers(1, &mRGBAMultisampledRenderbuffer);
+            glBindRenderbuffer(GL_RENDERBUFFER, mRGBAMultisampledRenderbuffer);
+            glRenderbufferStorageMultisampleANGLE(GL_RENDERBUFFER, 1, GL_RGBA8, getWindowWidth(),
+                                                  getWindowHeight());
 
-            glGenFramebuffers(1, &mBGRAMultisampledFBO);
-            glBindFramebuffer(GL_FRAMEBUFFER, mBGRAMultisampledFBO);
-            glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, mBGRAMultisampledRenderbuffer);
+            glGenFramebuffers(1, &mRGBAMultisampledFBO);
+            glBindFramebuffer(GL_FRAMEBUFFER, mRGBAMultisampledFBO);
+            glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER,
+                                      mRGBAMultisampledRenderbuffer);
 
             ASSERT_GL_NO_ERROR();
             ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+            if (extensionEnabled("GL_EXT_texture_format_BGRA8888"))
+            {
+                // BGRA single-sampled framebuffer
+                glGenTextures(1, &mBGRAColorbuffer);
+                glBindTexture(GL_TEXTURE_2D, mBGRAColorbuffer);
+                glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, getWindowWidth(), getWindowHeight(), 0,
+                             GL_BGRA_EXT, GL_UNSIGNED_BYTE, nullptr);
+
+                glGenFramebuffers(1, &mBGRAFBO);
+                glBindFramebuffer(GL_FRAMEBUFFER, mBGRAFBO);
+                glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+                                       mBGRAColorbuffer, 0);
+
+                ASSERT_GL_NO_ERROR();
+                ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+                // BGRA multisampled framebuffer
+                glGenRenderbuffers(1, &mBGRAMultisampledRenderbuffer);
+                glBindRenderbuffer(GL_RENDERBUFFER, mBGRAMultisampledRenderbuffer);
+                glRenderbufferStorageMultisampleANGLE(GL_RENDERBUFFER, 1, GL_BGRA8_EXT,
+                                                      getWindowWidth(), getWindowHeight());
+
+                glGenFramebuffers(1, &mBGRAMultisampledFBO);
+                glBindFramebuffer(GL_FRAMEBUFFER, mBGRAMultisampledFBO);
+                glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER,
+                                          mBGRAMultisampledRenderbuffer);
+
+                ASSERT_GL_NO_ERROR();
+                ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+            }
         }
 
         glBindFramebuffer(GL_FRAMEBUFFER, mOriginalFBO);
@@ -271,7 +310,27 @@ class BlitFramebufferANGLETest : public ANGLETest
 
         if (mRGBAFBO != 0)
         {
-            glDeleteFramebuffers(1, &mBGRAMultisampledFBO);
+            glDeleteFramebuffers(1, &mRGBAFBO);
+        }
+
+        if (mRGBAMultisampledRenderbuffer != 0)
+        {
+            glDeleteRenderbuffers(1, &mRGBAMultisampledRenderbuffer);
+        }
+
+        if (mRGBAMultisampledFBO != 0)
+        {
+            glDeleteFramebuffers(1, &mRGBAMultisampledFBO);
+        }
+
+        if (mBGRAColorbuffer != 0)
+        {
+            glDeleteTextures(1, &mBGRAColorbuffer);
+        }
+
+        if (mBGRAFBO != 0)
+        {
+            glDeleteFramebuffers(1, &mBGRAFBO);
         }
 
         if (mBGRAMultisampledRenderbuffer != 0)
@@ -287,6 +346,37 @@ class BlitFramebufferANGLETest : public ANGLETest
         ANGLETest::TearDown();
     }
 
+    void multisampleTestHelper(GLuint readFramebuffer, GLuint drawFramebuffer)
+    {
+        glClearColor(0.0, 1.0, 0.0, 1.0);
+        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, readFramebuffer);
+        glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+        EXPECT_GL_NO_ERROR();
+
+        glBindFramebuffer(GL_READ_FRAMEBUFFER, readFramebuffer);
+        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFramebuffer);
+        glBlitFramebufferANGLE(0, 0, getWindowWidth(), getWindowHeight(), 0, 0, getWindowWidth(),
+                               getWindowHeight(), GL_COLOR_BUFFER_BIT, GL_NEAREST);
+        EXPECT_GL_NO_ERROR();
+
+        glBindFramebuffer(GL_READ_FRAMEBUFFER, drawFramebuffer);
+        EXPECT_PIXEL_EQ(getWindowWidth() / 4, getWindowHeight() / 4, 0, 255, 0, 255);
+        EXPECT_PIXEL_EQ(3 * getWindowWidth() / 4, getWindowHeight() / 4, 0, 255, 0, 255);
+        EXPECT_PIXEL_EQ(3 * getWindowWidth() / 4, 3 * getWindowHeight() / 4, 0, 255, 0, 255);
+        EXPECT_PIXEL_EQ(getWindowWidth() / 4, 3 * getWindowHeight() / 4, 0, 255, 0, 255);
+    }
+
+    bool checkExtension(const std::string &extension)
+    {
+        if (!extensionEnabled(extension))
+        {
+            std::cout << "Test skipped because " << extension << " not supported." << std::endl;
+            return false;
+        }
+
+        return true;
+    }
+
     GLuint mCheckerProgram;
     GLuint mBlueProgram;
 
@@ -315,7 +405,11 @@ class BlitFramebufferANGLETest : public ANGLETest
 
     GLuint mRGBAColorbuffer;
     GLuint mRGBAFBO;
+    GLuint mRGBAMultisampledRenderbuffer;
+    GLuint mRGBAMultisampledFBO;
 
+    GLuint mBGRAColorbuffer;
+    GLuint mBGRAFBO;
     GLuint mBGRAMultisampledRenderbuffer;
     GLuint mBGRAMultisampledFBO;
 };
@@ -824,6 +918,60 @@ TEST_P(BlitFramebufferANGLETest, BlitMRT)
     glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1_EXT, GL_TEXTURE_2D, mMRTColorBuffer1, 0);
 }
 
+// Test multisampled framebuffer blits if supported
+TEST_P(BlitFramebufferANGLETest, MultisampledRGBAToRGBA)
+{
+    if (!checkExtension("GL_ANGLE_framebuffer_multisample"))
+        return;
+
+    if (!checkExtension("GL_OES_rgb8_rgba8"))
+        return;
+
+    multisampleTestHelper(mRGBAMultisampledFBO, mRGBAFBO);
+}
+
+TEST_P(BlitFramebufferANGLETest, MultisampledRGBAToBGRA)
+{
+    if (!checkExtension("GL_ANGLE_framebuffer_multisample"))
+        return;
+
+    if (!checkExtension("GL_OES_rgb8_rgba8"))
+        return;
+
+    if (!checkExtension("GL_EXT_texture_format_BGRA8888"))
+        return;
+
+    multisampleTestHelper(mRGBAMultisampledFBO, mBGRAFBO);
+}
+
+TEST_P(BlitFramebufferANGLETest, MultisampledBGRAToRGBA)
+{
+    if (!checkExtension("GL_ANGLE_framebuffer_multisample"))
+        return;
+
+    if (!checkExtension("GL_OES_rgb8_rgba8"))
+        return;
+
+    if (!checkExtension("GL_EXT_texture_format_BGRA8888"))
+        return;
+
+    multisampleTestHelper(mBGRAMultisampledFBO, mRGBAFBO);
+}
+
+TEST_P(BlitFramebufferANGLETest, MultisampledBGRAToBGRA)
+{
+    if (!checkExtension("GL_ANGLE_framebuffer_multisample"))
+        return;
+
+    if (!checkExtension("GL_OES_rgb8_rgba8"))
+        return;
+
+    if (!checkExtension("GL_EXT_texture_format_BGRA8888"))
+        return;
+
+    multisampleTestHelper(mBGRAMultisampledFBO, mBGRAFBO);
+}
+
 // Make sure that attempts to stretch in a blit call issue an error
 TEST_P(BlitFramebufferANGLETest, ErrorStretching)
 {
@@ -888,18 +1036,6 @@ TEST_P(BlitFramebufferANGLETest, Errors)
     glBlitFramebufferANGLE(0, 0, getWindowWidth(), getWindowHeight(), 0, 0, getWindowWidth(), getWindowHeight(),
                            GL_COLOR_BUFFER_BIT, GL_NEAREST);
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
-
-    if (extensionEnabled("GL_ANGLE_framebuffer_multisample"))
-    {
-        glBindFramebuffer(GL_READ_FRAMEBUFFER, mBGRAMultisampledFBO);
-        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, mRGBAFBO);
-        EXPECT_GL_NO_ERROR();
-
-        glBlitFramebufferANGLE(0, 0, getWindowWidth(), getWindowHeight(), 0, 0, getWindowWidth(), getWindowHeight(),
-                               GL_COLOR_BUFFER_BIT, GL_NEAREST);
-        EXPECT_GL_ERROR(GL_INVALID_OPERATION);
-    }
-
 }
 
 // TODO(geofflang): Fix the dependence on glBlitFramebufferANGLE without checks and assuming the
@@ -1070,6 +1206,8 @@ TEST_P(BlitFramebufferTest, MultisampleStencil)
 ANGLE_INSTANTIATE_TEST(BlitFramebufferANGLETest,
                        ES2_D3D9(),
                        ES2_D3D11(EGL_EXPERIMENTAL_PRESENT_PATH_COPY_ANGLE),
-                       ES2_D3D11(EGL_EXPERIMENTAL_PRESENT_PATH_FAST_ANGLE));
+                       ES2_D3D11(EGL_EXPERIMENTAL_PRESENT_PATH_FAST_ANGLE),
+                       ES2_OPENGL(),
+                       ES3_OPENGL());
 
 ANGLE_INSTANTIATE_TEST(BlitFramebufferTest, ES3_D3D11());