Fix SSBO bindings in GLES backend.

OpenGL ES doesn't have the glShaderStorageBlockBinding() call, so we
modify the binding decorations set in the shader instead. This requires
plumbing through some more pipeline state at shader translation time.

BUG=dawn:584
Change-Id: Ib7fdb6a7ad1eff1a99d44d55e9d923214affe702
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/34400
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>

src/dawn_native/opengl/PipelineGL.cpp

@@ -85,7 +85,7 @@ namespace dawn_native { namespace opengl {
         for (SingleShaderStage stage : IterateStages(activeStages)) {
             const ShaderModule* module = ToBackend(stages[stage].module.Get());
             std::string glsl = module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage,
-                                                       &combinedSamplers[stage]);
+                                                       &combinedSamplers[stage], layout);
             GLuint shader = CreateShader(gl, GLShaderType(stage), glsl.c_str());
             gl.AttachShader(mProgram, shader);
         }
@@ -131,14 +131,15 @@ namespace dawn_native { namespace opengl {
 
                     case wgpu::BindingType::StorageBuffer:
                     case wgpu::BindingType::ReadonlyStorageBuffer: {
-                        GLuint location = gl.GetProgramResourceIndex(
+                        // Since glShaderStorageBlockBinding doesn't exist in OpenGL ES, we skip
-                            mProgram, GL_SHADER_STORAGE_BLOCK, name.c_str());
+                        // that call and handle it during shader translation by modifying the
-                        if (location != GL_INVALID_INDEX) {
+                        // location decoration.
-                            if (gl.GetVersion().IsES()) {
+                        // Contrary to all other binding types, OpenGL ES's SSBO binding index in
-                                // TODO(crbug.com/dawn/584): Figure out a substitute for
+                        // the SSBO table is the value of the location= decoration in GLSL.
-                                // glShaderStorageBlockBinding on ES or add additional validation.
+                        if (gl.GetVersion().IsDesktop()) {
-                                ASSERT(false);
+                            GLuint location = gl.GetProgramResourceIndex(
-                            } else {
+                                mProgram, GL_SHADER_STORAGE_BLOCK, name.c_str());
+                            if (location != GL_INVALID_INDEX) {
                                 gl.ShaderStorageBlockBinding(mProgram, location,
                                                              indices[group][bindingIndex]);
                             }

src/dawn_native/opengl/ShaderModuleGL.cpp

@@ -16,8 +16,10 @@
 
 #include "common/Assert.h"
 #include "common/Platform.h"
+#include "dawn_native/BindGroupLayout.h"
 #include "dawn_native/SpirvUtils.h"
 #include "dawn_native/opengl/DeviceGL.h"
+#include "dawn_native/opengl/PipelineLayoutGL.h"
 
 #include <spirv_glsl.hpp>
 
@@ -65,7 +67,8 @@ namespace dawn_native { namespace opengl {
 
     std::string ShaderModule::TranslateToGLSL(const char* entryPointName,
                                               SingleShaderStage stage,
-                                              CombinedSamplerInfo* combinedSamplers) const {
+                                              CombinedSamplerInfo* combinedSamplers,
+                                              const PipelineLayout* layout) const {
         // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to
         // be updated.
         spirv_cross::CompilerGLSL::Options options;
@@ -134,8 +137,19 @@ namespace dawn_native { namespace opengl {
                 }
 
                 compiler.set_name(resourceId, GetBindingName(group, bindingNumber));
-                compiler.unset_decoration(info.id, spv::DecorationBinding);
                 compiler.unset_decoration(info.id, spv::DecorationDescriptorSet);
+                // OpenGL ES has no glShaderStorageBlockBinding call, so we adjust the SSBO binding
+                // decoration here instead.
+                if (version.IsES() && (info.type == wgpu::BindingType::StorageBuffer ||
+                                       info.type == wgpu::BindingType::ReadonlyStorageBuffer)) {
+                    const auto& indices = layout->GetBindingIndexInfo();
+                    BindingIndex bindingIndex =
+                        layout->GetBindGroupLayout(group)->GetBindingIndex(bindingNumber);
+                    compiler.set_decoration(info.id, spv::DecorationBinding,
+                                            indices[group][bindingIndex]);
+                } else {
+                    compiler.unset_decoration(info.id, spv::DecorationBinding);
+                }
             }
         }
 

src/dawn_native/opengl/ShaderModuleGL.h

@@ -22,6 +22,7 @@
 namespace dawn_native { namespace opengl {
 
     class Device;
+    class PipelineLayout;
 
     std::string GetBindingName(BindGroupIndex group, BindingNumber bindingNumber);
 
@@ -47,7 +48,8 @@ namespace dawn_native { namespace opengl {
 
         std::string TranslateToGLSL(const char* entryPointName,
                                     SingleShaderStage stage,
-                                    CombinedSamplerInfo* combinedSamplers) const;
+                                    CombinedSamplerInfo* combinedSamplers,
+                                    const PipelineLayout* layout) const;
 
       private:
         ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor);

src/tests/end2end/BindGroupTests.cpp

@@ -1095,6 +1095,7 @@ TEST_P(BindGroupTests, ReadonlyStorage) {
 // used correctly. The test loads a different value from each binding, and writes 1 to a storage
 // buffer if all values are correct.
 TEST_P(BindGroupTests, ReallyLargeBindGroup) {
+    DAWN_SKIP_TEST_IF(IsOpenGLES());
     std::string interface = "#version 450\n";
     std::string body;
     uint32_t binding = 0;
@@ -1232,4 +1233,5 @@ DAWN_INSTANTIATE_TEST(BindGroupTests,
                       D3D12Backend(),
                       MetalBackend(),
                       OpenGLBackend(),
+                      OpenGLESBackend(),
                       VulkanBackend());