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

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 26 matching lines @@
 #include "gpu/command_buffer/service/error_state.h"
 #include "gpu/command_buffer/service/feature_info.h"
 #include "gpu/command_buffer/service/framebuffer_manager.h"
 #include "gpu/command_buffer/service/gl_stream_texture_image.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 #include "gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.h"
 #include "gpu/command_buffer/service/gles2_cmd_clear_framebuffer.h"
 #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"
 #include "gpu/command_buffer/service/gpu_tracer.h"
 #include "gpu/command_buffer/service/image_factory.h"
 #include "gpu/command_buffer/service/image_manager.h"
 #include "gpu/command_buffer/service/logger.h"
 #include "gpu/command_buffer/service/mailbox_manager.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
 #include "gpu/command_buffer/service/path_manager.h"
@@ skipping 749 matching lines @@
 
   // Deletes the TransformFeedback info for the given transformfeedback.
   void RemoveTransformFeedback(GLuint client_id) {
     transform_feedback_manager_->RemoveTransformFeedback(client_id);
   }
 
   // Get the size (in pixels) of the currently bound frame buffer (either FBO
   // 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.
   GLenum GetBoundReadFramebufferTextureType();
   GLenum GetBoundReadFramebufferInternalFormat();
 
   // Get the i-th draw buffer's internal format/type from the bound framebuffer.
   // If no framebuffer is bound, or no image is attached, or the DrawBuffers
   // setting for that image is GL_NONE, return 0.
   GLenum GetBoundColorDrawBufferType(GLint drawbuffer_i);
@@ skipping 1223 matching lines @@
 
   // The GL_CHROMIUM_schedule_ca_layer extension requires that SwapBuffers and
   // equivalent functions reset shared state.
   void ClearScheduleCALayerState();
 
   // Helper method to call glClear workaround.
   void ClearFramebufferForWorkaround(GLbitfield mask);
 
   bool InitializeCopyTexImageBlitter(const char* function_name);
   bool InitializeCopyTextureCHROMIUM(const char* function_name);
+  bool InitializeSRGBConverter(const char* function_name);
   // Generate a member function prototype for each command in an automated and
   // typesafe way.
 #define GLES2_CMD_OP(name) \
   Error Handle##name(uint32_t immediate_data_size, const volatile void* data);
 
   GLES2_COMMAND_LIST(GLES2_CMD_OP)
 
   #undef GLES2_CMD_OP
 
   // The GL context this decoder renders to on behalf of the client.
@@ skipping 172 matching lines @@
   // Forces the backbuffer to use native GMBs rather than a TEXTURE_2D texture.
   bool should_use_native_gmb_for_backbuffer_;
 
   // Log extra info.
   bool service_logging_;
 
   std::unique_ptr<ApplyFramebufferAttachmentCMAAINTELResourceManager>
       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.
   GLsizei viewport_max_width_;
   GLsizei viewport_max_height_;
 
   // Command buffer stats.
   base::TimeDelta total_processing_commands_time_;
 
   // States related to each manager.
@@ skipping 1962 matching lines @@
       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());
-    state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
-  }
-
   return valid;
 }
 
 GLint GLES2DecoderImpl::GetColorEncodingFromInternalFormat(
     GLenum internalformat) {
   switch (internalformat) {
     case GL_SRGB_EXT:
     case GL_SRGB_ALPHA_EXT:
     case GL_SRGB8:
     case GL_SRGB8_ALPHA8:
@@ skipping 45 matching lines @@
   }
   if (offscreen_target_frame_buffer_.get()) {
     return offscreen_target_frame_buffer_->id();
   }
   if (surface_.get()) {
     return surface_->GetBackingFramebufferObject();
   }
   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);
   if (framebuffer) {
     return framebuffer->GetReadBufferTextureType();
   } else {  // Back buffer.
     if (back_buffer_read_buffer_ == GL_NONE)
       return 0;
     return GL_UNSIGNED_BYTE;
   }
@@ skipping 216 matching lines @@
     if (copy_tex_image_blit_.get()) {
       copy_tex_image_blit_->Destroy();
       copy_tex_image_blit_.reset();
     }
 
     if (copy_texture_CHROMIUM_.get()) {
       copy_texture_CHROMIUM_->Destroy();
       copy_texture_CHROMIUM_.reset();
     }
 
+    if (srgb_converter_.get()) {
+      srgb_converter_->Destroy();
+      srgb_converter_.reset();
+    }
+
     clear_framebuffer_blit_.reset();
 
     if (state_.current_program.get()) {
       program_manager()->UnuseProgram(shader_manager(),
                                       state_.current_program.get());
     }
 
     if (attrib_0_buffer_id_) {
       glDeleteBuffersARB(1, &attrib_0_buffer_id_);
     }
@@ skipping 77 matching lines @@
   // Current program must be cleared after calling ProgramManager::UnuseProgram.
   // Otherwise, we can leak objects. http://crbug.com/258772.
   // state_.current_program must be reset before group_ is reset because
   // the later deletes the ProgramManager object that referred by
   // state_.current_program object.
   state_.current_program = NULL;
 
   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()) {
     query_manager_->Destroy(have_context);
     query_manager_.reset();
   }
 
   if (vertex_array_manager_ .get()) {
     vertex_array_manager_->Destroy(have_context);
     vertex_array_manager_.reset();
@@ skipping 2933 matching lines @@
           read_framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
       const Framebuffer::Attachment* stencil_buffer_draw =
           draw_framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
       if (stencil_buffer_draw &&
           stencil_buffer_draw->IsSameAttachment(stencil_buffer_read)) {
         is_feedback_loop = FeedbackLoopTrue;
       }
     }
   }
 
-  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) {
       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);
+        GLES2Util::ConvertToSizedFormat(src_internal_format, src_type);
     DCHECK(read_framebuffer || (is_feedback_loop != FeedbackLoopUnknown));
     const Framebuffer::Attachment* read_buffer =
         is_feedback_loop == FeedbackLoopUnknown ?
         read_framebuffer->GetReadBufferAttachment() : nullptr;
     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 35 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;
     }
   }
 
-  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) &&
+      ((mask & GL_COLOR_BUFFER_BIT) != 0);
+  bool encode_srgb_only =
+      (draw_buffers_has_srgb && !read_buffer_has_srgb) &&
+      ((mask & GL_COLOR_BUFFER_BIT) != 0);
+  if (!enable_srgb ||
+      read_buffer_samples > 0 ||

[Ken Russell (switch to Gerrit), 2016/09/16 22:32:29]

Per https://bugs.chromium.org/p/chromium/issues/detail?id=634525#c4 , when the
source is a multisampled sRGB renderbuffer and the OpenGL version is less than
4.2, it's necessary to first blit to a temporary sRGB texture, and then use your
SRGBConverter class to blit from that temporary texture to the destination
texture.

It's fine by me to defer this work to a later patch, but could you please add a
TODO(yunchao) here indicating that this code isn't fully correct, and needs
tests of this code path?

[piman, 2016/09/16 22:53:29]

Ah, you're right. I was under the impression that the glCopyTexImage2D would do
the resolve, but that is not the case (it would GL_INVALID_OPERATION)

[Ken Russell (switch to Gerrit), 2016/09/16 23:19:42]

Maybe I'm missing something. This code path only executes BlitFramebuffer and
then returns; I don't see a glCopyTexImage2D being done. Assuming
BlitFramebuffer is not doing the desired thing in all cases from an (sRGB or
not) multisampled renderbuffer to an (sRGB or not) texture for older OpenGL
versions, that's the reason I thought more code would be needed.

[piman, 2016/09/16 23:28:44]

Oh, I thought you were referencing the code in the SRGBConverter which does the
glCopyTexImage2D.

If the source is sRGB, encode_srgb_only is false, so if we're on desktop 4.2 we
don't go through the simple glBlitFramebuffer path (see l.7714), but we use the
SRGBConverter.

Either way, yes this will need to be fixed, so a TODO would be good.

[yunchao, 2016/09/17 15:14:13]

Done. TODO has been added.

Yes, I think the current conformance test doesn't cover this feature. I will
submit corresponding conformance test, and fix the TODO too.  

[yunchao, 2016/09/19 07:25:00]

@piman and @kbr, I revisit this feature. multi-sampled srgb image can not blit
to linear image, and multi-sampled linear image can not blit to srgb image too.
Because if the read buffer is multi-sampled, the format/type of the read buffer
and draw buffer should be exactly the same. 

In addition, the src region and the dst region should be exactly the same too. 

So if the read buffer is multi-sampled, there are only two possible cases: 
1. blit multi-sampled linear image to a single-sampled linear image. (This case
is OK, I think. It is not under the discussion to emulate srgb for desktop OGL)
2. blit multi-sampled srgb image to a single-sampled srgb image (This is the
only case under discussion). 
So maybe we don't need to do any extra work here. Just call blitFramebuffer
directly. Because my code need to do like this: multi-sampled srgb image -
(copy) - srgb image - (decode) - linear image - (blit) - linear image - (encode)
- srgb image. 
But the first step is wrong, we can not copy a multi-sampled image to a
single-sampled image. Copy operation can not do resolution for multi-sampled
image. It will report INVALID_OPERATION. Considering that the src region and the
dst region is exactly the same (no filtering), and both the read buffer and draw
buffer are srgb image (no conversion). I suppose that the only work is resolve
multi-sampled image to a single-sampled image. I think the blitframebuffer
itself can do this underlying. 
WDYT? 
I have a conformance test for this two:
https://github.com/KhronosGroup/WebGL/pull/2034. PTAL. 

+      !feature_info_->feature_flags().desktop_srgb_support ||
+      gl_version_info().IsAtLeastGL(4, 4) ||
+      (gl_version_info().IsAtLeastGL(4, 2) && encode_srgb_only)) {
+    if (enable_srgb && gl_version_info().IsAtLeastGL(4, 2)) {
+      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 images.
+  // The others are not.
+  state_.EnableDisableFramebufferSRGB(true);
+  if (!InitializeSRGBConverter(func_name)) {
+    return;
+  }
+  GLenum src_format =
+      TextureManager::ExtractFormatFromStorageFormat(src_internal_format);
+  srgb_converter_->Blit(this, srcX0, srcY0, srcX1, srcY1,
+                        dstX0, dstY0, dstX1, dstY1,
+                        mask, filter,
+                        GetBoundReadFramebufferSize(),
+                        GetBoundReadFramebufferServiceId(),
+                        src_internal_format, src_format, src_type,
+                        GetBoundDrawFramebufferServiceId(),
+                        read_buffer_has_srgb, draw_buffers_has_srgb);
+}
+
+bool GLES2DecoderImpl::InitializeSRGBConverter(
+    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_->InitializeSRGBConverter(this);

[piman, 2016/09/16 20:49:27]

nit: maybe you can do that in the block above - since it will early-out the
second time it runs.

[yunchao, 2016/09/17 15:14:13]

Done.

+  if (LOCAL_PEEK_GL_ERROR(function_name) != GL_NO_ERROR) {
+    return false;
+  }
+  return true;
 }
 
 void GLES2DecoderImpl::EnsureRenderbufferBound() {
   if (!state_.bound_renderbuffer_valid) {
     state_.bound_renderbuffer_valid = true;
     glBindRenderbufferEXT(GL_RENDERBUFFER,
                           state_.bound_renderbuffer.get()
                               ? state_.bound_renderbuffer->service_id()
                               : 0);
   }
@@ skipping 10284 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