[Command Buffer] enable/disable FRAMEBUFFER_SRGB only when sRGB image is active

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 1341 matching lines @@
       GLenum gl_error,
       const char* func_name);
 
   bool CheckBoundDrawFramebufferValid(const char* func_name);
   // Generates |gl_error| if the bound read fbo is incomplete.
   bool CheckBoundReadFramebufferValid(const char* func_name, GLenum gl_error);
   // This is only used by DoBlitFramebufferCHROMIUM which operates read/draw
   // framebuffer at the same time.
   bool CheckBoundFramebufferValid(const char* func_name);
 
+  // If one or more drawBuffers have srgb color format, enable framebuffer srgb.
+  // Otherwise, disable framebuffer srgb.
+  void EnableDisableFramebufferSRGBForDrawBuffers();
+
   // Checks if the current program exists and is valid. If not generates the
   // appropriate GL error.  Returns true if the current program is in a usable
   // state.
   bool CheckCurrentProgram(const char* function_name);
 
   // Checks if the current program exists and is valid and that location is not
   // -1. If the current program is not valid generates the appropriate GL
   // error. Returns true if the current program is in a usable state and
   // location is not -1.
   bool CheckCurrentProgramForUniform(GLint location, const char* function_name);
@@ skipping 2805 matching lines @@
 }
 
 bool GLES2DecoderImpl::CheckBoundDrawFramebufferValid(const char* func_name) {
   GLenum target = features().chromium_framebuffer_multisample ?
       GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER;
   Framebuffer* framebuffer = GetFramebufferInfoForTarget(target);
   bool valid = CheckFramebufferValid(
       framebuffer, target, GL_INVALID_FRAMEBUFFER_OPERATION, func_name);
   if (valid && !features().chromium_framebuffer_multisample)
     OnUseFramebuffer();
-  if (valid && feature_info_->feature_flags().desktop_srgb_support) {
-    // If framebuffer contains sRGB images, then enable FRAMEBUFFER_SRGB.
-    // Otherwise, disable FRAMEBUFFER_SRGB. Assume default fbo does not have
-    // sRGB image.
-    // In theory, we can just leave FRAMEBUFFER_SRGB on. However, many drivers
-    // behave incorrectly when all images are linear encoding, they still apply
-    // the sRGB conversion, but when at least one image is sRGB, then they
-    // behave correctly.
-    bool enable_framebuffer_srgb =
-        framebuffer && framebuffer->HasSRGBAttachments();

[yunchao, 2016/08/24 14:08:51]

This is not correct. If the fbo has srgb color buffer, but they are not
drawBuffers, we should not enable framebuffer_srgb.

[Zhenyao Mo, 2016/08/25 00:39:37]

What we found is whether the sRGB image is currently active doesn't matter. 
However, if you want to fine-tuning it, that's OK. But why moving this out of
here and put it in all calls?  To me, putting it here is much cleaner.

[yunchao, 2016/08/25 16:29:51]

That's true. It can draw correctly if there is sRGB image, no matter the sRGB
image is currently active or not. 
moving this outside of this function is because of fine tuning. Suppose that the
drawBuffer (clear or draw, etc) is depth buffer, it is not necessary to do this
check at all. Because the sRGB image is definitely inactive, we don't need to
check/change the framebuffer srgb state. 

-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
-  }
   return valid;
 }
 
 bool GLES2DecoderImpl::CheckBoundReadFramebufferValid(
     const char* func_name, GLenum gl_error) {
   GLenum target = features().chromium_framebuffer_multisample ?
       GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
   Framebuffer* framebuffer = GetFramebufferInfoForTarget(target);
   bool valid = CheckFramebufferValid(
       framebuffer, target, gl_error, func_name);
   return valid;
 }
 
 bool GLES2DecoderImpl::CheckBoundFramebufferValid(const char* func_name) {
   GLenum target = features().chromium_framebuffer_multisample ?
       GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER;
   Framebuffer* draw_framebuffer = GetFramebufferInfoForTarget(target);
   bool valid = CheckFramebufferValid(
       draw_framebuffer, target, GL_INVALID_FRAMEBUFFER_OPERATION, func_name);
 
   target = features().chromium_framebuffer_multisample ?
       GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
   Framebuffer* read_framebuffer = GetFramebufferInfoForTarget(target);
   valid = valid && CheckFramebufferValid(
       read_framebuffer, target, GL_INVALID_FRAMEBUFFER_OPERATION, func_name);
-
-  if (valid && feature_info_->feature_flags().desktop_srgb_support) {
-    bool enable_framebuffer_srgb =
-        (draw_framebuffer && draw_framebuffer->HasSRGBAttachments()) ||
-        (read_framebuffer && read_framebuffer->HasSRGBAttachments());

[yunchao, 2016/08/24 14:08:51]

The same reason. If the fbo has srgb color buffer, but they are not
drawBuffers/readBuffer, we should not enable framebuffer_srgb.

-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
-  }
-
   return valid;
 }
 
+void GLES2DecoderImpl::EnableDisableFramebufferSRGBForDrawBuffers() {
+  bool draw_buffers_has_srgb = false;
+  for (uint32_t ii = 0; ii < group_->max_draw_buffers(); ++ii) {
+    GLenum dst_format = GetBoundColorDrawBufferInternalFormat(
+        static_cast<GLint>(ii));
+    if (dst_format == 0)
+      continue;
+    if (GetColorEncodingFromInternalFormat(dst_format) == GL_SRGB) {
+      draw_buffers_has_srgb = true;
+      break;
+    }
+  }
+  state_.EnableDisableFramebufferSRGB(draw_buffers_has_srgb);
+}
+
 GLint GLES2DecoderImpl::GetColorEncodingFromInternalFormat(
     GLenum internalformat) {
   switch (internalformat) {
     case GL_SRGB_EXT:
     case GL_SRGB_ALPHA_EXT:
     case GL_SRGB8:
     case GL_SRGB8_ALPHA8:
       return GL_SRGB;
     default:
       return GL_LINEAR;
@@ skipping 2662 matching lines @@
       return error::kNoError;
     }
     if (mask & GL_COLOR_BUFFER_BIT) {
       Framebuffer* framebuffer =
           framebuffer_state_.bound_draw_framebuffer.get();
       if (framebuffer && framebuffer->ContainsActiveIntegerAttachments()) {
         LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
             "can't be called on integer buffers");
         return error::kNoError;
       }
+      if (feature_info_->feature_flags().desktop_srgb_support) {
+        EnableDisableFramebufferSRGBForDrawBuffers();
+      }
     }
     glClear(mask);
   }
   return error::kNoError;
 }
 
 void GLES2DecoderImpl::DoClearBufferiv(
     GLenum buffer, GLint drawbuffer, const GLint* value) {
   const char* func_name = "glClearBufferiv";
   if (!CheckBoundDrawFramebufferValid(func_name))
     return;
   ApplyDirtyState();
 
   if (buffer == GL_COLOR) {
     if (drawbuffer < 0 ||
         drawbuffer >= static_cast<GLint>(group_->max_draw_buffers())) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "invalid drawBuffer");
       return;
     }
     GLenum internal_format =
         GetBoundColorDrawBufferInternalFormat(drawbuffer);
     if (!GLES2Util::IsSignedIntegerFormat(internal_format)) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
           "can only be called on signed integer buffers");
       return;
     }
+    if (feature_info_->feature_flags().desktop_srgb_support) {
+      bool draw_buffer_has_srgb =
+          GetColorEncodingFromInternalFormat(internal_format) == GL_SRGB;
+      state_.EnableDisableFramebufferSRGB(draw_buffer_has_srgb);
+    }
   } else {
     DCHECK(buffer == GL_STENCIL);
     if (drawbuffer != 0) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "invalid drawBuffer");
       return;
     }
     if (!BoundFramebufferHasStencilAttachment()) {
       return;
     }
   }
@@ skipping 13 matching lines @@
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "invalid drawBuffer");
     return;
   }
   GLenum internal_format =
       GetBoundColorDrawBufferInternalFormat(drawbuffer);
   if (!GLES2Util::IsUnsignedIntegerFormat(internal_format)) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
         "can only be called on unsigned integer buffers");
     return;
   }
+  if (feature_info_->feature_flags().desktop_srgb_support) {
+    bool draw_buffer_has_srgb =
+        GetColorEncodingFromInternalFormat(internal_format) == GL_SRGB;
+    state_.EnableDisableFramebufferSRGB(draw_buffer_has_srgb);
+  }
   MarkDrawBufferAsCleared(buffer, drawbuffer);
   glClearBufferuiv(buffer, drawbuffer, value);
 }
 
 void GLES2DecoderImpl::DoClearBufferfv(
     GLenum buffer, GLint drawbuffer, const GLfloat* value) {
   const char* func_name = "glClearBufferfv";
   if (!CheckBoundDrawFramebufferValid(func_name))
     return;
   ApplyDirtyState();
 
   if (buffer == GL_COLOR) {
     if (drawbuffer < 0 ||
         drawbuffer >= static_cast<GLint>(group_->max_draw_buffers())) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "invalid drawBuffer");
       return;
     }
     GLenum internal_format =
         GetBoundColorDrawBufferInternalFormat(drawbuffer);
     if (GLES2Util::IsIntegerFormat(internal_format)) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
           "can only be called on float buffers");
       return;
     }
+    if (feature_info_->feature_flags().desktop_srgb_support) {
+      bool draw_buffer_has_srgb =
+          GetColorEncodingFromInternalFormat(internal_format) == GL_SRGB;
+      state_.EnableDisableFramebufferSRGB(draw_buffer_has_srgb);
+    }
   } else {
     DCHECK(buffer == GL_DEPTH);
     if (drawbuffer != 0) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "invalid drawBuffer");
       return;
     }
     if (!BoundFramebufferHasDepthAttachment()) {
       return;
     }
   }
@@ skipping 494 matching lines @@
   if (read_buffer_samples > 0 &&
       (srcX0 != dstX0 || srcY0 != dstY0 || srcX1 != dstX1 || srcY1 != dstY1)) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
         "src framebuffer is multisampled, but src/dst regions are different");
     return;
   }
 
   GLenum src_format = GetBoundReadFrameBufferInternalFormat();
   GLenum src_type = GetBoundReadFrameBufferTextureType();
 
+  bool read_buffer_has_srgb =
+      GetColorEncodingFromInternalFormat(src_format) == GL_SRGB;
+  bool draw_buffers_has_srgb = false;
   if ((mask & GL_COLOR_BUFFER_BIT) != 0) {
     bool is_src_signed_int = GLES2Util::IsSignedIntegerFormat(src_format);
     bool is_src_unsigned_int = GLES2Util::IsUnsignedIntegerFormat(src_format);
     DCHECK(!is_src_signed_int || !is_src_unsigned_int);
 
     if ((is_src_signed_int || is_src_unsigned_int) && filter == GL_LINEAR) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
                          "invalid filter for integer format");
       return;
     }
 
     GLenum src_sized_format =
         GLES2Util::ConvertToSizedFormat(src_format, src_type);
     for (uint32_t ii = 0; ii < group_->max_draw_buffers(); ++ii) {
       GLenum dst_format = GetBoundColorDrawBufferInternalFormat(
           static_cast<GLint>(ii));
       GLenum dst_type = GetBoundColorDrawBufferType(static_cast<GLint>(ii));
       if (dst_format == 0)
         continue;
+      if (GetColorEncodingFromInternalFormat(dst_format) == GL_SRGB)
+        draw_buffers_has_srgb = true;
       if (read_buffer_samples > 0 &&
           (src_sized_format !=
            GLES2Util::ConvertToSizedFormat(dst_format, dst_type))) {
         LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
                            "src and dst formats differ for color");
         return;
       }
       bool is_dst_signed_int = GLES2Util::IsSignedIntegerFormat(dst_format);
       bool is_dst_unsigned_int = GLES2Util::IsUnsignedIntegerFormat(dst_format);
       DCHECK(!is_dst_signed_int || !is_dst_unsigned_int);
@@ skipping 16 matching lines @@
     if ((GetBoundFrameBufferDepthFormat(GL_READ_FRAMEBUFFER) !=
          GetBoundFrameBufferDepthFormat(GL_DRAW_FRAMEBUFFER)) ||
         (GetBoundFrameBufferStencilFormat(GL_READ_FRAMEBUFFER) !=
          GetBoundFrameBufferStencilFormat(GL_DRAW_FRAMEBUFFER))) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
                          "src and dst formats differ for depth/stencil");
       return;
     }
   }
 
+  if (feature_info_->feature_flags().desktop_srgb_support) {
+    bool enable_srgb = read_buffer_has_srgb || draw_buffers_has_srgb;
+    state_.EnableDisableFramebufferSRGB(enable_srgb);
+  }
+
   state_.SetDeviceCapabilityState(GL_SCISSOR_TEST, false);
   BlitFramebufferHelper(
       srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, mask, filter);
   state_.SetDeviceCapabilityState(GL_SCISSOR_TEST,
                                   state_.enable_flags.scissor_test);
 }
 
 void GLES2DecoderImpl::EnsureRenderbufferBound() {
   if (!state_.bound_renderbuffer_valid) {
     state_.bound_renderbuffer_valid = true;
@@ skipping 1608 matching lines @@
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "count < 0");
     return error::kNoError;
   }
   if (primcount < 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "primcount < 0");
     return error::kNoError;
   }
   if (!CheckBoundDrawFramebufferValid(function_name)) {
     return error::kNoError;
   }
+
+  if (feature_info_->feature_flags().desktop_srgb_support) {
+    EnableDisableFramebufferSRGBForDrawBuffers();
+  }
+
   // We have to check this here because the prototype for glDrawArrays
   // is GLint not GLsizei.
   if (first < 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "first < 0");
     return error::kNoError;
   }
 
   if (state_.bound_transform_feedback.get() &&
       state_.bound_transform_feedback->active() &&
       !state_.bound_transform_feedback->paused() &&
@@ skipping 121 matching lines @@
   }
   if (primcount < 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "primcount < 0");
     return error::kNoError;
   }
 
   if (!CheckBoundDrawFramebufferValid(function_name)) {
     return error::kNoError;
   }
 
+  if (feature_info_->feature_flags().desktop_srgb_support) {
+    EnableDisableFramebufferSRGBForDrawBuffers();
+  }
+
   if (state_.bound_transform_feedback.get() &&
       state_.bound_transform_feedback->active() &&
       !state_.bound_transform_feedback->paused()) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
         "transformfeedback is active and not paused");
     return error::kNoError;
   }
 
   if (count == 0 || primcount == 0) {
     return error::kNoError;
@@ skipping 8365 matching lines @@
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "base/macros.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu