[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..ca6b1d363450c6bad5ff978833692fceec3c43f8 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"
@@ -882,10 +883,14 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   // or regular back buffer).
   gfx::Size GetBoundReadFramebufferSize();
 
-  // Get the service side ID for the bound read frame buffer.
+  // Get the service side ID for the bound read framebuffer.
   // If it's back buffer, 0 is returned.
   GLuint GetBoundReadFramebufferServiceId();
 
+  // Get the service side ID for the bound draw framebuffer.
+  // If it's back buffer, 0 is returned.
+  GLuint GetBoundDrawFramebufferServiceId();
+
   // Get the format/type of the currently bound frame buffer (either FBO or
   // regular back buffer).
   // If the color image is a renderbuffer, returns 0 for type.
@@ -2071,6 +2076,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 +2269,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 +4234,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;
 }
 
@@ -4299,6 +4299,21 @@ GLuint GLES2DecoderImpl::GetBoundReadFramebufferServiceId() {
   return 0;
 }
 
+GLuint GLES2DecoderImpl::GetBoundDrawFramebufferServiceId() {
+  Framebuffer* framebuffer =
+    GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER_EXT);
+  if (framebuffer) {
+    return framebuffer->service_id();
+  }
+  if (offscreen_target_frame_buffer_.get()) {
+    return offscreen_target_frame_buffer_->id();
+  }
+  if (surface_.get()) {
+    return surface_->GetBackingFramebufferObject();
+  }
+  return 0;
+}
+
 GLenum GLES2DecoderImpl::GetBoundReadFramebufferTextureType() {
   Framebuffer* framebuffer =
     GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER_EXT);
@@ -4535,6 +4550,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 +4647,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 +7545,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 +7565,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 +7610,87 @@ 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;
+  if (!enable_srgb ||
+      !feature_info_->feature_flags().desktop_srgb_support ||
+      gl_version_info().IsAtLeastGL(4, 4)) {
+    if (enable_srgb && gl_version_info().IsAtLeastGL(4, 4)) {
+      state_.EnableDisableFramebufferSRGB(enable_srgb);
+    }
+
+    // TODO(yunchao) Need to revisit here. In GLES spec, blitFramebuffer
+    // should do scissor test per fragment operation.
+    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);
+    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.
+
+  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(),
+                                  GetBoundReadFramebufferServiceId(),
+                                  src_internal_format,
+                                  GetBoundDrawFramebufferServiceId(),
+                                  GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE);
+  }
+
+  if (draw_buffers_has_srgb) {

[piman, 2016/09/07 23:08:32]

What if both read and draw buffers have srgb?

[yunchao, 2016/09/08 18:32:49]

I would do this in follow-up changes. 

If both read and draw buffers have srgb image, we would convert srgb to linear,
then blit, then convert blit-ed linear image to srgb. Look into the code, it
would call SRGBToLinear and then call LinearToSRGB. Optionally, I can write a
new function SRGBToSRGB(...). The later solution can have better performance, I
suppose. 

A potential optimization may can be applied when the src region has exactly the
same size with the dst region. We may can blit directly from srgb read buffe to
srgb draw buffers. I will write conformance test and revisit this later. 

[piman, 2016/09/08 19:39:17]

How does that work? SRGBToLinear blits from the read FB to the draw FB.
LinearToSRGB also blits from the read FB to the draw FB.
So right now it would do a first incorrect blit, and then overwrite that with a
second incorrect blit.

[yunchao, 2016/09/08 23:16:49]

Yes, the current implementation is not correct here. It can pass the conformance
test, only because that the conformance tests have the same size (both width and
height) for src region and dst region when blit from srgb to srgb.  
So We have 2 solutions to resolve this issue. 
1) create a temp fbo with linear texture as color buffer, then call SRGBToLinear
to read from the source srgb image and draw to the temp fbo. After that, call
LinearToSRGB to read from that temp fbo and draw to the target srgb image.
2) create a new function in srgb_converter, say SRGBToSRGB. then we can blit
srgb to srgb in one shot, instead of call SRGBToLinear and then call
LinearToSRGB. We can do potential optimizations. 
Apparently, solution 2) is better from the perspective of performance. However,
here is a much more complicated scenario: what if drawbuffers have both linear
images and srgb images? So I want to put that work in follow-up change after
make a thorough thinking: Which is really better. So I add them into TODOs and
fix them later.
WDYT? If you want to impl the srgb to srgb bliting first, It is OK. I will impl
srgb->srgb bliting in the next patchset. 

+    state_.EnableDisableFramebufferSRGB(false);
+    if (!InitializeSRGBEncoder(func_name)) {
+      return;
+    }
+    srgb_converter_->LinearToSRGB(this, srcX0, srcY0, srcX1, srcY1,
+                                  dstX0, dstY0, dstX1, dstY1,
+                                  mask, filter,
+                                  GetBoundReadFramebufferSize(),
+                                  GetBoundReadFramebufferServiceId(),
+                                  src_internal_format,
+                                  GetBoundDrawFramebufferServiceId(),
+                                  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() {