Explicitly enable framebuffer SRGB blending in StateManagerGL.

src/libANGLE/renderer/gl/FramebufferGL.cpp

@@ -18,14 +18,20 @@
 #include "libANGLE/renderer/gl/RenderbufferGL.h"
 #include "libANGLE/renderer/gl/StateManagerGL.h"
 #include "libANGLE/renderer/gl/TextureGL.h"
+#include "libANGLE/renderer/gl/WorkaroundsGL.h"
 
 namespace rx
 {
 
-FramebufferGL::FramebufferGL(const gl::Framebuffer::Data &data, const FunctionsGL *functions, StateManagerGL *stateManager, bool isDefault)
+FramebufferGL::FramebufferGL(const gl::Framebuffer::Data &data,
+                             const FunctionsGL *functions,
+                             StateManagerGL *stateManager,
+                             const WorkaroundsGL &workarounds,
+                             bool isDefault)
     : FramebufferImpl(data),
       mFunctions(functions),
       mStateManager(stateManager),
+      mWorkarounds(workarounds),
       mFramebufferID(0),
       mIsDefault(isDefault)
 {
@@ -38,10 +44,12 @@ FramebufferGL::FramebufferGL(const gl::Framebuffer::Data &data, const FunctionsG
 FramebufferGL::FramebufferGL(GLuint id,
                              const gl::Framebuffer::Data &data,
                              const FunctionsGL *functions,
+                             const WorkaroundsGL &workarounds,
                              StateManagerGL *stateManager)
     : FramebufferImpl(data),
       mFunctions(functions),
       mStateManager(stateManager),
+      mWorkarounds(workarounds),
       mFramebufferID(id),
       mIsDefault(true)
 {
@@ -197,6 +205,7 @@ gl::Error FramebufferGL::invalidateSub(size_t count, const GLenum *attachments,
 
 gl::Error FramebufferGL::clear(const gl::Data &data, GLbitfield mask)
 {
+    syncClearState(mask);
     mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
     mFunctions->clear(mask);
 
@@ -205,6 +214,7 @@ gl::Error FramebufferGL::clear(const gl::Data &data, GLbitfield mask)
 
 gl::Error FramebufferGL::clearBufferfv(const gl::State &state, GLenum buffer, GLint drawbuffer, const GLfloat *values)
 {
+    syncClearBufferState(buffer, drawbuffer);
     mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
     mFunctions->clearBufferfv(buffer, drawbuffer, values);
 
@@ -213,6 +223,7 @@ gl::Error FramebufferGL::clearBufferfv(const gl::State &state, GLenum buffer, GL
 
 gl::Error FramebufferGL::clearBufferuiv(const gl::State &state, GLenum buffer, GLint drawbuffer, const GLuint *values)
 {
+    syncClearBufferState(buffer, drawbuffer);
     mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
     mFunctions->clearBufferuiv(buffer, drawbuffer, values);
 
@@ -221,6 +232,7 @@ gl::Error FramebufferGL::clearBufferuiv(const gl::State &state, GLenum buffer, G
 
 gl::Error FramebufferGL::clearBufferiv(const gl::State &state, GLenum buffer, GLint drawbuffer, const GLint *values)
 {
+    syncClearBufferState(buffer, drawbuffer);
     mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
     mFunctions->clearBufferiv(buffer, drawbuffer, values);
 
@@ -229,6 +241,7 @@ gl::Error FramebufferGL::clearBufferiv(const gl::State &state, GLenum buffer, GL
 
 gl::Error FramebufferGL::clearBufferfi(const gl::State &state, GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil)
 {
+    syncClearBufferState(buffer, drawbuffer);
     mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
     mFunctions->clearBufferfi(buffer, drawbuffer, depth, stencil);
 
@@ -290,4 +303,72 @@ GLuint FramebufferGL::getFramebufferID() const
     return mFramebufferID;
 }
 
+void FramebufferGL::syncDrawState() const
+{
+    if (mFunctions->standard == STANDARD_GL_DESKTOP)
+    {
+        // Enable SRGB blending for all framebuffers except the default framebuffer on Desktop
+        // OpenGL.
+        // When SRGB blending is enabled, only SRGB capable formats will use it but the default
+        // framebuffer will always use it if it is enabled.
+        // TODO(geofflang): Update this when the framebuffer binding dirty changes, when it exists.
+        mStateManager->setFramebufferSRGBEnabled(!mIsDefault);
+    }
+}
+
+void FramebufferGL::syncClearState(GLbitfield mask)
+{
+    if (mWorkarounds.doesSRGBClearsOnLinearFramebufferAttachments &&
+        (mask & GL_COLOR_BUFFER_BIT) != 0 && !mIsDefault)
+    {
+        bool hasSRBAttachment = false;
+        for (const auto &attachment : mData.getColorAttachments())
+        {
+            if (attachment.isAttached() && attachment.getColorEncoding() == GL_SRGB)
+            {
+                hasSRBAttachment = true;
+                break;
+            }
+        }
+
+        mStateManager->setFramebufferSRGBEnabled(hasSRBAttachment);
+    }
+    else
+    {
+        mStateManager->setFramebufferSRGBEnabled(!mIsDefault);
+    }
+}
+
+void FramebufferGL::syncClearBufferState(GLenum buffer, GLint drawBuffer)
+{
+    if (mFunctions->standard == STANDARD_GL_DESKTOP)
+    {
+        if (mWorkarounds.doesSRGBClearsOnLinearFramebufferAttachments && buffer == GL_COLOR &&
+            !mIsDefault)
+        {
+            // If doing a clear on a color buffer, set SRGB blend enabled only if the color buffer
+            // is an SRGB format.
+            const auto &drawbufferState  = mData.getDrawBufferStates();
+            const auto &colorAttachments = mData.getColorAttachments();
+
+            const gl::FramebufferAttachment *attachment = nullptr;
+            if (drawbufferState[drawBuffer] >= GL_COLOR_ATTACHMENT0 &&
+                drawbufferState[drawBuffer] < GL_COLOR_ATTACHMENT0 + colorAttachments.size())
+            {
+                size_t attachmentIdx =
+                    static_cast<size_t>(drawbufferState[drawBuffer] - GL_COLOR_ATTACHMENT0);
+                attachment = &colorAttachments[attachmentIdx];
+            }
+
+            if (attachment != nullptr)
+            {
+                mStateManager->setFramebufferSRGBEnabled(attachment->getColorEncoding() == GL_SRGB);
+            }
+        }
+        else
+        {
+            mStateManager->setFramebufferSRGBEnabled(!mIsDefault);
+        }
+    }
+}
 }

src/libANGLE/renderer/gl/FramebufferGL.h

@@ -16,17 +16,23 @@ namespace rx
 
 class FunctionsGL;
 class StateManagerGL;
+struct WorkaroundsGL;
 
 class FramebufferGL : public FramebufferImpl
 {
   public:
-    FramebufferGL(const gl::Framebuffer::Data &data, const FunctionsGL *functions, StateManagerGL *stateManager, bool isDefault);
+    FramebufferGL(const gl::Framebuffer::Data &data,
+                  const FunctionsGL *functions,
+                  StateManagerGL *stateManager,
+                  const WorkaroundsGL &workarounds,
+                  bool isDefault);
     // Constructor called when we need to create a FramebufferGL from an
     // existing framebuffer name, for example for the default framebuffer
     // on the Mac EGL CGL backend.
     FramebufferGL(GLuint id,
                   const gl::Framebuffer::Data &data,
                   const FunctionsGL *functions,
+                  const WorkaroundsGL &workarounds,
                   StateManagerGL *stateManager);
     ~FramebufferGL() override;
 
@@ -57,11 +63,17 @@ class FramebufferGL : public FramebufferImpl
 
     GLenum checkStatus() const override;
 
+    void syncDrawState() const;
+
     GLuint getFramebufferID() const;
 
   private:
+    void syncClearState(GLbitfield mask);
+    void syncClearBufferState(GLenum buffer, GLint drawBuffer);
+
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
+    const WorkaroundsGL &mWorkarounds;
 
     GLuint mFramebufferID;
     bool mIsDefault;

src/libANGLE/renderer/gl/RendererGL.cpp

@@ -255,7 +255,7 @@ ProgramImpl *RendererGL::createProgram(const gl::Program::Data &data)
 
 FramebufferImpl *RendererGL::createFramebuffer(const gl::Framebuffer::Data &data)
 {
-    return new FramebufferGL(data, mFunctions, mStateManager, false);
+    return new FramebufferGL(data, mFunctions, mStateManager, mWorkarounds, false);
 }
 
 TextureImpl *RendererGL::createTexture(GLenum target)

src/libANGLE/renderer/gl/RendererGL.h

@@ -108,6 +108,7 @@ class RendererGL : public Renderer
     const gl::Version &getMaxSupportedESVersion() const;
     const FunctionsGL *getFunctions() const { return mFunctions; }
     StateManagerGL *getStateManager() const { return mStateManager; }
+    const WorkaroundsGL &getWorkarounds() const { return mWorkarounds; }
 
   private:
     void generateCaps(gl::Caps *outCaps, gl::TextureCapsMap* outTextureCaps,

src/libANGLE/renderer/gl/StateManagerGL.cpp

@@ -92,6 +92,7 @@ StateManagerGL::StateManagerGL(const FunctionsGL *functions, const gl::Caps &ren
       mClearColor(0.0f, 0.0f, 0.0f, 0.0f),
       mClearDepth(1.0f),
       mClearStencil(0),
+      mFramebufferSRGBEnabled(false),
       mTextureCubemapSeamlessEnabled(false),
       mLocalDirtyBits()
 {
@@ -542,6 +543,7 @@ gl::Error StateManagerGL::setGenericDrawState(const gl::Data &data)
     const gl::Framebuffer *framebuffer = state.getDrawFramebuffer();
     const FramebufferGL *framebufferGL = GetImplAs<FramebufferGL>(framebuffer);
     bindFramebuffer(GL_DRAW_FRAMEBUFFER, framebufferGL->getFramebufferID());
+    framebufferGL->syncDrawState();
 
     // Seamless cubemaps are required for ES3 and higher contexts.
     setTextureCubemapSeamlessEnabled(data.clientVersion >= 3);
@@ -1263,6 +1265,22 @@ void StateManagerGL::syncState(const gl::State &state, const gl::State::DirtyBit
     }
 }
 
+void StateManagerGL::setFramebufferSRGBEnabled(bool enabled)
+{
+    if (mFramebufferSRGBEnabled != enabled)
+    {
+        mFramebufferSRGBEnabled = enabled;
+        if (mFramebufferSRGBEnabled)
+        {
+            mFunctions->enable(GL_FRAMEBUFFER_SRGB);
+        }
+        else
+        {
+            mFunctions->disable(GL_FRAMEBUFFER_SRGB);
+        }
+    }
+}
+
 void StateManagerGL::setTextureCubemapSeamlessEnabled(bool enabled)
 {
     if (mTextureCubemapSeamlessEnabled != enabled)

src/libANGLE/renderer/gl/StateManagerGL.h

@@ -111,6 +111,8 @@ class StateManagerGL final : angle::NonCopyable
                            GLint skipPixels,
                            GLuint packBuffer);
 
+    void setFramebufferSRGBEnabled(bool enabled);
+
     gl::Error setDrawArraysState(const gl::Data &data,
                                  GLint first,
                                  GLsizei count,
@@ -216,6 +218,7 @@ class StateManagerGL final : angle::NonCopyable
     float mClearDepth;
     GLint mClearStencil;
 
+    bool mFramebufferSRGBEnabled;
     bool mTextureCubemapSeamlessEnabled;
 
     gl::State::DirtyBits mLocalDirtyBits;

src/libANGLE/renderer/gl/SurfaceGL.cpp

@@ -24,6 +24,7 @@ SurfaceGL::~SurfaceGL()
 
 FramebufferImpl *SurfaceGL::createDefaultFramebuffer(const gl::Framebuffer::Data &data)
 {
-    return new FramebufferGL(data, mRenderer->getFunctions(), mRenderer->getStateManager(), true);
+    return new FramebufferGL(data, mRenderer->getFunctions(), mRenderer->getStateManager(),
+                             mRenderer->getWorkarounds(), true);
 }
 }

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -15,7 +15,9 @@ namespace rx
 struct WorkaroundsGL
 {
     WorkaroundsGL()
-        : avoid1BitAlphaTextureFormats(false), rgba4IsNotSupportedForColorRendering(false)
+        : avoid1BitAlphaTextureFormats(false),
+          rgba4IsNotSupportedForColorRendering(false),
+          doesSRGBClearsOnLinearFramebufferAttachments(false)
     {
     }
 
@@ -32,6 +34,12 @@ struct WorkaroundsGL
     // returns GL_FRAMEBUFFER_UNSUPPORTED. Work around this by using a known color-renderable
     // format.
     bool rgba4IsNotSupportedForColorRendering;
+
+    // When clearing a framebuffer on Intel or AMD drivers, when GL_FRAMEBUFFER_SRGB is enabled, the
+    // driver clears to the linearized clear color despite the framebuffer not supporting SRGB
+    // blending.  It only seems to do this when the framebuffer has only linear attachments, mixed
+    // attachments appear to get the correct clear color.
+    bool doesSRGBClearsOnLinearFramebufferAttachments;
 };
 }
 

src/libANGLE/renderer/gl/cgl/PbufferSurfaceCGL.h

@@ -17,6 +17,7 @@ namespace rx
 
 class FunctionsGL;
 class StateManagerGL;
+struct WorkaroundsGL;
 
 class PbufferSurfaceCGL : public SurfaceGL
 {
@@ -51,6 +52,7 @@ class PbufferSurfaceCGL : public SurfaceGL
 
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
+    const WorkaroundsGL &mWorkarounds;
 
     GLuint mFramebuffer;
     GLuint mColorRenderbuffer;

src/libANGLE/renderer/gl/cgl/PbufferSurfaceCGL.mm

@@ -27,6 +27,7 @@ PbufferSurfaceCGL::PbufferSurfaceCGL(RendererGL *renderer,
       mHeight(height),
       mFunctions(functions),
       mStateManager(renderer->getStateManager()),
+      mWorkarounds(renderer->getWorkarounds()),
       mFramebuffer(0),
       mColorRenderbuffer(0),
       mDSRenderbuffer(0)
@@ -134,7 +135,7 @@ EGLint PbufferSurfaceCGL::getSwapBehavior() const
 FramebufferImpl *PbufferSurfaceCGL::createDefaultFramebuffer(const gl::Framebuffer::Data &data)
 {
     // TODO(cwallez) assert it happens only once?
-    return new FramebufferGL(mFramebuffer, data, mFunctions, mStateManager);
+    return new FramebufferGL(mFramebuffer, data, mFunctions, mWorkarounds, mStateManager);
 }
 
 }

src/libANGLE/renderer/gl/cgl/WindowSurfaceCGL.h

@@ -22,6 +22,7 @@ class DisplayCGL;
 class FramebufferGL;
 class FunctionsGL;
 class StateManagerGL;
+struct WorkaroundsGL;
 
 class DisplayLink;
 
@@ -63,6 +64,7 @@ class WindowSurfaceCGL : public SurfaceGL
     CALayer *mLayer;
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
+    const WorkaroundsGL &mWorkarounds;
     DisplayLink *mDisplayLink;
 
     // CGL doesn't have a default framebuffer, we instead render to an IOSurface

src/libANGLE/renderer/gl/cgl/WindowSurfaceCGL.mm

@@ -145,6 +145,7 @@ WindowSurfaceCGL::WindowSurfaceCGL(RendererGL *renderer,
       mLayer(layer),
       mFunctions(functions),
       mStateManager(renderer->getStateManager()),
+      mWorkarounds(renderer->getWorkarounds()),
       mDisplayLink(nullptr),
       mCurrentSurface(0),
       mFramebuffer(0),
@@ -342,7 +343,7 @@ EGLint WindowSurfaceCGL::getSwapBehavior() const
 FramebufferImpl *WindowSurfaceCGL::createDefaultFramebuffer(const gl::Framebuffer::Data &data)
 {
     // TODO(cwallez) assert it happens only once?
-    return new FramebufferGL(mFramebuffer, data, mFunctions, mStateManager);
+    return new FramebufferGL(mFramebuffer, data, mFunctions, mWorkarounds, mStateManager);
 }
 
 void WindowSurfaceCGL::freeSurfaceData(Surface *surface)

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -557,6 +557,10 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
 
     workarounds->rgba4IsNotSupportedForColorRendering =
         functions->standard == STANDARD_GL_DESKTOP && vendor == VENDOR_ID_INTEL;
+
+    workarounds->doesSRGBClearsOnLinearFramebufferAttachments =
+        functions->standard == STANDARD_GL_DESKTOP &&
+        (vendor == VENDOR_ID_INTEL || vendor == VENDOR_ID_AMD);
 }
 
 }

src/tests/gl_tests/ClearTest.cpp

@@ -73,6 +73,33 @@ class ClearTestBase : public ANGLETest
 class ClearTest : public ClearTestBase {};
 class ClearTestES3 : public ClearTestBase {};
 
+// Test clearing the default framebuffer
+TEST_P(ClearTest, DefaultFramebuffer)
+{
+    glClearColor(0.25f, 0.5f, 0.5f, 0.5f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    EXPECT_PIXEL_NEAR(0, 0, 64, 128, 128, 128, 1.0);
+}
+
+// Test clearing a RGBA8 Framebuffer
+TEST_P(ClearTest, RGBA8Framebuffer)
+{
+    glBindFramebuffer(GL_FRAMEBUFFER, mFBO);
+
+    GLuint texture;
+    glGenTextures(1, &texture);
+
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, getWindowWidth(), getWindowHeight(), 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, nullptr);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+
+    glClearColor(0.5f, 0.5f, 0.5f, 0.5f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    EXPECT_PIXEL_NEAR(0, 0, 128, 128, 128, 128, 1.0);
+}
+
 TEST_P(ClearTest, ClearIssue)
 {
     // TODO(geofflang): Figure out why this is broken on Intel OpenGL
@@ -208,6 +235,61 @@ TEST_P(ClearTestES3, BadFBOSerialBug)
     glDeleteFramebuffers(1, &fbo2);
 }
 
+// Test that SRGB framebuffers clear to the linearized clear color
+TEST_P(ClearTestES3, SRGBClear)
+{
+    // First make a simple framebuffer, and clear it
+    glBindFramebuffer(GL_FRAMEBUFFER, mFBO);
+
+    GLuint texture;
+    glGenTextures(1, &texture);
+
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_SRGB8_ALPHA8, getWindowWidth(), getWindowHeight());
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+
+    glClearColor(0.5f, 0.5f, 0.5f, 0.5f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    EXPECT_PIXEL_NEAR(0, 0, 188, 188, 188, 128, 1.0);
+}
+
+// Test that framebuffers with mixed SRGB/Linear attachments clear to the correct color for each
+// attachment
+TEST_P(ClearTestES3, MixedSRGBClear)
+{
+    glBindFramebuffer(GL_FRAMEBUFFER, mFBO);
+
+    GLuint textures[2];
+    glGenTextures(2, &textures[0]);
+
+    glBindTexture(GL_TEXTURE_2D, textures[0]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_SRGB8_ALPHA8, getWindowWidth(), getWindowHeight());
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[0], 0);
+
+    glBindTexture(GL_TEXTURE_2D, textures[1]);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, getWindowWidth(), getWindowHeight());
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1, GL_TEXTURE_2D, textures[1], 0);
+
+    GLenum drawBuffers[] = {GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1};
+    glDrawBuffers(2, drawBuffers);
+
+    // Clear both textures
+    glClearColor(0.5f, 0.5f, 0.5f, 0.5f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1, GL_TEXTURE_2D, 0, 0);
+
+    // Check value of texture0
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[0], 0);
+    EXPECT_PIXEL_NEAR(0, 0, 188, 188, 188, 128, 1.0);
+
+    // Check value of texture1
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[1], 0);
+    EXPECT_PIXEL_NEAR(0, 0, 128, 128, 128, 128, 1.0);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(ClearTest, ES2_D3D9(), ES2_D3D11(), ES3_D3D11(), ES2_OPENGL(), ES3_OPENGL());
-ANGLE_INSTANTIATE_TEST(ClearTestES3, ES3_D3D11());
+ANGLE_INSTANTIATE_TEST(ClearTestES3, ES3_D3D11(), ES3_OPENGL());