[Command Buffer] emulate SRGB color format for BlitFramebuffer in OpenGL

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 991d525ff7b79a21c7e6434401e8a9b21c22ed3e..113e2b227a26a0d8586516cdce423da46f8541b0 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -44,6 +44,7 @@
 #include "gpu/command_buffer/service/gles2_cmd_copy_tex_image.h"
 #include "gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h"
+#include "gpu/command_buffer/service/gles2_cmd_srgb_converter.h"
 #include "gpu/command_buffer/service/gles2_cmd_validation.h"
 #include "gpu/command_buffer/service/gpu_preferences.h"
 #include "gpu/command_buffer/service/gpu_state_tracer.h"
@@ -2071,6 +2072,8 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
 
   bool InitializeCopyTexImageBlitter(const char* function_name);
   bool InitializeCopyTextureCHROMIUM(const char* function_name);
+  bool InitializeSRGBDecoder(const char* function_name);
+  bool InitializeSRGBEncoder(const char* function_name);
   // Generate a member function prototype for each command in an automated and
   // typesafe way.
 #define GLES2_CMD_OP(name) \
@@ -2262,6 +2265,7 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
       apply_framebuffer_attachment_cmaa_intel_;
   std::unique_ptr<CopyTexImageResourceManager> copy_tex_image_blit_;
   std::unique_ptr<CopyTextureCHROMIUMResourceManager> copy_texture_CHROMIUM_;
+  std::unique_ptr<SRGBConverter> srgb_converter_;
   std::unique_ptr<ClearFramebufferResourceManager> clear_framebuffer_blit_;
 
   // Cached values of the currently assigned viewport dimensions.
@@ -4226,14 +4230,6 @@ bool GLES2DecoderImpl::CheckBoundFramebufferValid(const char* func_name) {
   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());
-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
-  }
-
   return valid;
 }
 
@@ -4535,6 +4531,11 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
       copy_texture_CHROMIUM_.reset();
     }
 
+    if (srgb_converter_.get()) {
+      srgb_converter_->Destroy();
+      srgb_converter_.reset();
+    }
+
     clear_framebuffer_blit_.reset();
 
     if (state_.current_program.get()) {
@@ -4627,6 +4628,7 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
   apply_framebuffer_attachment_cmaa_intel_.reset();
   copy_tex_image_blit_.reset();
   copy_texture_CHROMIUM_.reset();
+  srgb_converter_.reset();
   clear_framebuffer_blit_.reset();
 
   if (query_manager_.get()) {
@@ -7524,12 +7526,17 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     return;
   }
 
-  GLenum src_format = GetBoundReadFramebufferInternalFormat();
+  GLenum src_internal_format = GetBoundReadFramebufferInternalFormat();
   GLenum src_type = GetBoundReadFramebufferTextureType();
 
+  bool read_buffer_has_srgb =
+      GetColorEncodingFromInternalFormat(src_internal_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);
+    bool is_src_signed_int =
+        GLES2Util::IsSignedIntegerFormat(src_internal_format);
+    bool is_src_unsigned_int =
+        GLES2Util::IsUnsignedIntegerFormat(src_internal_format);
     DCHECK(!is_src_signed_int || !is_src_unsigned_int);
 
     if ((is_src_signed_int || is_src_unsigned_int) && filter == GL_LINEAR) {
@@ -7539,13 +7546,15 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     }
 
     GLenum src_sized_format =
-        GLES2Util::ConvertToSizedFormat(src_format, src_type);
+        GLES2Util::ConvertToSizedFormat(src_internal_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))) {
@@ -7582,11 +7591,94 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     }
   }
 
-  state_.SetDeviceCapabilityState(GL_SCISSOR_TEST, false);
-  BlitFramebufferHelper(
-      srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, mask, filter);
-  state_.SetDeviceCapabilityState(GL_SCISSOR_TEST,
+  bool enable_srgb = read_buffer_has_srgb || draw_buffers_has_srgb;

[yunchao, 2016/08/29 14:33:47]

We need to traverse the read buffer and draw buffers to handle the TODO below: 
drawBuffers have srgb images, as well as non-srgb images during bliting. 

That patch is also ready, but it is a little tricky to cover all corner cases.
Moreover, there is no test cases to cover this feature and expose potential
errors. So I would like you to review the main part at first. 

+  bool same_size = (abs(srcX1 - srcX0) == abs(dstX1 - dstX0)) &&
+                   (abs(srcY1 - srcY0) == abs(dstY1 - dstY0));
+  if (!enable_srgb ||
+      !feature_info_->feature_flags().desktop_srgb_support ||
+      !feature_info_->gl_version_info().is_desktop_core_profile ||

[Zhenyao Mo, 2016/08/29 23:36:50]

why core profile here?

[yunchao, 2016/08/31 09:18:44]

Removed them already.

+      (read_buffer_has_srgb && draw_buffers_has_srgb && same_size) ||

[Zhenyao Mo, 2016/08/29 23:36:50]

Why same_size here?

[yunchao, 2016/08/31 09:18:44]

When we blit a read buffer with srgb color format to draw buffers with srgb
color format, if the size of src region is not the same as the size of the dst
region, we need to filter. You know, srgb is non-linear, filtering against srgb
color format is not correct (See qiankun's conformance test for generateMipmap
against srgb: https://github.com/KhronosGroup/WebGL/pull/1951). 

In this situation, we need to convert srgb to linear, then blit and filter,
finally, convert linear to srgb. 

However, if the src region is exactly the same size with the dst region, we can
blit srgb read buffer to srgb draw buffers directly without filtering.  

[Zhenyao Mo, 2016/09/01 19:28:33]

Our purpose is to emulate ES3 sRGB behavior, which GL 4.4 is the same, but older
version isn't, they didn't perform decoding when read from sRGB formats.  So
even with the same size, you will still need to do some emulation.

[yunchao, 2016/09/02 14:11:16]

Yes, you are correct. I thought that if the size was the same (It was common in
real case), we can disable framebuffer_srgb_ and blit-ed directly without any
decode/encode. It would be an performance optimization. Unfortunately, the
underlying driver may do encode during bliting even framebuffer_srgb_ flag was
false, so it would get wrong result. 
The latest patchset removes this line, and always do encode/decode by emulation.

+      gl_version_info().IsAtLeastGL(4, 4)) {
+    if (enable_srgb && gl_version_info().IsAtLeastGL(4, 4)) {
+      state_.EnableDisableFramebufferSRGB(enable_srgb);
+    }
+
+    state_.SetDeviceCapabilityState(GL_SCISSOR_TEST, false);

[Zhenyao Mo, 2016/08/29 23:39:59]

This is a nasty situation. We define the CHROMIUM version of blitframebuffer
extension as no SCISSOR_TEST, whereas the ES3 version should have SCISSOR_TEST. 
If you add a conformance test to verify SCISSOR_TEST and BlitFramebuffer, you
will have to fix here also, i.e., branch out into CHROMIUM extension behavior
and ES3 behavior,

[yunchao, 2016/08/31 09:18:44]

That's true. The GLES spec says that blitFramebuffer should do scissor test per
fragment operation. 
The original code has the same problem too. Seems that current CTS doesn't cover
this feature. I will write conformance test and revisit this later. I have added
a TODO for it.

+    BlitFramebufferHelper(
+        srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, mask, filter);
+    state_.SetDeviceCapabilityState(GL_SCISSOR_TEST,
                                   state_.enable_flags.scissor_test);
+    return;
+  }
+
+  // emulate srgb for desktop core profile when GL version < 4.4
+  // TODO(yunchao): Need to handle this situation:
+  // There are multiple draw buffers. Some of them are srgb image.
+  // The others are not.
+  GLenum target = features().chromium_framebuffer_multisample ?
+      GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER;
+  Framebuffer* draw_framebuffer = GetFramebufferInfoForTarget(target);
+
+  target = features().chromium_framebuffer_multisample ?
+      GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+  Framebuffer* read_framebuffer = GetFramebufferInfoForTarget(target);

[Zhenyao Mo, 2016/08/29 23:36:50]

You should just use GetBoundReadFramebufferServiceId() and
GetBoundDrawFramebufferServiceId(). The latter needs to be added though.

[yunchao, 2016/08/31 09:18:44]

Done.

+
+  if (read_buffer_has_srgb) {
+    state_.EnableDisableFramebufferSRGB(false);
+    if (!InitializeSRGBDecoder(func_name)) {
+      return;
+    }
+    srgb_converter_->SRGBToLinear(this, srcX0, srcY0, srcX1, srcY1,
+                                  dstX0, dstY0, dstX1, dstY1,
+                                  mask, filter,
+                                  GetBoundReadFramebufferSize(),
+                                  read_framebuffer->service_id(),
+                                  src_internal_format,
+                                  draw_framebuffer->service_id(),
+                                  GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE);
+  }
+
+  if (draw_buffers_has_srgb) {
+    state_.EnableDisableFramebufferSRGB(false);
+    if (!InitializeSRGBEncoder(func_name)) {
+      return;
+    }
+    srgb_converter_->LinearToSRGB(this, srcX0, srcY0, srcX1, srcY1,
+                                  dstX0, dstY0, dstX1, dstY1,
+                                  mask, filter,
+                                  GetBoundReadFramebufferSize(),
+                                  read_framebuffer->service_id(),
+                                  src_internal_format,
+                                  draw_framebuffer->service_id(),
+                                  GL_SRGB8_ALPHA8,
+                                  GL_RGBA, GL_UNSIGNED_BYTE);
+  }
+}
+
+bool GLES2DecoderImpl::InitializeSRGBDecoder(
+    const char* function_name) {
+  if (!srgb_converter_.get()) {
+    LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER(function_name);
+    srgb_converter_.reset(
+        new SRGBConverter(feature_info_.get()));
+    srgb_converter_->InitializeSRGBDecoder(this);
+    if (LOCAL_PEEK_GL_ERROR(function_name) != GL_NO_ERROR)
+      return false;
+  }
+  return true;
+}
+
+bool GLES2DecoderImpl::InitializeSRGBEncoder(
+    const char* function_name) {
+  if (!srgb_converter_.get()) {
+    LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER(function_name);
+    srgb_converter_.reset(
+        new SRGBConverter(feature_info_.get()));
+    srgb_converter_->InitializeSRGBEncoder(this);
+    if (LOCAL_PEEK_GL_ERROR(function_name) != GL_NO_ERROR)
+      return false;
+  }
+  return true;
 }
 
 void GLES2DecoderImpl::EnsureRenderbufferBound() {