From 870caf4d48738256593ab12d80db2f0d8012b116 Mon Sep 17 00:00:00 2001
From: Ken Russell <[email protected]>
Date: Tue, 14 Jul 2026 19:04:38 -0700
Subject: [PATCH] gpu: Unbind unpack buffer for CopyTextureCHROMIUM intermediate

When Copy{Sub}TextureCHROMIUM takes the DRAW_AND_COPY or
DRAW_AND_READBACK path, the validating decoder allocates an intermediate
GL_TEXTURE_2D via glTexImage2D(..., nullptr). The client's
GL_PIXEL_UNPACK_BUFFER binding is still applied to the driver at this
point, so the driver sources the intermediate texture from the client's
buffer instead of allocating fresh storage. With an undersized client
buffer this fails with GL_INVALID_OPERATION, leaving the intermediate
texture without storage and the subsequent draw / readback operating on
an incomplete framebuffer.

Unbind GL_PIXEL_UNPACK_BUFFER before allocating the intermediate texture
in DoCopyTexture and DoCopySubTexture, gated on IsES3Capable() to match
the condition under which RestoreBufferBindings() restores the binding
afterwards.

Add a gl_tests regression test that binds a tiny unpack buffer, performs
a copy that requires an intermediate texture, verifies the destination
receives the source pixels, and checks that the client binding is
preserved.

Fixed: 523713150
Change-Id: I42989c54ecb1a71be228bd457a1378d003feb1e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8088997
Reviewed-by: Zhenyao Mo <[email protected]>
Commit-Queue: Kenneth Russell <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1662326}
---

diff --git a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
index fda1e42..16d4f37 100644
--- a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
+++ b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
@@ -1119,7 +1119,11 @@
         adjusted_internal_format);
     GLenum type =
         TextureManager::ExtractTypeFromStorageFormat(adjusted_internal_format);
-
+    // Allocate from client memory, not from any currently bound unpack
+    // buffer. The binding is restored by the calls below.
+    if (decoder->GetFeatureInfo()->IsES3Capable()) {
+      glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+    }
     glTexImage2D(dest_target, 0, adjusted_internal_format, width, height, 0,
                  format, type, nullptr);
     dest_texture = intermediate_texture;
@@ -1204,6 +1208,11 @@
         adjusted_internal_format);
     GLenum type =
         TextureManager::ExtractTypeFromStorageFormat(adjusted_internal_format);
+    // Allocate from client memory, not from any currently bound unpack
+    // buffer. The binding is restored by the calls below.
+    if (decoder->GetFeatureInfo()->IsES3Capable()) {
+      glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+    }
     glTexImage2D(dest_target, 0, adjusted_internal_format, width, height, 0,
                  format, type, nullptr);
     dest_texture = intermediate_texture;
diff --git a/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc b/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
index 28135403..6d564708 100644
--- a/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
+++ b/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
@@ -1948,4 +1948,104 @@
 #endif  // !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
 }
 
+// A bound GL_PIXEL_UNPACK_BUFFER must not be picked up when allocating the
+// intermediate texture used by the DRAW_AND_COPY / DRAW_AND_READBACK paths.
+TEST_P(GLCopyTextureCHROMIUMES3Test, PixelUnpackBufferDoesNotInterfere) {
+#if !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
+  GTEST_SKIP() << "Test only reproduces with validating decoder";
+#else
+  if (gl_.gpu_preferences().use_passthrough_cmd_decoder) {
+    GTEST_SKIP() << "Skipping test because it's run with the passthrough "
+                    "command decoder";
+  }
+
+  if (!gl_.IsInitialized()) {
+    GTEST_SKIP() << "ES3 context unavailable";
+  }
+
+#if BUILDFLAG(IS_ANDROID) && defined(ARCH_CPU_X86_FAMILY)
+  GTEST_SKIP() << "Skipping test on Android x86/x64";
+#else
+
+  constexpr GLsizei kW = 8, kH = 8;
+  const uint8_t kGreen[4] = {0u, 255u, 0u, 255u};
+
+  GLuint src;
+  glGenTextures(1, &src);
+  glBindTexture(GL_TEXTURE_2D, src);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  std::vector<uint8_t> green(kW * kH * 4);
+  for (size_t i = 0; i < green.size(); i += 4) {
+    green[i + 0] = kGreen[0];
+    green[i + 1] = kGreen[1];
+    green[i + 2] = kGreen[2];
+    green[i + 3] = kGreen[3];
+  }
+  glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kW, kH, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+               green.data());
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // Cube-map destination so the copy goes through an intermediate texture.
+  GLuint dest;
+  glGenTextures(1, &dest);
+  glBindTexture(GL_TEXTURE_CUBE_MAP, dest);
+  glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+  glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  std::vector<uint8_t> zeros(kW * kH * 4, 0);
+  for (int face = 0; face < 6; ++face) {
+    glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, 0, GL_RGBA8, kW, kH, 0,
+                 GL_RGBA, GL_UNSIGNED_BYTE, zeros.data());
+  }
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // Bind a tiny pixel-unpack buffer and leave it bound across the copy. The
+  // copy must not source the intermediate texture's storage from it.
+  GLuint pbo;
+  glGenBuffers(1, &pbo);
+  glBindBuffer(GL_PIXEL_UNPACK_BUFFER, pbo);
+  const uint8_t kByte = 0;
+  glBufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(kByte), &kByte, GL_STATIC_DRAW);
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // flip_y forces a draw-based path; cube-map dest requires an intermediate.
+  CopyType copy_type = GetParam();
+  if (copy_type == TexImage) {
+    glCopyTextureCHROMIUM(src, 0, GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0,
+                          GL_RGBA8, GL_UNSIGNED_BYTE, GL_TRUE, GL_FALSE,
+                          GL_FALSE);
+  } else {
+    glCopySubTextureCHROMIUM(src, 0, GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0, 0,
+                             0, 0, 0, kW, kH, GL_TRUE, GL_FALSE, GL_FALSE);
+  }
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // The client binding must be preserved.
+  GLint bound_pbo = 0;
+  glGetIntegerv(GL_PIXEL_UNPACK_BUFFER_BINDING, &bound_pbo);
+  EXPECT_EQ(pbo, static_cast<GLuint>(bound_pbo));
+
+  glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+
+  GLuint fbo;
+  glGenFramebuffers(1, &fbo);
+  glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+  glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                         GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0);
+  ASSERT_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE),
+            glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+  EXPECT_TRUE(GLTestHelper::CheckPixels(0, 0, kW, kH, 0, kGreen, nullptr));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  glDeleteFramebuffers(1, &fbo);
+  glDeleteBuffers(1, &pbo);
+  glDeleteTextures(1, &src);
+  glDeleteTextures(1, &dest);
+#endif  // BUILDFLAG(IS_ANDROID) && defined(ARCH_CPU_X86_FAMILY)
+#endif  // !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
+}
+
 }  // namespace gpu

diff --git a/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc b/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
index 28135403..6d564708 100644
--- a/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
+++ b/gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc
@@ -1948,4 +1948,104 @@
 #endif  // !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
 }
 
+// A bound GL_PIXEL_UNPACK_BUFFER must not be picked up when allocating the
+// intermediate texture used by the DRAW_AND_COPY / DRAW_AND_READBACK paths.
+TEST_P(GLCopyTextureCHROMIUMES3Test, PixelUnpackBufferDoesNotInterfere) {
+#if !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
+  GTEST_SKIP() << "Test only reproduces with validating decoder";
+#else
+  if (gl_.gpu_preferences().use_passthrough_cmd_decoder) {
+    GTEST_SKIP() << "Skipping test because it's run with the passthrough "
+                    "command decoder";
+  }
+
+  if (!gl_.IsInitialized()) {
+    GTEST_SKIP() << "ES3 context unavailable";
+  }
+
+#if BUILDFLAG(IS_ANDROID) && defined(ARCH_CPU_X86_FAMILY)
+  GTEST_SKIP() << "Skipping test on Android x86/x64";
+#else
+
+  constexpr GLsizei kW = 8, kH = 8;
+  const uint8_t kGreen[4] = {0u, 255u, 0u, 255u};
+
+  GLuint src;
+  glGenTextures(1, &src);
+  glBindTexture(GL_TEXTURE_2D, src);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  std::vector<uint8_t> green(kW * kH * 4);
+  for (size_t i = 0; i < green.size(); i += 4) {
+    green[i + 0] = kGreen[0];
+    green[i + 1] = kGreen[1];
+    green[i + 2] = kGreen[2];
+    green[i + 3] = kGreen[3];
+  }
+  glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kW, kH, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+               green.data());
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // Cube-map destination so the copy goes through an intermediate texture.
+  GLuint dest;
+  glGenTextures(1, &dest);
+  glBindTexture(GL_TEXTURE_CUBE_MAP, dest);
+  glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+  glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  std::vector<uint8_t> zeros(kW * kH * 4, 0);
+  for (int face = 0; face < 6; ++face) {
+    glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, 0, GL_RGBA8, kW, kH, 0,
+                 GL_RGBA, GL_UNSIGNED_BYTE, zeros.data());
+  }
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // Bind a tiny pixel-unpack buffer and leave it bound across the copy. The
+  // copy must not source the intermediate texture's storage from it.
+  GLuint pbo;
+  glGenBuffers(1, &pbo);
+  glBindBuffer(GL_PIXEL_UNPACK_BUFFER, pbo);
+  const uint8_t kByte = 0;
+  glBufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(kByte), &kByte, GL_STATIC_DRAW);
+  ASSERT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // flip_y forces a draw-based path; cube-map dest requires an intermediate.
+  CopyType copy_type = GetParam();
+  if (copy_type == TexImage) {
+    glCopyTextureCHROMIUM(src, 0, GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0,
+                          GL_RGBA8, GL_UNSIGNED_BYTE, GL_TRUE, GL_FALSE,
+                          GL_FALSE);
+  } else {
+    glCopySubTextureCHROMIUM(src, 0, GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0, 0,
+                             0, 0, 0, kW, kH, GL_TRUE, GL_FALSE, GL_FALSE);
+  }
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  // The client binding must be preserved.
+  GLint bound_pbo = 0;
+  glGetIntegerv(GL_PIXEL_UNPACK_BUFFER_BINDING, &bound_pbo);
+  EXPECT_EQ(pbo, static_cast<GLuint>(bound_pbo));
+
+  glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+
+  GLuint fbo;
+  glGenFramebuffers(1, &fbo);
+  glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+  glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                         GL_TEXTURE_CUBE_MAP_POSITIVE_X, dest, 0);
+  ASSERT_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE),
+            glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+  EXPECT_TRUE(GLTestHelper::CheckPixels(0, 0, kW, kH, 0, kGreen, nullptr));
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
+
+  glDeleteFramebuffers(1, &fbo);
+  glDeleteBuffers(1, &pbo);
+  glDeleteTextures(1, &src);
+  glDeleteTextures(1, &dest);
+#endif  // BUILDFLAG(IS_ANDROID) && defined(ARCH_CPU_X86_FAMILY)
+#endif  // !BUILDFLAG(ENABLE_VALIDATING_COMMAND_DECODER)
+}
+
 }  // namespace gpu