From b85d52f7278cd892cbc643c1cd416dfa8486ab09 Mon Sep 17 00:00:00 2001 From: Serge Metral Date: Thu, 29 Jan 2026 16:27:32 -0800 Subject: [PATCH] External sampler bind index bug (#9664) Patching a fix for the external sampler use case where the same sampler is bound to two different indices. As it stands, the code fails to differentiate between the two layouts. --- .../src/vulkan/VulkanDescriptorSetLayoutCache.cpp | 14 +++++++------- .../src/vulkan/VulkanDescriptorSetLayoutCache.h | 2 +- filament/backend/src/vulkan/VulkanDriver.cpp | 3 ++- .../src/vulkan/VulkanExternalImageManager.cpp | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.cpp b/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.cpp index 7483b5d239..7c144d19b4 100644 --- a/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.cpp +++ b/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.cpp @@ -62,7 +62,7 @@ uint32_t appendBindings(VkDescriptorSetLayoutBinding* toBind, VkDescriptorType t uint32_t appendSamplerBindings(VkDescriptorSetLayoutBinding* toBind, fvkutils::SamplerBitmask const& mask, fvkutils::SamplerBitmask const& external, - utils::FixedCapacityVector const& immutableSamplers) { + utils::FixedCapacityVector> const& immutableSamplers) { using Bitmask = fvkutils::SamplerBitmask; uint32_t count = 0; Bitmask alreadySeen; @@ -92,7 +92,7 @@ uint32_t appendSamplerBindings(VkDescriptorSetLayoutBinding* toBind, .descriptorCount = 1, .stageFlags = stages, .pImmutableSamplers = external[index] && immutableSamplerCount > immutableIndex - ? &immutableSamplers[immutableIndex++] + ? &(immutableSamplers[immutableIndex++].second) : nullptr, }; } @@ -100,14 +100,14 @@ uint32_t appendSamplerBindings(VkDescriptorSetLayoutBinding* toBind, return count; } -uint64_t computeImmutableSamplerHash(utils::FixedCapacityVector const& samplers) { +uint64_t computeImmutableSamplerHash( + utils::FixedCapacityVector> const& samplers) { size_t const size = samplers.size(); if (size == 0) { return 0; - } else if (size == 1) { - return (uint64_t) samplers[0]; } - return utils::hash::murmur3((uint32_t*) samplers.data(), samplers.size() * 2, 0); + //64bit + 64bit per element = 4 words + return utils::hash::murmur3((uint32_t*) samplers.data(), samplers.size() * 4, 0); } } // anonymous namespace @@ -128,7 +128,7 @@ void VulkanDescriptorSetLayoutCache::terminate() noexcept { VkDescriptorSetLayout VulkanDescriptorSetLayoutCache::getVkLayout( VulkanDescriptorSetLayout::Bitmask const& bitmasks, fvkutils::SamplerBitmask externalSamplers, - utils::FixedCapacityVector immutableSamplers) { + utils::FixedCapacityVector> immutableSamplers) { LayoutKey key = { .bitmask = bitmasks, .immutableSamplerHash = computeImmutableSamplerHash(immutableSamplers), diff --git a/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.h b/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.h index 93f538f199..3e4c1620bc 100644 --- a/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.h +++ b/filament/backend/src/vulkan/VulkanDescriptorSetLayoutCache.h @@ -47,7 +47,7 @@ public: // This method is meant to be used with external samplers VkDescriptorSetLayout getVkLayout(VulkanDescriptorSetLayout::Bitmask const& bitmasks, fvkutils::SamplerBitmask externalSamplers, - utils::FixedCapacityVector immutableSamplers = {}); + utils::FixedCapacityVector> immutableSamplers = {}); private: VkDevice mDevice; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index dd80c62cde..0f9c4b99f4 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -882,7 +882,8 @@ void VulkanDriver::createProgramR(Handle ph, Program&& program, utils // formats. It seems to be enough, in practicce, to simply run through a list of the types of // samplers that *might* appear. As long as the real pipeline is close enough to something that // the driver has seen before, we are able to get a cache hit. - utils::FixedCapacityVector externalSamplers (layouts[i]->bitmask.externalSampler.count(), externalSampler); + utils::FixedCapacityVector> externalSamplers( + layouts[i]->bitmask.externalSampler.count(), { 0, externalSampler }); vkLayouts[i] = mDescriptorSetLayoutCache.getVkLayout( layouts[i]->bitmask, layouts[i]->bitmask.externalSampler, externalSamplers); } diff --git a/filament/backend/src/vulkan/VulkanExternalImageManager.cpp b/filament/backend/src/vulkan/VulkanExternalImageManager.cpp index 17a565ba0a..e5f2a6a1b5 100644 --- a/filament/backend/src/vulkan/VulkanExternalImageManager.cpp +++ b/filament/backend/src/vulkan/VulkanExternalImageManager.cpp @@ -142,10 +142,10 @@ void VulkanExternalImageManager::updateSetAndLayout( return std::get<0>(a) < std::get<0>(b); }); - utils::FixedCapacityVector outSamplers; + utils::FixedCapacityVector> outSamplers; outSamplers.reserve(MAX_SAMPLER_COUNT); std::for_each(samplerAndBindings.begin(), samplerAndBindings.end(), - [&](auto const& b) { outSamplers.push_back(std::get<1>(b)); }); + [&](auto const& b) { outSamplers.push_back({ static_cast(std::get<0>(b)), std::get<1>(b) }); }); VkDescriptorSetLayout const newLayout = mDescriptorSetLayoutCache->getVkLayout(layout->bitmask, actualExternalSamplers, outSamplers);