diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index bcff274022..4417610ec3 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -542,6 +542,10 @@ namespace dawn_native { return mBindingMap; } + bool BindGroupLayoutBase::HasBinding(BindingNumber bindingNumber) const { + return mBindingMap.count(bindingNumber) != 0; + } + BindingIndex BindGroupLayoutBase::GetBindingIndex(BindingNumber bindingNumber) const { ASSERT(!IsError()); const auto& it = mBindingMap.find(bindingNumber); diff --git a/src/dawn_native/BindGroupLayout.h b/src/dawn_native/BindGroupLayout.h index 113c5912df..641bf45175 100644 --- a/src/dawn_native/BindGroupLayout.h +++ b/src/dawn_native/BindGroupLayout.h @@ -54,6 +54,7 @@ namespace dawn_native { return mBindingInfo[bindingIndex]; } const BindingMap& GetBindingMap() const; + bool HasBinding(BindingNumber bindingNumber) const; BindingIndex GetBindingIndex(BindingNumber bindingNumber) const; // Functions necessary for the unordered_set-based cache. diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp index a0181efafe..01100254b4 100644 --- a/src/dawn_native/opengl/PipelineGL.cpp +++ b/src/dawn_native/opengl/PipelineGL.cpp @@ -119,125 +119,58 @@ namespace dawn_native { namespace opengl { } } - // The uniforms are part of the program state so we can pre-bind buffer units, texture units - // etc. + // Compute links between stages for combined samplers, then bind them to texture units gl.UseProgram(mProgram); const auto& indices = layout->GetBindingIndexInfo(); - for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) { - const BindGroupLayoutBase* bgl = layout->GetBindGroupLayout(group); - - for (const auto& it : bgl->GetBindingMap()) { - BindingNumber bindingNumber = it.first; - BindingIndex bindingIndex = it.second; - - std::string name = GetBindingName(group, bindingNumber); - const BindingInfo& bindingInfo = bgl->GetBindingInfo(bindingIndex); - switch (bindingInfo.bindingType) { - case BindingInfoType::Buffer: - switch (bindingInfo.buffer.type) { - case wgpu::BufferBindingType::Uniform: { - GLint location = gl.GetUniformBlockIndex(mProgram, name.c_str()); - if (location != -1) { - gl.UniformBlockBinding(mProgram, location, - indices[group][bindingIndex]); - } - break; - } - case wgpu::BufferBindingType::Storage: - case wgpu::BufferBindingType::ReadOnlyStorage: { - // Since glShaderStorageBlockBinding doesn't exist in OpenGL ES, we - // skip that call and handle it during shader translation by - // modifying the location decoration. Contrary to all other binding - // types, OpenGL ES's SSBO binding index in the SSBO table is the - // value of the location= decoration in GLSL. - if (gl.GetVersion().IsDesktop()) { - GLuint location = gl.GetProgramResourceIndex( - mProgram, GL_SHADER_STORAGE_BLOCK, name.c_str()); - if (location != GL_INVALID_INDEX) { - gl.ShaderStorageBlockBinding(mProgram, location, - indices[group][bindingIndex]); - } - } - break; - } - case wgpu::BufferBindingType::Undefined: - UNREACHABLE(); - } - break; - - case BindingInfoType::Sampler: - case BindingInfoType::Texture: - // These binding types are handled in the separate sampler and texture - // emulation - break; - - case BindingInfoType::StorageTexture: { - if (gl.GetVersion().IsDesktop()) { - GLint location = gl.GetUniformLocation(mProgram, name.c_str()); - if (location != -1) { - gl.Uniform1i(location, indices[group][bindingIndex]); - } - } - break; - } - } + std::set combinedSamplersSet; + for (SingleShaderStage stage : IterateStages(activeStages)) { + for (const CombinedSampler& combined : combinedSamplers[stage]) { + combinedSamplersSet.insert(combined); } } - // Compute links between stages for combined samplers, then bind them to texture units - { - std::set combinedSamplersSet; - for (SingleShaderStage stage : IterateStages(activeStages)) { - for (const CombinedSampler& combined : combinedSamplers[stage]) { - combinedSamplersSet.insert(combined); - } + mUnitsForSamplers.resize(layout->GetNumSamplers()); + mUnitsForTextures.resize(layout->GetNumSampledTextures()); + + GLuint textureUnit = layout->GetTextureUnitsUsed(); + for (const auto& combined : combinedSamplersSet) { + const std::string& name = combined.GetName(); + GLint location = gl.GetUniformLocation(mProgram, name.c_str()); + + if (location == -1) { + continue; } - mUnitsForSamplers.resize(layout->GetNumSamplers()); - mUnitsForTextures.resize(layout->GetNumSampledTextures()); + gl.Uniform1i(location, textureUnit); - GLuint textureUnit = layout->GetTextureUnitsUsed(); - for (const auto& combined : combinedSamplersSet) { - std::string name = combined.GetName(); - GLint location = gl.GetUniformLocation(mProgram, name.c_str()); + bool shouldUseFiltering; + { + const BindGroupLayoutBase* bgl = + layout->GetBindGroupLayout(combined.textureLocation.group); + BindingIndex bindingIndex = bgl->GetBindingIndex(combined.textureLocation.binding); - if (location == -1) { - continue; - } + GLuint textureIndex = indices[combined.textureLocation.group][bindingIndex]; + mUnitsForTextures[textureIndex].push_back(textureUnit); - gl.Uniform1i(location, textureUnit); - - bool shouldUseFiltering; - { + shouldUseFiltering = bgl->GetBindingInfo(bindingIndex).texture.sampleType == + wgpu::TextureSampleType::Float; + } + { + if (combined.useDummySampler) { + mDummySamplerUnits.push_back(textureUnit); + } else { const BindGroupLayoutBase* bgl = - layout->GetBindGroupLayout(combined.textureLocation.group); + layout->GetBindGroupLayout(combined.samplerLocation.group); BindingIndex bindingIndex = - bgl->GetBindingIndex(combined.textureLocation.binding); + bgl->GetBindingIndex(combined.samplerLocation.binding); - GLuint textureIndex = indices[combined.textureLocation.group][bindingIndex]; - mUnitsForTextures[textureIndex].push_back(textureUnit); - - shouldUseFiltering = bgl->GetBindingInfo(bindingIndex).texture.sampleType == - wgpu::TextureSampleType::Float; + GLuint samplerIndex = indices[combined.samplerLocation.group][bindingIndex]; + mUnitsForSamplers[samplerIndex].push_back({textureUnit, shouldUseFiltering}); } - { - 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}); - } - } - - textureUnit++; } + + textureUnit++; } } diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index 8d8c31124a..b147626cbf 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -179,38 +179,41 @@ namespace dawn_native { namespace opengl { // Change binding names to be "dawn_binding__". // Also unsets the SPIRV "Binding" decoration as it outputs "layout(binding=)" which // isn't supported on OSX's OpenGL. - for (BindGroupIndex group(0); group < kMaxBindGroupsTyped; ++group) { + const PipelineLayout::BindingIndexInfo& indices = layout->GetBindingIndexInfo(); + + // Modify the decoration of variables so that SPIRV-Cross outputs only + // layout(binding=) for interface variables. + // + // When the use_tint_generator toggle is on, Tint is used for the reflection of bindings + // for the implicit pipeline layout and pipeline/layout validation, but bindingInfo is set + // to mGLEntryPoints which is the SPIRV-Cross reflection. Tint reflects bindings used more + // precisely than SPIRV-Cross so some bindings in bindingInfo might not exist in the layout + // and querying the layout for them would cause an ASSERT. That's why we defensively check + // that bindings are in the layout before modifying them. This slight hack is ok because in + // the long term we will use Tint to produce GLSL. + for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) { for (const auto& it : bindingInfo[group]) { + const BindGroupLayoutBase* bgl = layout->GetBindGroupLayout(group); BindingNumber bindingNumber = it.first; const auto& info = it.second; - uint32_t resourceId; - switch (info.bindingType) { - // When the resource is a uniform or shader storage block, we should change the - // block name instead of the instance name. - case BindingInfoType::Buffer: - resourceId = info.base_type_id; - break; - default: - resourceId = info.id; - break; + if (!bgl->HasBinding(bindingNumber)) { + continue; } - compiler.set_name(resourceId, GetBindingName(group, bindingNumber)); + // Remove the name of the base type. This works around an issue where if the SPIRV + // has two uniform/storage interface variables that point to the same base type, + // then SPIRV-Cross would emit two bindings with type names that conflict: + // + // layout(binding=0) uniform Buf {...} binding0; + // layout(binding=1) uniform Buf {...} binding1; + compiler.set_name(info.base_type_id, ""); + + BindingIndex bindingIndex = bgl->GetBindingIndex(bindingNumber); + 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.bindingType == BindingInfoType::Buffer && - (info.buffer.type == wgpu::BufferBindingType::Storage || - info.buffer.type == wgpu::BufferBindingType::ReadOnlyStorage)) { - 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); - } + compiler.set_decoration(info.id, spv::DecorationBinding, + indices[group][bindingIndex]); } }