From 2395ff5be170bc96d9c92c198a1ab153edd5671e Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Wed, 2 Dec 2020 16:51:19 +0000 Subject: [PATCH] OpenGL: Bind a dummy sampler for OpImageFetch if not present In WGSL, textureLoad translates to an OpImageFetch without the combined sampler. In OpenGL, we need to use SPIRV-Cross to insert a dummy nearest filtering sampler and bind that in the backend. Bug: dawn:585 Change-Id: I92ae6ad35263d3720e59fa93688ca914a9495a81 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/34401 Reviewed-by: Stephen White Reviewed-by: Austin Eng Commit-Queue: Austin Eng --- src/dawn_native/opengl/PipelineGL.cpp | 42 +++++++++++++++++------ src/dawn_native/opengl/PipelineGL.h | 6 ++++ src/dawn_native/opengl/ShaderModuleGL.cpp | 37 +++++++++++++++----- src/dawn_native/opengl/ShaderModuleGL.h | 6 +++- 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp index ee4aa5c0c5..3396c495df 100644 --- a/src/dawn_native/opengl/PipelineGL.cpp +++ b/src/dawn_native/opengl/PipelineGL.cpp @@ -17,10 +17,12 @@ #include "common/BitSetIterator.h" #include "common/Log.h" #include "dawn_native/BindGroupLayout.h" +#include "dawn_native/Device.h" #include "dawn_native/Pipeline.h" #include "dawn_native/opengl/Forward.h" #include "dawn_native/opengl/OpenGLFunctions.h" #include "dawn_native/opengl/PipelineLayoutGL.h" +#include "dawn_native/opengl/SamplerGL.h" #include "dawn_native/opengl/ShaderModuleGL.h" #include @@ -42,8 +44,8 @@ namespace dawn_native { namespace opengl { } // namespace - PipelineGL::PipelineGL() { - } + PipelineGL::PipelineGL() = default; + PipelineGL::~PipelineGL() = default; void PipelineGL::Initialize(const OpenGLFunctions& gl, const PipelineLayout* layout, @@ -82,14 +84,25 @@ namespace dawn_native { namespace opengl { // Create an OpenGL shader for each stage and gather the list of combined samplers. PerStage combinedSamplers; + bool needsDummySampler = false; 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], layout); + std::string glsl = + module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage, + &combinedSamplers[stage], layout, &needsDummySampler); GLuint shader = CreateShader(gl, GLShaderType(stage), glsl.c_str()); gl.AttachShader(mProgram, shader); } + if (needsDummySampler) { + SamplerDescriptor desc = {}; + ASSERT(desc.minFilter == wgpu::FilterMode::Nearest); + ASSERT(desc.magFilter == wgpu::FilterMode::Nearest); + ASSERT(desc.mipmapFilter == wgpu::FilterMode::Nearest); + mDummySampler = AcquireRef( + ToBackend(layout->GetDevice()->GetOrCreateSampler(&desc).AcquireSuccess())); + } + // Link all the shaders together. gl.LinkProgram(mProgram); @@ -204,13 +217,18 @@ namespace dawn_native { namespace opengl { wgpu::TextureComponentType::Float; } { - const BindGroupLayoutBase* bgl = - layout->GetBindGroupLayout(combined.samplerLocation.group); - BindingIndex bindingIndex = - bgl->GetBindingIndex(combined.samplerLocation.binding); + if (combined.useDummySampler) { + mDummySamplerUnits.push_back(textureUnit); + } else { + const BindGroupLayoutBase* bgl = + layout->GetBindGroupLayout(combined.samplerLocation.group); + BindingIndex bindingIndex = + bgl->GetBindingIndex(combined.samplerLocation.binding); - GLuint samplerIndex = indices[combined.samplerLocation.group][bindingIndex]; - mUnitsForSamplers[samplerIndex].push_back({textureUnit, shouldUseFiltering}); + GLuint samplerIndex = indices[combined.samplerLocation.group][bindingIndex]; + mUnitsForSamplers[samplerIndex].push_back( + {textureUnit, shouldUseFiltering}); + } } textureUnit++; @@ -235,6 +253,10 @@ namespace dawn_native { namespace opengl { void PipelineGL::ApplyNow(const OpenGLFunctions& gl) { gl.UseProgram(mProgram); + for (GLuint unit : mDummySamplerUnits) { + ASSERT(mDummySampler.Get() != nullptr); + gl.BindSampler(unit, mDummySampler->GetNonFilteringHandle()); + } } }} // namespace dawn_native::opengl diff --git a/src/dawn_native/opengl/PipelineGL.h b/src/dawn_native/opengl/PipelineGL.h index ffef4d2e95..1d995d0e8a 100644 --- a/src/dawn_native/opengl/PipelineGL.h +++ b/src/dawn_native/opengl/PipelineGL.h @@ -31,10 +31,12 @@ namespace dawn_native { namespace opengl { struct OpenGLFunctions; class PersistentPipelineState; class PipelineLayout; + class Sampler; class PipelineGL { public: PipelineGL(); + ~PipelineGL(); void Initialize(const OpenGLFunctions& gl, const PipelineLayout* layout, @@ -56,6 +58,10 @@ namespace dawn_native { namespace opengl { GLuint mProgram; std::vector> mUnitsForSamplers; std::vector> mUnitsForTextures; + std::vector mDummySamplerUnits; + // TODO(enga): This could live on the Device, or elsewhere, but currently it makes Device + // destruction complex as it requires the sampler to be destroyed before the sampler cache. + Ref mDummySampler; }; }} // namespace dawn_native::opengl diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index ff2ecafbf2..792a2fc84f 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -39,15 +39,19 @@ namespace dawn_native { namespace opengl { } bool operator<(const CombinedSampler& a, const CombinedSampler& b) { - return std::tie(a.samplerLocation, a.textureLocation) < - std::tie(b.samplerLocation, b.textureLocation); + return std::tie(a.useDummySampler, a.samplerLocation, a.textureLocation) < + std::tie(b.useDummySampler, a.samplerLocation, b.textureLocation); } std::string CombinedSampler::GetName() const { std::ostringstream o; o << "dawn_combined"; - o << "_" << static_cast(samplerLocation.group) << "_" - << static_cast(samplerLocation.binding); + if (useDummySampler) { + o << "_dummy_sampler"; + } else { + o << "_" << static_cast(samplerLocation.group) << "_" + << static_cast(samplerLocation.binding); + } o << "_with_" << static_cast(textureLocation.group) << "_" << static_cast(textureLocation.binding); return o.str(); @@ -68,7 +72,8 @@ namespace dawn_native { namespace opengl { std::string ShaderModule::TranslateToGLSL(const char* entryPointName, SingleShaderStage stage, CombinedSamplerInfo* combinedSamplers, - const PipelineLayout* layout) const { + const PipelineLayout* layout, + bool* needsDummySampler) const { // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to // be updated. spirv_cross::CompilerGLSL::Options options; @@ -92,6 +97,13 @@ namespace dawn_native { namespace opengl { compiler.set_common_options(options); compiler.set_entry_point(entryPointName, ShaderStageToExecutionModel(stage)); + // Analyzes all OpImageFetch opcodes and checks if there are instances where + // said instruction is used without a combined image sampler. + // GLSL does not support texelFetch without a sampler. + // To workaround this, we must inject a dummy sampler which can be used to form a sampler2D + // at the call-site of texelFetch as necessary. + spirv_cross::VariableID dummySamplerId = compiler.build_dummy_sampler_for_combined_images(); + // Extract bindings names so that it can be used to get its location in program. // Now translate the separate sampler / textures into combined ones and store their info. We // need to do this before removing the set and binding decorations. @@ -101,10 +113,17 @@ namespace dawn_native { namespace opengl { combinedSamplers->emplace_back(); CombinedSampler* info = &combinedSamplers->back(); - info->samplerLocation.group = BindGroupIndex( - compiler.get_decoration(combined.sampler_id, spv::DecorationDescriptorSet)); - info->samplerLocation.binding = - BindingNumber(compiler.get_decoration(combined.sampler_id, spv::DecorationBinding)); + if (combined.sampler_id == dummySamplerId) { + *needsDummySampler = true; + info->useDummySampler = true; + info->samplerLocation = {}; + } else { + info->useDummySampler = false; + info->samplerLocation.group = BindGroupIndex( + compiler.get_decoration(combined.sampler_id, spv::DecorationDescriptorSet)); + info->samplerLocation.binding = BindingNumber( + compiler.get_decoration(combined.sampler_id, spv::DecorationBinding)); + } info->textureLocation.group = BindGroupIndex( compiler.get_decoration(combined.image_id, spv::DecorationDescriptorSet)); info->textureLocation.binding = diff --git a/src/dawn_native/opengl/ShaderModuleGL.h b/src/dawn_native/opengl/ShaderModuleGL.h index 65456dd7ba..c18fa48cf5 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.h +++ b/src/dawn_native/opengl/ShaderModuleGL.h @@ -35,6 +35,9 @@ namespace dawn_native { namespace opengl { struct CombinedSampler { BindingLocation samplerLocation; BindingLocation textureLocation; + // OpenGL requires a sampler with texelFetch. If this is true, the developer did not provide + // one and Dawn should bind a dummy non-filtering sampler. |samplerLocation| is unused. + bool useDummySampler; std::string GetName() const; }; bool operator<(const CombinedSampler& a, const CombinedSampler& b); @@ -49,7 +52,8 @@ namespace dawn_native { namespace opengl { std::string TranslateToGLSL(const char* entryPointName, SingleShaderStage stage, CombinedSamplerInfo* combinedSamplers, - const PipelineLayout* layout) const; + const PipelineLayout* layout, + bool* needsDummySampler) const; private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor);