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 <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp
index ee4aa5c..3396c49 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 <set>
@@ -42,8 +44,8 @@
} // namespace
- PipelineGL::PipelineGL() {
- }
+ PipelineGL::PipelineGL() = default;
+ PipelineGL::~PipelineGL() = default;
void PipelineGL::Initialize(const OpenGLFunctions& gl,
const PipelineLayout* layout,
@@ -82,14 +84,25 @@
// Create an OpenGL shader for each stage and gather the list of combined samplers.
PerStage<CombinedSamplerInfo> 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 @@
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 @@
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 ffef4d2..1d995d0 100644
--- a/src/dawn_native/opengl/PipelineGL.h
+++ b/src/dawn_native/opengl/PipelineGL.h
@@ -31,10 +31,12 @@
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 @@
GLuint mProgram;
std::vector<std::vector<SamplerUnit>> mUnitsForSamplers;
std::vector<std::vector<GLuint>> mUnitsForTextures;
+ std::vector<GLuint> 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<Sampler> mDummySampler;
};
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp
index ff2ecaf..792a2fc 100644
--- a/src/dawn_native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn_native/opengl/ShaderModuleGL.cpp
@@ -39,15 +39,19 @@
}
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<uint32_t>(samplerLocation.group) << "_"
- << static_cast<uint32_t>(samplerLocation.binding);
+ if (useDummySampler) {
+ o << "_dummy_sampler";
+ } else {
+ o << "_" << static_cast<uint32_t>(samplerLocation.group) << "_"
+ << static_cast<uint32_t>(samplerLocation.binding);
+ }
o << "_with_" << static_cast<uint32_t>(textureLocation.group) << "_"
<< static_cast<uint32_t>(textureLocation.binding);
return o.str();
@@ -68,7 +72,8 @@
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 @@
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 @@
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 65456dd..c18fa48 100644
--- a/src/dawn_native/opengl/ShaderModuleGL.h
+++ b/src/dawn_native/opengl/ShaderModuleGL.h
@@ -35,6 +35,9 @@
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 @@
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);
