commit 69c94adeb4da95573263e66510446b6bcccf650a
author Stephen White <senorblanco@chromium.org> Tue Sep 05 18:11:56 2023 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Sep 05 18:11:56 2023 +0000
tree 7f154ce01b526b276313630fba78538ae4655642
parent a985f73277b71dd33393652d011e9389640bfe02

OpenGL: fix single-plane external textures.

When bind group layouts for external textures are created, they
initially use invalid binding points of kMaxBindingsPerBindGroup, in
order avoid collisions. Replace those binding points with the
valid external texture binding expansion points prior to GL shader
compilation, as is done on other backends.

This fixes single-plane external textures. Multiplane external textures
are still broken, likely due to a conflict between the
MultiplanarExternalTexture transform and the CombineSamplers transform
used by the GLSL writer.

Bug: dawn:1774
Change-Id: Iabc108f4ee5fe831bd7e4e4639119095b7f9e4a2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/149500
Commit-Queue: Stephen White <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/opengl/ShaderModuleGL.cpp

diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index c7028ba..fb69ef4 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -167,10 +167,11 @@
     std::unordered_map<BindingPoint, BindingPoint> glBindings;
     for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
         const BindGroupLayoutInternalBase* bgl = layout->GetBindGroupLayout(group);
+        const auto& indices = layout->GetBindingIndexInfo()[group];
         const auto& groupBindingInfo = moduleBindingInfo[group];
         for (const auto& [bindingNumber, bindingInfo] : groupBindingInfo) {
             BindingIndex bindingIndex = bgl->GetBindingIndex(bindingNumber);
-            GLuint shaderIndex = layout->GetBindingIndexInfo()[group][bindingIndex];
+            GLuint shaderIndex = indices[bindingIndex];
             BindingPoint srcBindingPoint{static_cast<uint32_t>(group),
                                          static_cast<uint32_t>(bindingNumber)};
             BindingPoint dstBindingPoint{0, shaderIndex};
@@ -178,6 +179,17 @@
                 glBindings.emplace(srcBindingPoint, dstBindingPoint);
             }
         }
+
+        for (const auto& [_, expansion] : bgl->GetExternalTextureBindingExpansionMap()) {
+            uint32_t plane1Slot = indices[bgl->GetBindingIndex(expansion.plane1)];
+            uint32_t paramsSlot = indices[bgl->GetBindingIndex(expansion.params)];
+            glBindings.emplace(
+                BindingPoint{static_cast<uint32_t>(group), static_cast<uint32_t>(expansion.plane1)},
+                BindingPoint{0u, plane1Slot});
+            glBindings.emplace(
+                BindingPoint{static_cast<uint32_t>(group), static_cast<uint32_t>(expansion.params)},
+                BindingPoint{0u, paramsSlot});
+        }
     }
 
     // Some texture builtin functions are unsupported on GLSL ES. These are emulated with internal

src/dawn/tests/end2end/ExternalTextureTests.cpp

diff --git a/src/dawn/tests/end2end/ExternalTextureTests.cpp b/src/dawn/tests/end2end/ExternalTextureTests.cpp
index 06cb928..c0af3a6 100644
--- a/src/dawn/tests/end2end/ExternalTextureTests.cpp
+++ b/src/dawn/tests/end2end/ExternalTextureTests.cpp
@@ -143,10 +143,6 @@
 }
 
 TEST_P(ExternalTextureTests, SampleExternalTexture) {
-    // TODO(crbug.com/tint/1774): Tint has an issue compiling shaders that use external textures on
-    // OpenGL/OpenGLES.
-    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
-
     wgpu::Texture sampledTexture =
         Create2DTexture(device, kWidth, kHeight, kFormat,
                         wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::RenderAttachment);
@@ -407,10 +403,6 @@
 // square in the lower left and a blue square in the lower right. The image is then sampled as an
 // external texture and rotated 0, 90, 180, and 270 degrees with and without the y-axis flipped.
 TEST_P(ExternalTextureTests, RotateAndOrFlipSinglePlane) {
-    // TODO(crbug.com/tint/1774): Tint has an issue compiling shaders that use external textures on
-    // OpenGL/OpenGLES.
-    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
-
     const wgpu::ShaderModule sourceTextureFsModule = utils::CreateShaderModule(device, R"(
         @fragment fn main(@builtin(position) FragCoord : vec4f)
                                  -> @location(0) vec4f {
@@ -754,10 +746,6 @@
 // This test draws a 2x2 multi-colored square surrounded by a 1px black border. We test the external
 // texture crop functionality by cropping to specific ranges inside the texture.
 TEST_P(ExternalTextureTests, CropSinglePlane) {
-    // TODO(crbug.com/tint/1774): Tint has an issue compiling shaders that use external textures on
-    // OpenGL/OpenGLES.
-    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
-
     const wgpu::ShaderModule sourceTextureFsModule = utils::CreateShaderModule(device, R"(
         @fragment fn main(@builtin(position) FragCoord : vec4f)
                                  -> @location(0) vec4f {
@@ -1109,10 +1097,6 @@
 
 // Test that sampling an external texture with non-one alpha preserves the alpha channel.
 TEST_P(ExternalTextureTests, SampleExternalTextureAlpha) {
-    // TODO(crbug.com/tint/1774): Tint has an issue compiling shaders that use external textures on
-    // OpenGL/OpenGLES.
-    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
-
     wgpu::Texture sampledTexture =
         Create2DTexture(device, kWidth, kHeight, kFormat,
                         wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::RenderAttachment);