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

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 4e24451bc274c7eb418d39f6611a8042ea240225..8eb3d7d28fe32c7dde3c40754a2ed8cd80909ad7 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1360,6 +1360,10 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   // 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.
@@ -4189,18 +4193,6 @@ bool GLES2DecoderImpl::CheckBoundDrawFramebufferValid(const char* func_name) {
       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();
-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
-  }
   return valid;
 }
 
@@ -4215,26 +4207,29 @@ bool GLES2DecoderImpl::CheckBoundReadFramebufferValid(
 }
 
 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);

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

It is not necessary to check twice if the target is GL_FRAMEBUFFER.

-
-  if (valid && feature_info_->feature_flags().desktop_srgb_support) {
-    bool enable_framebuffer_srgb =
-        (draw_framebuffer && draw_framebuffer->HasSRGBAttachments()) ||
-        (read_framebuffer && read_framebuffer->HasSRGBAttachments());
-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
+  if (!features().chromium_framebufer_multisample) {
+    return CheckFramebufferValid(GetFramebufferInfoForTarget(GL_FRAMEBUFFER),
+        GL_FRAMEBUFFER, GL_INVALID_FRAMEBUFFER_OPERATION, func_name);
+  }
+  return CheckFramebufferValid(GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER),
+           GL_READ_FRAMEBUFFER, GL_INVALID_FRAMEBUFFER_OPERATION, func_name) &&
+         CheckFramebufferValid(GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER),
+           GL_DRAW_FRAMEBUFFER, GL_INVALID_FRAMEBUFFER_OPERATION, func_name);
+}
+
+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;
+    }
   }
-
-  return valid;
+  state_.EnableDisableFramebufferSRGB(draw_buffers_has_srgb);
 }
 
 GLint GLES2DecoderImpl::GetColorEncodingFromInternalFormat(
@@ -6930,6 +6925,9 @@ error::Error GLES2DecoderImpl::DoClear(GLbitfield mask) {
             "can't be called on integer buffers");
         return error::kNoError;
       }
+      if (feature_info_->feature_flags().desktop_srgb_support) {
+        EnableDisableFramebufferSRGBForDrawBuffers();
+      }
     }
     glClear(mask);
   }
@@ -6956,6 +6954,11 @@ void GLES2DecoderImpl::DoClearBufferiv(
           "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) {
@@ -6989,6 +6992,11 @@ void GLES2DecoderImpl::DoClearBufferuiv(
         "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);
 }
@@ -7013,6 +7021,11 @@ void GLES2DecoderImpl::DoClearBufferfv(
           "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) {
@@ -7527,6 +7540,9 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
   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);
@@ -7546,6 +7562,8 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
       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))) {
@@ -7582,6 +7600,11 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     }
   }
 
+  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);
@@ -9214,6 +9237,11 @@ error::Error GLES2DecoderImpl::DoDrawArrays(
   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) {
@@ -9355,6 +9383,10 @@ error::Error GLES2DecoderImpl::DoDrawElements(const char* 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()) {