diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index bc8355626c..da76a0e465 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -468,7 +468,7 @@ Handle FEngine::createPostProcessProgram(MaterialParser& parser, map.getSamplerBinding(bindingPoint, (uint8_t)i, &binding); samplers[i] = { uniformName, binding }; } - pb.addSamplerGroup(bindingPoint, samplers.data(), samplers.size()); + pb.setSamplerGroup(bindingPoint, samplers.data(), samplers.size()); } }; diff --git a/filament/src/Material.cpp b/filament/src/Material.cpp index 023b049173..396adc76c2 100644 --- a/filament/src/Material.cpp +++ b/filament/src/Material.cpp @@ -337,7 +337,7 @@ Handle FMaterial::getProgramSlow(uint8_t variantKey) const noexcept { map.getSamplerBinding(bindingPoint, (uint8_t)i, &binding); samplers[i] = { uniformName, binding }; } - pb.addSamplerGroup(bindingPoint, samplers.data(), samplers.size()); + pb.setSamplerGroup(bindingPoint, samplers.data(), samplers.size()); } }; diff --git a/filament/src/driver/Program.cpp b/filament/src/driver/Program.cpp index c80da19c4a..e3a61c8bde 100644 --- a/filament/src/driver/Program.cpp +++ b/filament/src/driver/Program.cpp @@ -53,7 +53,8 @@ Program& Program::addUniformBlock(size_t index, const UniformInterfaceBlock* ib) return *this; } -Program& Program::addSamplerGroup(size_t bindingPoint, const Program::Sampler* samplers, size_t count) { +Program& Program::setSamplerGroup(size_t bindingPoint, + const Program::Sampler* samplers, size_t count) { auto& samplerList = mSamplerGroups[bindingPoint]; samplerList.clear(); samplerList.insert(samplerList.begin(), samplers, samplers + count); diff --git a/filament/src/driver/Program.h b/filament/src/driver/Program.h index 9e15772089..2f844e2779 100644 --- a/filament/src/driver/Program.h +++ b/filament/src/driver/Program.h @@ -67,10 +67,11 @@ public: // The lifetime of UniformInterfaceBlock* must be longer than Program's Program& addUniformBlock(size_t index, const UniformInterfaceBlock* ib); - // sets the 'index' sampler group descriptor for this program. + // sets the 'bindingPoint' sampler group descriptor for this program. // 'samplers' can be destroyed after this call. - // This effectively associates a set of (BindingPoints, index) to a (shader-binding) - Program& addSamplerGroup(size_t bindingPoint, Sampler const* samplers, size_t count); + // This effectively associates a set of (BindingPoints, index) to a texture unit in the shader. + // Or more precisely, what layout(binding=) is set to in GLSL. + Program& setSamplerGroup(size_t bindingPoint, Sampler const* samplers, size_t count); Program& withVertexShader(void const* data, size_t size) { return shader(Shader::VERTEX, data, size); diff --git a/filament/src/driver/metal/MetalDriver.mm b/filament/src/driver/metal/MetalDriver.mm index 17da389ea6..d0bff35199 100644 --- a/filament/src/driver/metal/MetalDriver.mm +++ b/filament/src/driver/metal/MetalDriver.mm @@ -758,7 +758,7 @@ void MetalDriver::enumerateSamplerGroups( continue; } - if (!program->samplerBindings[bufferIdx].empty()) { + if (samplerIdx < program->samplerBindings[bufferIdx].size()) { uint8_t binding = (uint8_t)program->samplerBindings[bufferIdx][samplerIdx].binding; f(sampler, binding); } diff --git a/filament/src/driver/vulkan/VulkanDriver.cpp b/filament/src/driver/vulkan/VulkanDriver.cpp index 7695885b04..8e6ef6922b 100644 --- a/filament/src/driver/vulkan/VulkanDriver.cpp +++ b/filament/src/driver/vulkan/VulkanDriver.cpp @@ -963,10 +963,9 @@ void VulkanDriver::draw(Driver::PipelineState pipelineState, Driver::RenderPrimi // Obtain the global sampler binding index and pass this to VulkanBinder. Note that // "binding" is an offset that is global to the shader, whereas "samplerIndex" is an // offset into the virtual sampler buffer. - uint8_t binding; - if (!program->samplerBindings[bufferIdx].empty()) { - binding = (uint8_t)program->samplerBindings[bufferIdx][samplerIndex].binding; + if (samplerIndex < program->samplerBindings[bufferIdx].size()) { + uint8_t binding = (uint8_t)program->samplerBindings[bufferIdx][samplerIndex].binding; const SamplerParams& samplerParams = sampler->s; VkSampler vksampler = mSamplerCache.getSampler(samplerParams); const auto* tex = handle_const_cast(mHandleMap, sampler->t); diff --git a/libs/filabridge/include/private/filament/SamplerBindingMap.h b/libs/filabridge/include/private/filament/SamplerBindingMap.h index 5b02c2bb4e..a4d5af709c 100644 --- a/libs/filabridge/include/private/filament/SamplerBindingMap.h +++ b/libs/filabridge/include/private/filament/SamplerBindingMap.h @@ -53,8 +53,8 @@ public: const SamplerInterfaceBlock* perMaterialSib = nullptr, const char* materialName = nullptr); - // Given a valid Filament binding point and an offset with the block, returns true and sets - // the output arguments: (1) the globally unique binding index, and (2) the grouping index. + // Given a valid Filament binding point and an offset within the block, returns true and sets + // the output argument 'globalOffset' to the globally unique binding index. bool getSamplerBinding(uint8_t blockIndex, uint8_t localOffset, uint8_t* globalOffset) const { assert(globalOffset); auto iter = mBindingMap.find(getBindingKey(blockIndex, localOffset)); diff --git a/libs/filabridge/include/private/filament/SibGenerator.h b/libs/filabridge/include/private/filament/SibGenerator.h index d38deca838..9c729c5954 100644 --- a/libs/filabridge/include/private/filament/SibGenerator.h +++ b/libs/filabridge/include/private/filament/SibGenerator.h @@ -56,13 +56,12 @@ struct PostProcessSib { inline constexpr uint8_t getSamplerBindingsStart(driver::Backend api) noexcept { switch (api) { default: - case driver::Backend::OPENGL: - case driver::Backend::VULKAN: { - // Vulkan and OpenGL have a single namespace for uniforms and samplers. + case driver::Backend::VULKAN: + // The Vulkan backend has single namespace for uniforms and samplers. // To avoid collision, the sampler bindings start after the last UBO binding. return filament::BindingPoints::COUNT; - } + case driver::Backend::OPENGL: case driver::Backend::METAL: // Metal has a separate namespace for uniforms and samplers- collisions aren't an issue. return 0;