Revert "Explicitly enable framebuffer SRGB blending in StateManagerGL."

src/libANGLE/renderer/gl/FramebufferGL.cpp

@@ -18,20 +18,14 @@
 #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,
-                             const WorkaroundsGL &workarounds,
-                             bool isDefault)
+FramebufferGL::FramebufferGL(const gl::Framebuffer::Data &data, const FunctionsGL *functions, StateManagerGL *stateManager, bool isDefault)
     : FramebufferImpl(data),
       mFunctions(functions),
       mStateManager(stateManager),
-      mWorkarounds(workarounds),
       mFramebufferID(0),
       mIsDefault(isDefault)
 {
@@ -44,12 +38,10 @@ FramebufferGL::FramebufferGL(const gl::Framebuffer::Data &data,
 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)
 {
@@ -205,7 +197,6 @@ 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);
 
@@ -214,7 +205,6 @@ 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);
 
@@ -223,7 +213,6 @@ 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);
 
@@ -232,7 +221,6 @@ 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);
 
@@ -241,7 +229,6 @@ 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);
 
@@ -303,72 +290,4 @@ 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,23 +16,17 @@ namespace rx
 
 class FunctionsGL;
 class StateManagerGL;
-struct WorkaroundsGL;
 
 class FramebufferGL : public FramebufferImpl
 {
   public:
-    FramebufferGL(const gl::Framebuffer::Data &data,
-                  const FunctionsGL *functions,
-                  StateManagerGL *stateManager,
-                  const WorkaroundsGL &workarounds,
-                  bool isDefault);
+    FramebufferGL(const gl::Framebuffer::Data &data, const FunctionsGL *functions, StateManagerGL *stateManager, 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;
 
@@ -63,17 +57,11 @@ 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, mWorkarounds, false);
+    return new FramebufferGL(data, mFunctions, mStateManager, false);
 }
 
 TextureImpl *RendererGL::createTexture(GLenum target)

src/libANGLE/renderer/gl/RendererGL.h

@@ -108,7 +108,6 @@ 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,7 +92,6 @@ 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()
 {
@@ -543,7 +542,6 @@ 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);
@@ -1265,22 +1263,6 @@ 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,8 +111,6 @@ class StateManagerGL final : angle::NonCopyable
                            GLint skipPixels,
                            GLuint packBuffer);
 
-    void setFramebufferSRGBEnabled(bool enabled);
-
     gl::Error setDrawArraysState(const gl::Data &data,
                                  GLint first,
                                  GLsizei count,
@@ -218,7 +216,6 @@ class StateManagerGL final : angle::NonCopyable
     float mClearDepth;
     GLint mClearStencil;
 
-    bool mFramebufferSRGBEnabled;
     bool mTextureCubemapSeamlessEnabled;
 
     gl::State::DirtyBits mLocalDirtyBits;

src/libANGLE/renderer/gl/SurfaceGL.cpp

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

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -15,9 +15,7 @@ namespace rx
 struct WorkaroundsGL
 {
     WorkaroundsGL()
-        : avoid1BitAlphaTextureFormats(false),
-          rgba4IsNotSupportedForColorRendering(false),
-          doesSRGBClearsOnLinearFramebufferAttachments(false)
+        : avoid1BitAlphaTextureFormats(false), rgba4IsNotSupportedForColorRendering(false)
     {
     }
 
@@ -34,12 +32,6 @@ 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,7 +17,6 @@ namespace rx
 
 class FunctionsGL;
 class StateManagerGL;
-struct WorkaroundsGL;
 
 class PbufferSurfaceCGL : public SurfaceGL
 {
@@ -52,7 +51,6 @@ class PbufferSurfaceCGL : public SurfaceGL
 
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
-    const WorkaroundsGL &mWorkarounds;
 
     GLuint mFramebuffer;
     GLuint mColorRenderbuffer;

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

@@ -27,7 +27,6 @@ PbufferSurfaceCGL::PbufferSurfaceCGL(RendererGL *renderer,
       mHeight(height),
       mFunctions(functions),
       mStateManager(renderer->getStateManager()),
-      mWorkarounds(renderer->getWorkarounds())
       mFramebuffer(0),
       mColorRenderbuffer(0),
       mDSRenderbuffer(0)
@@ -135,7 +134,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, mWorkarounds, mStateManager);
+    return new FramebufferGL(mFramebuffer, data, mFunctions, mStateManager);
 }
 
 }

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

@@ -22,7 +22,6 @@ class DisplayCGL;
 class FramebufferGL;
 class FunctionsGL;
 class StateManagerGL;
-struct WorkaroundsGL;
 
 class DisplayLink;
 
@@ -64,7 +63,6 @@ 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,7 +145,6 @@ WindowSurfaceCGL::WindowSurfaceCGL(RendererGL *renderer,
       mLayer(layer),
       mFunctions(functions),
       mStateManager(renderer->getStateManager()),
-      mWorkarounds(renderer->getWorkarounds())
       mDisplayLink(nullptr),
       mCurrentSurface(0),
       mFramebuffer(0),
@@ -343,7 +342,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, mWorkarounds, mStateManager);
+    return new FramebufferGL(mFramebuffer, data, mFunctions, mStateManager);
 }
 
 void WindowSurfaceCGL::freeSurfaceData(Surface *surface)

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -557,10 +557,6 @@ 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,33 +73,6 @@ 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
@@ -235,61 +208,6 @@ 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(), ES3_OPENGL());
+ANGLE_INSTANTIATE_TEST(ClearTestES3, ES3_D3D11());