VK: More refactor of the external sampler flow

Query the texture directly to know if it uses an
external sampler or not. This is already known
when the texture is created.

Remove dead code related to the external sampler
flow.

Simplify the code of the VulkanExternalImageManager
class.

Author: rafadevai

filament/backend/src/vulkan/VulkanDescriptorSetCache.cpp

@@ -355,8 +355,8 @@ void VulkanDescriptorSetCache::updateBuffer(fvkmemory::resource_ptr<VulkanDescri
 }
 
 void VulkanDescriptorSetCache::updateSampler(fvkmemory::resource_ptr<VulkanDescriptorSet> set,
-        uint8_t binding, fvkmemory::resource_ptr<VulkanTexture> texture, VkSampler sampler,
-        VkDescriptorSetLayout externalSamplerLayout) noexcept {
+        uint8_t binding, fvkmemory::resource_ptr<VulkanTexture> texture,
+        VkSampler sampler) noexcept {
 
     // We have to update a bound set for two use cases
     //   - streaming API (a changing feed of AHardwareBuffer)

filament/backend/src/vulkan/VulkanDescriptorSetCache.h

@@ -55,12 +55,7 @@ class VulkanDescriptorSetCache {
             VkDeviceSize size) noexcept;
 
     void updateSampler(fvkmemory::resource_ptr<VulkanDescriptorSet> set, uint8_t binding,
-            fvkmemory::resource_ptr<VulkanTexture> texture, VkSampler sampler,
-            VkDescriptorSetLayout externalSamplerLayout = VK_NULL_HANDLE) noexcept;
-
-    void updateSamplerForExternalSamplerSet(fvkmemory::resource_ptr<VulkanDescriptorSet> set, uint8_t binding,
-            fvkmemory::resource_ptr<VulkanTexture> texture) noexcept;
-
+            fvkmemory::resource_ptr<VulkanTexture> texture, VkSampler sampler) noexcept;
 
     void updateInputAttachment(fvkmemory::resource_ptr<VulkanDescriptorSet> set,
             VulkanAttachment const& attachment) noexcept;

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -507,7 +507,7 @@ void VulkanDriver::updateDescriptorSetTexture(
     auto set = resource_ptr<VulkanDescriptorSet>::cast(&mResourceManager, dsh);
     auto texture = resource_ptr<VulkanTexture>::cast(&mResourceManager, th);
 
-    if (UTILS_UNLIKELY(mExternalImageManager.isExternallySampledTexture(texture))) {
+    if (UTILS_UNLIKELY(texture->isExternallySampled())) {
         mExternalImageManager.bindExternallySampledTexture(set, binding, texture, params);
         mAppState.hasBoundExternalImages = true;
         set->isAnExternalSamplerBound = true;
@@ -767,10 +767,6 @@ void VulkanDriver::createTextureExternalImage2R(Handle<HwTexture> th, backend::S
     // texture into the read layout.
     texture->transitionLayout(&commands, texture->getPrimaryViewRange(), VulkanLayout::FRAG_READ);
 
-    if (imgData.external.valid()) {
-        mExternalImageManager.addExternallySampledTexture(texture, conversion);
-    }
-
     texture.inc();
     mResourceManager.associateHandle(th.getId(), std::move(tag));
 }
@@ -814,8 +810,6 @@ void VulkanDriver::destroyTexture(Handle<HwTexture> th) {
     }
     auto texture = resource_ptr<VulkanTexture>::cast(&mResourceManager, th);
     texture.dec();
-
-    mExternalImageManager.removeExternallySampledTexture(texture);
 }
 
 void VulkanDriver::createProgramR(Handle<HwProgram> ph, Program&& program, utils::ImmutableCString&& tag) {
@@ -1429,8 +1423,6 @@ void VulkanDriver::updateStreams(CommandStream* driver) {
                             VulkanLayout::FRAG_READ);
 
                     if (imgData.external.valid()) {
-                        mExternalImageManager.addExternallySampledTexture(newTexture,
-                                conversion);
                         // Cache the AHB backed image. Acquires the image here.
                         s->pushImage(image, newTexture);
                     }
@@ -2409,7 +2401,6 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) {
         if (std::any_of(layoutHandles.begin(), layoutHandles.end(), haveExternalSamplers)) {
             BindInDrawBundle bundle = {
                 .pipelineState = pipelineState,
-                .dsLayoutHandles = layoutHandles,
                 .descriptorSetMask = descriptorSetMask,
             };
             mPipelineState.bindInDraw = { true, bundle };
@@ -2555,7 +2546,6 @@ void VulkanDriver::draw2(uint32_t indexOffset, uint32_t indexCount, uint32_t ins
     VkCommandBuffer cmdbuffer = mCurrentRenderPass.commandBuffer->buffer();
     auto const& [doBindInDraw, bundle] = mPipelineState.bindInDraw;
 
-    fvkutils::DescriptorSetMask setsWithExternalSamplers = {};
     if (doBindInDraw) {
         // Create the new pipeline layout from the current bounded descriptor sets.
         // The layout of the descriptor sets at this point should have the final one taking into account

filament/backend/src/vulkan/VulkanDriver.h

@@ -177,7 +177,6 @@ class VulkanDriver final : public DriverBase {
 
     struct BindInDrawBundle {
         PipelineState pipelineState = {};
-        DescriptorSetLayoutHandleList dsLayoutHandles = {};
         fvkutils::DescriptorSetMask descriptorSetMask = {};
         resource_ptr<VulkanProgram> program = {};
     };

filament/backend/src/vulkan/VulkanExternalImageManager.cpp

@@ -37,16 +37,6 @@ void erasep(std::vector<T>& v, std::function<bool(T const&)> f) {
     v.erase(newEnd, v.end());
 }
 
-using ImageData = VulkanExternalImageManager::VulkanExternalImageManager::ImageData;
-ImageData& findImage(std::vector<ImageData>& images,
-        fvkmemory::resource_ptr<VulkanTexture> texture) {
-    auto itr = std::find_if(images.begin(), images.end(), [&](ImageData const& data) {
-        return data.image == texture;
-    });
-    assert_invariant(itr != images.end());
-    return *itr;
-}
-
 }// namespace
 
 VulkanExternalImageManager::VulkanExternalImageManager(VulkanSamplerCache* samplerCache,
@@ -61,7 +51,6 @@ VulkanExternalImageManager::~VulkanExternalImageManager() = default;
 
 void VulkanExternalImageManager::terminate() {
     mSetBindings.clear();
-    mImages.clear();
 }
 
 void VulkanExternalImageManager::updateSetAndLayout(
@@ -96,9 +85,7 @@ void VulkanExternalImageManager::updateSetAndLayout(
             actualExternalSamplers, outSamplers);
     // Update the external samplers in the set
     for (auto& [binding, sampler, image]: samplerAndBindings) {
-        // We cannot call updateSamplerForExternalSamplerSet because some samplers are non NULL
-        // (RGB) and we cannot do a combined update with a NULL sampler.
-        mDescriptorSetCache->updateSampler(set, binding, image, sampler, set->boundLayout);
+        mDescriptorSetCache->updateSampler(set, binding, image, sampler);
     }
 }
 
@@ -137,8 +124,6 @@ void VulkanExternalImageManager::removeDescriptorSet(
 void VulkanExternalImageManager::bindExternallySampledTexture(
         fvkmemory::resource_ptr<VulkanDescriptorSet> set, uint8_t bindingPoint,
         fvkmemory::resource_ptr<VulkanTexture> image, SamplerParams samplerParams) {
-    // Should we do duplicate validation here?
-    auto& imageData = findImage(mImages, image);
     // according to spec, these must match chromaFilter
     // https://registry.khronos.org/vulkan/specs/latest/man/html/VkSamplerCreateInfo.html#VUID-VkSamplerCreateInfo-minFilter-01645
     samplerParams.filterMag = SamplerMagFilter::NEAREST;
@@ -153,34 +138,10 @@ void VulkanExternalImageManager::bindExternallySampledTexture(
 
     VkSampler const sampler = mSamplerCache->getSampler({
         .sampler = samplerParams,
-        .conversion = imageData.conversion,
-    });
-
-    mSetBindings.push_back({ bindingPoint, imageData.image, set, sampler });
-}
-
-void VulkanExternalImageManager::addExternallySampledTexture(
-        fvkmemory::resource_ptr<VulkanTexture> image, VkSamplerYcbcrConversion const conversion) {
-    mImages.push_back({
-        .image = image,
-        .conversion = conversion,
-    });
-}
-
-void VulkanExternalImageManager::removeExternallySampledTexture(
-        fvkmemory::resource_ptr<VulkanTexture> image) {
-    erasep<SetBindingInfo>(mSetBindings,
-            [&](auto const& bindingInfo) { return (bindingInfo.image == image); });
-    erasep<ImageData>(mImages, [&](auto const& imageData) {
-        return imageData.image == image;
+        .conversion = image->getYcbcrConversion(),
     });
-}
 
-bool VulkanExternalImageManager::isExternallySampledTexture(
-        fvkmemory::resource_ptr<VulkanTexture> image) const {
-    return std::find_if(mImages.begin(), mImages.end(), [&](auto const& imageData) {
-        return imageData.image == image;
-    }) != mImages.end();
+    mSetBindings.push_back({ bindingPoint, image, set, sampler });
 }
 
 void VulkanExternalImageManager::clearTextureBinding(

filament/backend/src/vulkan/VulkanExternalImageManager.h

@@ -62,21 +62,9 @@ class VulkanExternalImageManager {
     void clearTextureBinding(fvkmemory::resource_ptr<VulkanDescriptorSet> set,
                              uint8_t bindingPoint);
 
-    void addExternallySampledTexture(fvkmemory::resource_ptr<VulkanTexture> external,
-            VkSamplerYcbcrConversion const conversion);
-
-    void removeExternallySampledTexture(fvkmemory::resource_ptr<VulkanTexture> image);
-
-    bool isExternallySampledTexture(fvkmemory::resource_ptr<VulkanTexture> image) const;
-
     VkSamplerYcbcrConversion getVkSamplerYcbcrConversion(
             VulkanPlatform::ExternalImageMetadata const& metadata);
 
-    struct ImageData {
-        fvkmemory::resource_ptr<VulkanTexture> image;
-        VkSamplerYcbcrConversion conversion = VK_NULL_HANDLE;
-    };
-
     // - Update the descriptor set layout with the external samplers. This layout will be assign the
     // respective immutable samplers to the layout.
     // - Switch from binding the base layout to the new one.
@@ -104,7 +92,6 @@ class VulkanExternalImageManager {
 
     // Use vectors instead of hash maps because we only expect small number of entries.
     std::vector<SetBindingInfo> mSetBindings;
-    std::vector<ImageData> mImages;
 };
 
 } // filament::backend

filament/backend/src/vulkan/VulkanTexture.cpp

@@ -840,19 +840,6 @@ void VulkanTexture::setLayout(VkImageSubresourceRange const& range, VulkanLayout
     }
 }
 
-void VulkanTexture::setYcbcrConversion(VkSamplerYcbcrConversion conversion) {
-    // Note that this comparison is valid because we only ever create VkSamplerYcbcrConversion from
-    // a cache.  So for each set of parameters, there is exactly one conversion (similar to
-    // samplers).
-    VulkanTextureState::Ycbcr ycbcr = {
-        .conversion = conversion,
-    };
-    if (mState->mYcbcr != ycbcr) {
-        mState->mYcbcr = ycbcr;
-        mState->clearCachedImageViews();
-    }
-}
-
 VulkanLayout VulkanTexture::getLayout(uint32_t layer, uint32_t level) const {
     assert_invariant(level <= 0xffff && layer <= 0xffff);
     const uint32_t key = (layer << 16) | level;

filament/backend/src/vulkan/VulkanTexture.h

@@ -242,6 +242,8 @@ struct VulkanTexture : public HwTexture, fvkmemory::Resource {
         return mState->mIsProtected;
     }
 
+    bool isExternallySampled() const { return mState->mYcbcr.conversion != VK_NULL_HANDLE; }
+
     bool transitionLayout(VulkanCommandBuffer* commands, VkImageSubresourceRange const& range,
             VulkanLayout newLayout);
 
@@ -262,10 +264,7 @@ struct VulkanTexture : public HwTexture, fvkmemory::Resource {
     // manually (outside of calls to transitionLayout).
     void setLayout(VkImageSubresourceRange const& range, VulkanLayout newLayout);
 
-    // This is used in the case of external images and external samplers. AHB might update the
-    // conversion per-frame. This implies that we need to invalidate the view cache when that
-    // happens.
-    void setYcbcrConversion(VkSamplerYcbcrConversion conversion);
+    VkSamplerYcbcrConversion getYcbcrConversion() const { return mState->mYcbcr.conversion; }
 
 #if FVK_ENABLED(FVK_DEBUG_TEXTURE)
     void print() const;