[110] Fix NativePixmapEGLBinding setting GLImageNativePixmap color space

NOTE: There were conflicts in the cherrypick - see their resolution
between PS1 and PS4.

GLImageNativePixmap::SetColorSpace() sets its |color_space_| ivar.
NativePixmapEGLBinding invokes this method after creating a
GLImageNativePixmap via GLImageNativePixmap::CreateForPlane(). However,
GLImageNativePixmap accesses |color_space_| only in its
InitializeFromNativePixmap() method, which is itself called from
CreateForPlane() following
https://chromium-review.googlesource.com/c/chromium/src/+/4089503
(prior to that CL NativePixmapEGLBinding called CreateForPlane(),
followed by SetColorSpace(), followed by Initialize()).
Hence, the call to GLImageNativePixmap::SetColorSpace() no longer has
the intended effect.

This CL restores the previous behavior by passing |color_space|
directly to CreateForPlane() and threading it through to its only usage
point in GLImageNativePixmap::InitializeFromNativePixmap(). As part of
this fix, we simplify the code by eliminating GLImageNativePixmap's
|color_space_| ivar and SetColorSpace() method - neither of these are
needed. In particular, note that while NativePixmapEGLBinding currently
calls SetColorSpace() only if the color space is valid, the color space
is defined to be valid iff it is not default-constructed. Hence,
passing an invalid (i.e., default-constructed) ColorSpace object in the
new approach has the same effect as the current behavior of not calling
SetColorSpace() in this case and having GLImageNativePixmap operate on
the default-constructed |color_space_| ivar.

Bug: 1310018, 1404994
Change-Id: I78fe34f759924671a9672668224020d566c47bdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4127314
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1089652}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4152272
Cr-Commit-Position: refs/branch-heads/5481@{#200}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

ui/gl/gl_image_native_pixmap.cc

@@ -131,17 +131,18 @@ scoped_refptr<GLImageNativePixmap> GLImageNativePixmap::Create(
     gfx::BufferFormat format,
     scoped_refptr<gfx::NativePixmap> pixmap) {
   return CreateForPlane(size, format, gfx::BufferPlane::DEFAULT,
-                        std::move(pixmap));
+                        std::move(pixmap), gfx::ColorSpace());
 }
 
 scoped_refptr<GLImageNativePixmap> GLImageNativePixmap::CreateForPlane(
     const gfx::Size& size,
     gfx::BufferFormat format,
     gfx::BufferPlane plane,
-    scoped_refptr<gfx::NativePixmap> pixmap) {
+    scoped_refptr<gfx::NativePixmap> pixmap,
+    const gfx::ColorSpace& color_space) {
   auto image =
       base::WrapRefCounted(new GLImageNativePixmap(size, format, plane));
-  if (!image->Initialize(std::move(pixmap))) {
+  if (!image->Initialize(std::move(pixmap), color_space)) {
     return nullptr;
   }
   return image;
@@ -170,7 +171,8 @@ GLImageNativePixmap::GLImageNativePixmap(const gfx::Size& size,
 
 GLImageNativePixmap::~GLImageNativePixmap() {}
 
-bool GLImageNativePixmap::Initialize(scoped_refptr<gfx::NativePixmap> pixmap) {
+bool GLImageNativePixmap::Initialize(scoped_refptr<gfx::NativePixmap> pixmap,
+                                     const gfx::ColorSpace& color_space) {
   DCHECK(!pixmap_);
   if (GLInternalFormat(format_) == GL_NONE) {
     LOG(ERROR) << "Unsupported format: " << gfx::BufferFormatToString(format_);
@@ -205,7 +207,7 @@ bool GLImageNativePixmap::Initialize(scoped_refptr<gfx::NativePixmap> pixmap) {
       // promoted to overlays). We'll need to revisit this once we plumb the
       // color space and range to DRM/KMS.
       attrs.push_back(EGL_YUV_COLOR_SPACE_HINT_EXT);
-      switch (color_space_.GetMatrixID()) {
+      switch (color_space.GetMatrixID()) {
         case gfx::ColorSpace::MatrixID::BT2020_NCL:
           attrs.push_back(EGL_ITU_REC2020_EXT);
           break;
@@ -214,7 +216,7 @@ bool GLImageNativePixmap::Initialize(scoped_refptr<gfx::NativePixmap> pixmap) {
       }
 
       attrs.push_back(EGL_SAMPLE_RANGE_HINT_EXT);
-      switch (color_space_.GetRangeID()) {
+      switch (color_space.GetRangeID()) {
         case gfx::ColorSpace::RangeID::FULL:
           attrs.push_back(EGL_YUV_FULL_RANGE_EXT);
           break;

ui/gl/gl_image_native_pixmap.h

@@ -23,12 +23,16 @@ class GL_EXPORT GLImageNativePixmap : public gl::GLImageEGL {
       gfx::BufferFormat format,
       scoped_refptr<gfx::NativePixmap> pixmap);
 
-  // Create an EGLImage from a given NativePixmap and plane.
+  // Create an EGLImage from a given NativePixmap and plane. The color space is
+  // for the external sampler: When we sample the YUV buffer as RGB, we need to
+  // tell it the encoding (BT.601, BT.709, or BT.2020) and range (limited or
+  // null), and |color_space| conveys this.
   static scoped_refptr<GLImageNativePixmap> CreateForPlane(
       const gfx::Size& size,
       gfx::BufferFormat format,
       gfx::BufferPlane plane,
-      scoped_refptr<gfx::NativePixmap> pixmap);
+      scoped_refptr<gfx::NativePixmap> pixmap,
+      const gfx::ColorSpace& color_space);
   // Create an EGLImage from a given GL texture.
   static scoped_refptr<GLImageNativePixmap> CreateFromTexture(
       const gfx::Size& size,
@@ -59,7 +63,8 @@ class GL_EXPORT GLImageNativePixmap : public gl::GLImageEGL {
                       gfx::BufferFormat format,
                       gfx::BufferPlane plane);
   // Create an EGLImage from a given NativePixmap.
-  bool Initialize(scoped_refptr<gfx::NativePixmap> pixmap);
+  bool Initialize(scoped_refptr<gfx::NativePixmap> pixmap,
+                  const gfx::ColorSpace& color_space);
   // Create an EGLImage from a given GL texture.
   bool InitializeFromTexture(uint32_t texture_id);
 

ui/ozone/common/native_pixmap_egl_binding.cc

@@ -23,15 +23,12 @@ std::unique_ptr<NativePixmapGLBinding> NativePixmapEGLBinding::Create(
     GLenum target,
     GLuint texture_id) {
   auto gl_image = gl::GLImageNativePixmap::CreateForPlane(
-      plane_size, plane_format, plane, std::move(pixmap));
+      plane_size, plane_format, plane, std::move(pixmap), color_space);
   if (!gl_image) {
     LOG(ERROR) << "Unable to initialize GL image from pixmap";
     return nullptr;
   }
 
-  if (color_space.IsValid())
-    gl_image->SetColorSpace(color_space);
-
   auto binding = std::make_unique<NativePixmapEGLBinding>();
   if (!binding->BindTexture(std::move(gl_image), target, texture_id)) {
     return nullptr;