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.

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<VkSampler> const& immutableSamplers) {
+        utils::FixedCapacityVector<std::pair<uint64_t, VkSampler>> 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<VkSampler> const& samplers) {
+uint64_t computeImmutableSamplerHash(
+        utils::FixedCapacityVector<std::pair<uint64_t, VkSampler>> 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<VkSampler> immutableSamplers) {
+        utils::FixedCapacityVector<std::pair<uint64_t, VkSampler>> immutableSamplers) {
     LayoutKey key = {
         .bitmask = bitmasks,
         .immutableSamplerHash = computeImmutableSamplerHash(immutableSamplers),

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<VkSampler> immutableSamplers = {});
+            utils::FixedCapacityVector<std::pair<uint64_t, VkSampler>> immutableSamplers = {});
 
 private:
     VkDevice mDevice;

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -882,7 +882,8 @@ void VulkanDriver::createProgramR(Handle<HwProgram> 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<VkSampler> externalSamplers (layouts[i]->bitmask.externalSampler.count(), externalSampler);
+            utils::FixedCapacityVector<std::pair<uint64_t, VkSampler>> externalSamplers(
+                    layouts[i]->bitmask.externalSampler.count(), { 0, externalSampler });
             vkLayouts[i] = mDescriptorSetLayoutCache.getVkLayout(
                 layouts[i]->bitmask, layouts[i]->bitmask.externalSampler, externalSamplers);
         }

filament/backend/src/vulkan/VulkanExternalImageManager.cpp

@@ -142,10 +142,10 @@ void VulkanExternalImageManager::updateSetAndLayout(
         return std::get<0>(a) < std::get<0>(b);
     });
 
-    utils::FixedCapacityVector<VkSampler> outSamplers;
+    utils::FixedCapacityVector<std::pair<uint64_t,VkSampler>> 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<uint64_t>(std::get<0>(b)), std::get<1>(b) }); });
 
     VkDescriptorSetLayout const newLayout = mDescriptorSetLayoutCache->getVkLayout(layout->bitmask,
             actualExternalSamplers, outSamplers);