Use serialized gfx::ColorSpace in glResizeCHROMIUM

This capability already exists for glSetColorSpaceMetadataCHROMIUM, so use it for glResizeCHROMIUM, to avoid introducing another class of enums. Rename GLColorSpace to GLcolorSpace for consistency. R=ericrk TBR=avi (chrome/) Bug: 1037954 Change-Id: Ife23c4646d0d5677dd14f4d63c189bf2c93d2eff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985009 Commit-Queue: ccameron <ccameron@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Cr-Commit-Position: refs/heads/master@{#728375}
marcosholgado · Jan 4, 2020 · 4dbe77a · 4dbe77a
1 parent 9338add
commit 4dbe77a
Show file tree

Hide file tree

Showing 53 changed files with 218 additions and 307 deletions.
diff --git a/cc/test/pixel_test_output_surface.cc b/cc/test/pixel_test_output_surface.cc
@@ -15,7 +15,6 @@
 #include "ui/gfx/presentation_feedback.h"
 #include "ui/gfx/swap_result.h"
 #include "ui/gfx/transform.h"
-#include "ui/gl/color_space_utils.h"
 
 namespace cc {
 
@@ -59,7 +58,7 @@ void PixelTestOutputSurface::Reshape(const gfx::Size& size,
   if (context_provider()) {
     context_provider()->ContextGL()->ResizeCHROMIUM(
         size.width(), size.height(), device_scale_factor,
-        gl::ColorSpaceUtils::GetGLColorSpace(color_space), has_alpha);
+        color_space.AsGLColorSpace(), has_alpha);
   } else {
     software_device()->Resize(size, device_scale_factor);
   }

diff --git a/chrome/browser/android/vr/mailbox_to_surface_bridge.cc b/chrome/browser/android/vr/mailbox_to_surface_bridge.cc
@@ -28,6 +28,7 @@
 #include "gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.h"
 #include "gpu/ipc/common/gpu_surface_tracker.h"
 #include "services/viz/public/cpp/gpu/context_provider_command_buffer.h"
+#include "ui/gfx/color_space.h"
 #include "ui/gl/android/surface_texture.h"
 
 #include <android/native_window_jni.h>
@@ -283,8 +284,9 @@ void MailboxToSurfaceBridge::ResizeSurface(int width, int height) {
   }
   DVLOG(1) << __FUNCTION__ << ": resize Surface to " << surface_width_ << "x"
            << surface_height_;
+  gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
   gl_->ResizeCHROMIUM(surface_width_, surface_height_, 1.f,
-                      GL_COLOR_SPACE_UNSPECIFIED_CHROMIUM, false);
+                      color_space.AsGLColorSpace(), false);
   gl_->Viewport(0, 0, surface_width_, surface_height_);
 }
 

diff --git a/components/viz/service/display/gl_renderer.cc b/components/viz/service/display/gl_renderer.cc
@@ -3485,8 +3485,7 @@ void GLRenderer::ScheduleDCLayers() {
     DCHECK(texture_ids[0]);
     // TODO(sunnyps): Set color space in renderer like we do for tiles.
     gl_->SetColorSpaceMetadataCHROMIUM(
-        texture_ids[0],
-        reinterpret_cast<GLColorSpace>(&dc_layer_overlay.color_space));
+        texture_ids[0], dc_layer_overlay.color_space.AsGLColorSpace());
 
     int z_order = dc_layer_overlay.z_order;
     const gfx::Rect& content_rect = dc_layer_overlay.content_rect;

diff --git a/components/viz/service/display/gl_renderer_unittest.cc b/components/viz/service/display/gl_renderer_unittest.cc
@@ -2084,7 +2084,7 @@ class OutputSurfaceMockGLES2Interface : public TestGLES2Interface {
                void(GLuint width,
                     GLuint height,
                     float device_scale,
-                    GLenum color_space,
+                    GLcolorSpace color_space,
                     GLboolean has_alpha));
   MOCK_METHOD4(
       DrawElements,

diff --git a/components/viz/service/display/scoped_gpu_memory_buffer_texture.cc b/components/viz/service/display/scoped_gpu_memory_buffer_texture.cc
@@ -49,8 +49,7 @@ ScopedGpuMemoryBufferTexture::ScopedGpuMemoryBufferTexture(
                                 GL_SCANOUT_CHROMIUM, size_.width(),
                                 size_.height());
   if (color_space_.IsValid()) {
-    gl->SetColorSpaceMetadataCHROMIUM(
-        gl_id_, reinterpret_cast<GLColorSpace>(&color_space_));
+    gl->SetColorSpaceMetadataCHROMIUM(gl_id_, color_space_.AsGLColorSpace());
   }
   gl->BindTexture(target_, 0);
 }

diff --git a/components/viz/service/display_embedder/buffer_queue.cc b/components/viz/service/display_embedder/buffer_queue.cc
@@ -177,8 +177,7 @@ std::unique_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
   gl_->BindTexImage2DCHROMIUM(texture_target_, id);
 
   // The texture must be bound to the image before setting the color space.
-  gl_->SetColorSpaceMetadataCHROMIUM(
-      texture, reinterpret_cast<GLColorSpace>(&color_space_));
+  gl_->SetColorSpaceMetadataCHROMIUM(texture, color_space_.AsGLColorSpace());
 
   return std::make_unique<AllocatedSurface>(this, std::move(buffer), texture,
                                             id, stencil, gfx::Rect(size_));

diff --git a/components/viz/service/display_embedder/gl_output_surface.cc b/components/viz/service/display_embedder/gl_output_surface.cc
@@ -20,7 +20,6 @@
 #include "gpu/command_buffer/common/swap_buffers_complete_params.h"
 #include "gpu/command_buffer/common/swap_buffers_flags.h"
 #include "ui/gfx/overlay_transform_utils.h"
-#include "ui/gl/color_space_utils.h"
 
 namespace viz {
 
@@ -100,7 +99,7 @@ void GLOutputSurface::Reshape(const gfx::Size& size,
   has_set_draw_rectangle_since_last_resize_ = false;
   context_provider()->ContextGL()->ResizeCHROMIUM(
       size.width(), size.height(), device_scale_factor,
-      gl::ColorSpaceUtils::GetGLColorSpace(color_space), has_alpha);
+      color_space.AsGLColorSpace(), has_alpha);
 }
 
 void GLOutputSurface::SwapBuffers(OutputSurfaceFrame frame) {

diff --git a/components/viz/service/display_embedder/skia_output_device_buffer_queue.cc b/components/viz/service/display_embedder/skia_output_device_buffer_queue.cc
@@ -19,7 +19,6 @@
 #include "third_party/skia/include/core/SkSurfaceProps.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/gfx/geometry/rect_conversions.h"
-#include "ui/gl/color_space_utils.h"
 #include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_surface.h"
 

diff --git a/components/viz/service/display_embedder/skia_output_device_gl.cc b/components/viz/service/display_embedder/skia_output_device_gl.cc
@@ -20,7 +20,6 @@
 #include "third_party/skia/include/gpu/GrBackendSurface.h"
 #include "third_party/skia/include/gpu/GrContext.h"
 #include "third_party/skia/include/gpu/gl/GrGLTypes.h"
-#include "ui/gl/color_space_utils.h"
 #include "ui/gl/dc_renderer_layer_params.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"

diff --git a/components/viz/test/fake_output_surface.cc b/components/viz/test/fake_output_surface.cc
@@ -12,7 +12,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/presentation_feedback.h"
 #include "ui/gfx/swap_result.h"
-#include "ui/gl/color_space_utils.h"
 
 namespace viz {
 
@@ -38,7 +37,7 @@ void FakeOutputSurface::Reshape(const gfx::Size& size,
   if (context_provider()) {
     context_provider()->ContextGL()->ResizeCHROMIUM(
         size.width(), size.height(), device_scale_factor,
-        gl::ColorSpaceUtils::GetGLColorSpace(color_space), has_alpha);
+        color_space.AsGLColorSpace(), has_alpha);
   } else {
     software_device()->Resize(size, device_scale_factor);
   }

diff --git a/components/viz/test/test_gles2_interface.cc b/components/viz/test/test_gles2_interface.cc
@@ -425,7 +425,7 @@ GLuint TestGLES2Interface::CreateAndTexStorage2DSharedImageCHROMIUM(
 void TestGLES2Interface::ResizeCHROMIUM(GLuint width,
                                         GLuint height,
                                         float device_scale,
-                                        GLenum color_space,
+                                        GLcolorSpace color_space,
                                         GLboolean has_alpha) {
   reshape_called_ = true;
   width_ = width;

diff --git a/components/viz/test/test_gles2_interface.h b/components/viz/test/test_gles2_interface.h
@@ -93,7 +93,7 @@ class TestGLES2Interface : public gpu::gles2::GLES2InterfaceStub {
   void ResizeCHROMIUM(GLuint width,
                       GLuint height,
                       float device_scale,
-                      GLenum color_space,
+                      GLcolorSpace color_space,
                       GLboolean has_alpha) override;
   void LoseContextCHROMIUM(GLenum current, GLenum other) override;
   GLenum GetGraphicsResetStatusKHR() override;

diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc
@@ -80,7 +80,6 @@
 #include "ui/display/screen.h"
 #include "ui/gfx/ca_layer_params.h"
 #include "ui/gfx/swap_result.h"
-#include "ui/gl/color_space_utils.h"
 #include "ui/latency/latency_tracker.h"
 
 namespace content {

diff --git a/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_resize.txt b/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_resize.txt
@@ -23,46 +23,24 @@ Issues
 
     None
 
-New Tokens
-
-    Accepted by the <color_space> parameter of glResizeCHROMIUM:
-    GL_COLOR_SPACE_UNSPECIFIED_CHROMIUM               0x8AF1
-    GL_COLOR_SPACE_SCRGB_LINEAR_CHROMIUM              0x8AF2
-    GL_COLOR_SPACE_SRGB_CHROMIUM                      0x8AF3
-    GL_COLOR_SPACE_DISPLAY_P3_CHROMIUM                0x8AF4
-    GL_COLOR_SPACE_HDR10_CHROMIUM                     0x8AF5
-
 New Procedures and Functions
 
     The command
 
         glResizeCHROMIUM(GLint width,
                          GLint height,
                          GLfloat scale_factor,
-                         GLenum color_space,
+                         GLcolorSpace color_space,
                          GLboolean alpha);
 
     changes the current output surface to be changed.
     <width> and <height> specify the dimensions for the surface in pixels.
     <scale_factor> specifies the device scale factor for the surface.
     <color_space> specifies the color space in which the pixels of the surface
     should be interpreted by the display system. Note that this value does not
-    impact blending. All blending will be done on the raw pixel values.
-    The values have the following interpretation:
-        GL_COLOR_SPACE_UNSPECIFIED_CHROMIUM: Indicates that the display system
-        should use whatever its default interpretation of color values is.
-        GL_COLOR_SPACE_SRGB_CHROMIUM: Indicates that the display system should
-        interpret output colors as being sRGB values. On EGL-based systems this
-        corresponds to using the default value, EGL_GL_COLORSPACE_LINEAR_KHR,
-        for the EGL_GL_COLORSPACE_KHR property in EGL_KHR_gl_colorspace.
-        GL_COLOR_SPACE_DISPLAY_P3_CHROMIUM: Indicates that the display system
-        should interpret output colors as being in P3 D65 color space. As above,
-        this corresponds to the EGL_GL_COLORSPACE_DISPLAY_P3_LINEAR_EXT value
-        from EGL_EXT_gl_colorspace_display_p3_linear.
-        GL_COLOR_SPACE_SCRGB_LINEAR_CHROMIUM: Indicates that the display system
-        should interpret output colors as being in linear-gamma extended scRGB
-        color space. On Windows, this will result in HDR being enabled for the
-        surface, when possible.
+    impact blending. All blending will be done on the raw pixel values. It is
+    valid to specify nullptr for <color_space>, which will be interpreted as
+    the default (invalid) color space.
     <alpha> indicates whether or not the surface must allocate an alpha channel
 
 Errors

diff --git a/gpu/GLES2/gl2extchromium.h b/gpu/GLES2/gl2extchromium.h
@@ -614,40 +614,20 @@ typedef void (GL_APIENTRYP PFNGLDRAWBUFFERSEXTPROC) (
 /* GL_CHROMIUM_resize */
 #ifndef GL_CHROMIUM_resize
 #define GL_CHROMIUM_resize 1
+typedef const struct _GLcolorSpace* GLcolorSpace;
 #ifdef GL_GLEXT_PROTOTYPES
 GL_APICALL void GL_APIENTRY glResizeCHROMIUM(GLuint width,
                                              GLuint height,
                                              GLfloat scale_factor,
-                                             GLenum color_space,
+                                             GLcolorSpace color_space,
                                              GLboolean alpha);
 
 #endif
 typedef void(GL_APIENTRYP PFNGLRESIZECHROMIUMPROC)(GLuint width,
                                                    GLuint height,
                                                    GLfloat scale_factor,
-                                                   GLenum color_space,
+                                                   GLcolorSpace color_space,
                                                    GLboolean alpha);
-
-#ifndef GL_COLOR_SPACE_UNSPECIFIED_CHROMIUM
-#define GL_COLOR_SPACE_UNSPECIFIED_CHROMIUM 0x8AF1
-#endif
-
-#ifndef GL_COLOR_SPACE_SCRGB_LINEAR_CHROMIUM
-#define GL_COLOR_SPACE_SCRGB_LINEAR_CHROMIUM 0x8AF2
-#endif
-
-#ifndef GL_COLOR_SPACE_SRGB_CHROMIUM
-#define GL_COLOR_SPACE_SRGB_CHROMIUM 0x8AF3
-#endif
-
-#ifndef GL_COLOR_SPACE_DISPLAY_P3_CHROMIUM
-#define GL_COLOR_SPACE_DISPLAY_P3_CHROMIUM 0x8AF4
-#endif
-
-#ifndef GL_COLOR_SPACE_HDR10_CHROMIUM
-#define GL_COLOR_SPACE_HDR10_CHROMIUM 0x8AF5
-#endif
-
 #endif  /* GL_CHROMIUM_resize */
 
 /* GL_CHROMIUM_get_multiple */
@@ -1255,14 +1235,13 @@ typedef void(GL_APIENTRYP PFNGLTEXSTORAGE2DIMAGECHROMIUM)(GLenum target,
 
 #ifndef GL_CHROMIUM_color_space_metadata
 #define GL_CHROMIUM_color_space_metadata 1
-typedef struct _GLColorSpace* GLColorSpace;
 #ifdef GL_GLEXT_PROTOTYPES
 GL_APICALL void GL_APIENTRY
-glSetColorSpaceMetadataCHROMIUM(GLuint texture_id, GLColorSpace color_space);
+glSetColorSpaceMetadataCHROMIUM(GLuint texture_id, GLcolorSpace color_space);
 #endif
 typedef void(GL_APIENTRYP PFNGLSETCOLORSPACEMETADATACHROMIUM)(
     GLuint texture_id,
-    GLColorSpace color_space);
+    GLcolorSpace color_space);
 #endif /* GL_CHROMIUM_color_space_metadata */
 
 /* GL_CHROMIUM_dither_and_premultiply_copy */

diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py
@@ -3652,6 +3652,10 @@
   'ResizeCHROMIUM': {
     'type': 'Custom',
     'impl_func': False,
+    'client_test': False,
+    'cmd_args': 'GLint width, GLint height, GLfloat scale_factor, GLboolean '
+                'alpha, GLuint shm_id, GLuint shm_offset, GLsizei '
+                'color_space_size',
     'extension': True,
     'trace_level': 1,
   },

diff --git a/gpu/command_buffer/client/gles2_c_lib_autogen.h b/gpu/command_buffer/client/gles2_c_lib_autogen.h
@@ -1413,7 +1413,7 @@ void GL_APIENTRY GLES2UnmapTexSubImage2DCHROMIUM(const void* mem) {
 void GL_APIENTRY GLES2ResizeCHROMIUM(GLuint width,
                                      GLuint height,
                                      GLfloat scale_factor,
-                                     GLenum color_space,
+                                     GLcolorSpace color_space,
                                      GLboolean alpha) {
   gles2::GetGLContext()->ResizeCHROMIUM(width, height, scale_factor,
                                         color_space, alpha);
@@ -1932,7 +1932,7 @@ void GL_APIENTRY GLES2TexStorage2DImageCHROMIUM(GLenum target,
                                                    bufferUsage, width, height);
 }
 void GL_APIENTRY GLES2SetColorSpaceMetadataCHROMIUM(GLuint texture_id,
-                                                    GLColorSpace color_space) {
+                                                    GLcolorSpace color_space) {
   gles2::GetGLContext()->SetColorSpaceMetadataCHROMIUM(texture_id, color_space);
 }
 void GL_APIENTRY GLES2WindowRectanglesEXT(GLenum mode,

diff --git a/gpu/command_buffer/client/gles2_cmd_helper_autogen.h b/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
@@ -2701,14 +2701,17 @@ void FlushMappedBufferRange(GLenum target, GLintptr offset, GLsizeiptr size) {
   }
 }
 
-void ResizeCHROMIUM(GLuint width,
-                    GLuint height,
+void ResizeCHROMIUM(GLint width,
+                    GLint height,
                     GLfloat scale_factor,
-                    GLenum color_space,
-                    GLboolean alpha) {
+                    GLboolean alpha,
+                    GLuint shm_id,
+                    GLuint shm_offset,
+                    GLsizei color_space_size) {
   gles2::cmds::ResizeCHROMIUM* c = GetCmdSpace<gles2::cmds::ResizeCHROMIUM>();
   if (c) {
-    c->Init(width, height, scale_factor, color_space, alpha);
+    c->Init(width, height, scale_factor, alpha, shm_id, shm_offset,
+            color_space_size);
   }
 }
 

diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
@@ -5658,18 +5658,18 @@ void GLES2Implementation::ScheduleCALayerCHROMIUM(GLuint contents_texture_id,
 
 void GLES2Implementation::SetColorSpaceMetadataCHROMIUM(
     GLuint texture_id,
-    GLColorSpace color_space) {
+    GLcolorSpace color_space) {
 #if defined(__native_client__)
   // Including gfx::ColorSpace would bring Skia and a lot of other code into
   // NaCl's IRT.
   SetGLError(GL_INVALID_VALUE, "GLES2::SetColorSpaceMetadataCHROMIUM",
              "not supported");
 #else
-  gfx::ColorSpace* gfx_color_space =
-      reinterpret_cast<gfx::ColorSpace*>(color_space);
+  gfx::ColorSpace gfx_color_space;
+  if (color_space)
+    gfx_color_space = *reinterpret_cast<const gfx::ColorSpace*>(color_space);
   base::Pickle color_space_data;
-  IPC::ParamTraits<gfx::ColorSpace>::Write(&color_space_data, *gfx_color_space);
-
+  IPC::ParamTraits<gfx::ColorSpace>::Write(&color_space_data, gfx_color_space);
   ScopedTransferBufferPtr buffer(color_space_data.size(), helper_,
                                  transfer_buffer_);
   if (!buffer.valid() || buffer.size() < color_space_data.size()) {
@@ -6027,12 +6027,35 @@ void GLES2Implementation::UnmapTexSubImage2DCHROMIUM(const void* mem) {
 void GLES2Implementation::ResizeCHROMIUM(GLuint width,
                                          GLuint height,
                                          float scale_factor,
-                                         GLenum color_space,
+                                         GLcolorSpace gl_color_space,
                                          GLboolean alpha) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glResizeCHROMIUM(" << width << ", "
                      << height << ", " << scale_factor << ", " << alpha << ")");
-  helper_->ResizeCHROMIUM(width, height, scale_factor, color_space, alpha);
+  // Including gfx::ColorSpace would bring Skia and a lot of other code into
+  // NaCl's IRT, so just leave the color space unspecified.
+#if !defined(__native_client__)
+  if (gl_color_space) {
+    gfx::ColorSpace gfx_color_space =
+        *reinterpret_cast<const gfx::ColorSpace*>(gl_color_space);
+    base::Pickle color_space_data;
+    IPC::ParamTraits<gfx::ColorSpace>::Write(&color_space_data,
+                                             gfx_color_space);
+    ScopedTransferBufferPtr buffer(color_space_data.size(), helper_,
+                                   transfer_buffer_);
+    if (!buffer.valid() || buffer.size() < color_space_data.size()) {
+      SetGLError(GL_OUT_OF_MEMORY, "GLES2::SetColorSpaceMetadataCHROMIUM",
+                 "out of memory");
+      return;
+    }
+    memcpy(buffer.address(), color_space_data.data(), color_space_data.size());
+    helper_->ResizeCHROMIUM(width, height, scale_factor, alpha, buffer.shm_id(),
+                            buffer.offset(), color_space_data.size());
+    CheckGLError();
+    return;
+  }
+#endif
+  helper_->ResizeCHROMIUM(width, height, scale_factor, alpha, 0, 0, 0);
   CheckGLError();
 }
 

diff --git a/gpu/command_buffer/client/gles2_implementation_autogen.h b/gpu/command_buffer/client/gles2_implementation_autogen.h
@@ -994,7 +994,7 @@ void UnmapTexSubImage2DCHROMIUM(const void* mem) override;
 void ResizeCHROMIUM(GLuint width,
                     GLuint height,
                     GLfloat scale_factor,
-                    GLenum color_space,
+                    GLcolorSpace color_space,
                     GLboolean alpha) override;
 
 const GLchar* GetRequestableExtensionsCHROMIUM() override;
@@ -1366,7 +1366,7 @@ void TexStorage2DImageCHROMIUM(GLenum target,
                                GLsizei height) override;
 
 void SetColorSpaceMetadataCHROMIUM(GLuint texture_id,
-                                   GLColorSpace color_space) override;
+                                   GLcolorSpace color_space) override;
 
 void WindowRectanglesEXT(GLenum mode, GLsizei count, const GLint* box) override;