vk: delay destruction

filament/backend/src/vulkan/VulkanDescriptorSetCache.cpp

@@ -332,6 +332,7 @@ void VulkanDescriptorSetCache::commit(VulkanCommandBuffer* commands,
 void VulkanDescriptorSetCache::updateBuffer(fvkmemory::resource_ptr<VulkanDescriptorSet> set,
         uint8_t binding, fvkmemory::resource_ptr<VulkanBufferObject> bufferObject,
         VkDeviceSize offset, VkDeviceSize size) noexcept {
+    prepareForUpdate(set);
     VkDescriptorBufferInfo const info = {
         .buffer = bufferObject->getVkBuffer(),
         .offset = offset,
@@ -368,20 +369,15 @@ void VulkanDescriptorSetCache::updateSampler(fvkmemory::resource_ptr<VulkanDescr
         // Build a new descriptor set from the new layout
         VkDescriptorSetLayout const genLayout = set->boundLayout;
         VkDescriptorSet const newSet = getVkSet(layout->count, genLayout);
-        Bitmask const ubo = layout->bitmask.ubo | layout->bitmask.dynamicUbo;
-        Bitmask samplers = layout->bitmask.sampler;
-        samplers.unset(binding);
 
-        // Each bitmask denotes a binding index, and separated into two stages - vertex and buffer
-        // We fold the two stages into just the lower half of the bits to denote a combined set of
-        // bindings.
-        Bitmask const copyBindings = foldBitsInHalf(ubo | samplers);
         VkDescriptorSet const srcSet = set->getVkSet();
-        copySet(srcSet, newSet, copyBindings);
+        copySet(srcSet, newSet, layout);
         set->addNewSet(newSet,
                 [this, layoutCount = layout->count, genLayout, newSet](VulkanDescriptorSet*) {
                     this->manualRecycle(layoutCount, genLayout, newSet);
                 });
+    } else {
+        prepareForUpdate(set);
     }
 
     VkDescriptorSet const vkset = set->getVkSet();
@@ -448,21 +444,41 @@ void VulkanDescriptorSetCache::manualRecycle(VulkanDescriptorSetLayout::Count co
 
 void VulkanDescriptorSetCache::gc() { mStashedSets = {}; }
 
+void VulkanDescriptorSetCache::prepareForUpdate(
+        fvkmemory::resource_ptr<VulkanDescriptorSet> set) noexcept {
+    if (set->isBound()) {
+        auto layout = set->getLayout();
+        VkDescriptorSetLayout const vklayout = set->boundLayout;
+        VkDescriptorSet const newSet = mDescriptorPool->obtainSet(layout->count, vklayout);
+        copySet(set->getVkSet(), newSet, layout);
+        set->addNewSet(newSet,
+                [this, layoutCount = layout->count, vklayout, newSet](VulkanDescriptorSet*) {
+                    this->manualRecycle(layoutCount, vklayout, newSet);
+                });
+    }
+}
+
 void VulkanDescriptorSetCache::copySet(VkDescriptorSet srcSet, VkDescriptorSet dstSet,
-        fvkutils::SamplerBitmask bindings) const {
-    // TODO: fix the size for better memory management
+        fvkmemory::resource_ptr<VulkanDescriptorSetLayout> layout) const {
+    Bitmask const ubo = layout->bitmask.ubo | layout->bitmask.dynamicUbo;
+    Bitmask const samplers = layout->bitmask.sampler;
+    Bitmask const inputAttachments = layout->bitmask.inputAttachment;
+    Bitmask const bindings = foldBitsInHalf(ubo | samplers | inputAttachments);
+
     std::vector<VkCopyDescriptorSet> copies;
     bindings.forEachSetBit([&](size_t index) {
         copies.push_back({
-            .sType = VK_STRUCTURE_TYPE_COPY_DESCRIPTOR_SET,
-            .srcSet = srcSet,
-            .srcBinding = (uint32_t) index,
-            .dstSet = dstSet,
-            .dstBinding = (uint32_t) index,
-            .descriptorCount = 1,
+                .sType = VK_STRUCTURE_TYPE_COPY_DESCRIPTOR_SET,
+                .srcSet = srcSet,
+                .srcBinding = (uint32_t) index,
+                .dstSet = dstSet,
+                .dstBinding = (uint32_t) index,
+                .descriptorCount = 1,
         });
     });
-    vkUpdateDescriptorSets(mDevice, 0, nullptr, copies.size(), copies.data());
+    if (!copies.empty()) {
+        vkUpdateDescriptorSets(mDevice, 0, nullptr, (uint32_t) copies.size(), copies.data());
+    }
 }
 
 

filament/backend/src/vulkan/VulkanDescriptorSetCache.h

@@ -90,8 +90,10 @@ public:
     void resetCachedState() noexcept { mLastBoundInfo = {}; }
 
 private:
+    void prepareForUpdate(fvkmemory::resource_ptr<VulkanDescriptorSet> set) noexcept;
+
     void copySet(VkDescriptorSet srcSet, VkDescriptorSet destSet,
-            fvkutils::SamplerBitmask copyBindings) const;
+            fvkmemory::resource_ptr<VulkanDescriptorSetLayout> layout) const;
 
     class DescriptorInfinitePool;
 

filament/backend/src/vulkan/VulkanHandles.cpp

@@ -260,8 +260,8 @@ void VulkanDescriptorSet::gc() {
 
 void VulkanDescriptorSet::addNewSet(VkDescriptorSet vkSet, OnRecycle&& onRecycleFn) {
     gc();
-    mCurrentSetIndex = mSets.size();
     mSets.push_back({ vkSet, std::move(onRecycleFn) });
+    mCurrentSetIndex = (uint8_t) (mSets.size() - 1);
 }
 
 PushConstantDescription::PushConstantDescription(backend::Program const& program) {

filament/backend/src/vulkan/VulkanHandles.h

@@ -191,7 +191,8 @@ public:
     void referencedBy(VulkanCommandBuffer& commands);
 
     bool isBound() const {
-        return bool(mSets[mCurrentSetIndex].fenceStatus);
+        auto const& set = mSets[mCurrentSetIndex];
+        return set.fenceStatus && set.fenceStatus->getStatus() != VK_SUCCESS;
     }
 
     // The current layout used by the descriptor set. This one will match the bindings, including

filament/backend/src/vulkan/VulkanTexture.cpp

@@ -780,10 +780,16 @@ void VulkanTexture::samplerToAttachmentBarrier(VulkanCommandBuffer* commands,
     VkCommandBuffer const cmdbuf = commands->buffer();
     VkImageLayout const layout =
             fvkutils::getVkLayout(getLayout(range.baseArrayLayer, range.baseMipLevel));
+
+    const bool isDepth = fvkutils::isVkDepthFormat(mState->mVkFormat);
+
     VkImageMemoryBarrier barrier = {
         .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER,
         .srcAccessMask = VK_ACCESS_SHADER_READ_BIT,
-        .dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+        .dstAccessMask = static_cast<VkAccessFlags>(isDepth ? (VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT |
+                                    VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT)
+                                 : (VK_ACCESS_COLOR_ATTACHMENT_READ_BIT |
+                                    VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT)),
         .oldLayout = layout,
         .newLayout = layout,
         .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
@@ -791,9 +797,13 @@ void VulkanTexture::samplerToAttachmentBarrier(VulkanCommandBuffer* commands,
         .image = mState->mTextureImage,
         .subresourceRange = range,
     };
+
+    VkPipelineStageFlags dstStage = isDepth ? (VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT |
+                                               VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT)
+                                            : VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
+
     vkCmdPipelineBarrier(cmdbuf, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
-            VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT |
-                    VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
+            dstStage,
             0, 0, nullptr, 0, nullptr, 1, &barrier);
 }
 
@@ -802,9 +812,13 @@ void VulkanTexture::attachmentToSamplerBarrier(VulkanCommandBuffer* commands,
     VkCommandBuffer const cmdbuf = commands->buffer();
     VkImageLayout const layout
             = fvkutils::getVkLayout(getLayout(range.baseArrayLayer, range.baseMipLevel));
+
+    const bool isDepth = fvkutils::isVkDepthFormat(mState->mVkFormat);
+
     VkImageMemoryBarrier barrier = {
         .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER,
-        .srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+        .srcAccessMask = static_cast<VkAccessFlags>(isDepth ? VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT
+                                 : VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT),
         .dstAccessMask = VK_ACCESS_SHADER_READ_BIT,
         .oldLayout = layout,
         .newLayout = layout,
@@ -813,7 +827,12 @@ void VulkanTexture::attachmentToSamplerBarrier(VulkanCommandBuffer* commands,
         .image = mState->mTextureImage,
         .subresourceRange = range,
     };
-    vkCmdPipelineBarrier(cmdbuf, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
+
+    VkPipelineStageFlags srcStage = isDepth ? (VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT |
+                                               VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT)
+                                            : VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
+
+    vkCmdPipelineBarrier(cmdbuf, srcStage,
             VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier);
 }
 

filament/backend/src/vulkan/memory/ResourceManager.cpp

@@ -41,23 +41,55 @@ void ResourceManager::gc() noexcept {
         list.clear();
     };
 
+    FrameGcList frameGc;
+
     {
         // Note that we're not copying mThreadSafeGcList because the objects here do not have
         // resource_ptrs to other handle objects, so their desctruction would not add more elements
         // to mThreadSafeGcList.
         std::unique_lock<utils::Mutex> lock(mThreadSafeGcListMutex);
-        destroyAll(mThreadSafeGcList);
+        std::swap(frameGc.threadSafeGcList, mThreadSafeGcList);
     }
 
-    GcList gcs;
-    std::swap(gcs, mGcList);
-    destroyAll(gcs);
+    std::swap(frameGc.gcList, mGcList);
+
+    mFramesGcList.push_back(std::move(frameGc));
+
+    if (mFramesGcList.size() > 4) {
+        FrameGcList oldest = std::move(mFramesGcList.front());
+        mFramesGcList.erase(mFramesGcList.begin());
+        destroyAll(oldest.threadSafeGcList);
+        destroyAll(oldest.gcList);
+    }
     FVK_SYSTRACE_END();
 }
 
 void ResourceManager::terminate() noexcept {
-    while (!mThreadSafeGcList.empty() || !mGcList.empty()) {
-        gc();
+    auto destroyAll = [this](GcList& list) {
+        for (auto const& [type, id]: list) {
+            destroyWithType(type, id);
+        }
+        list.clear();
+    };
+
+    while (!mThreadSafeGcList.empty() || !mGcList.empty() || !mFramesGcList.empty()) {
+        if (!mFramesGcList.empty()) {
+            FrameGcList oldest = std::move(mFramesGcList.front());
+            mFramesGcList.erase(mFramesGcList.begin());
+            destroyAll(oldest.threadSafeGcList);
+            destroyAll(oldest.gcList);
+        }
+
+        GcList threadSafeList;
+        {
+            std::unique_lock<utils::Mutex> lock(mThreadSafeGcListMutex);
+            std::swap(threadSafeList, mThreadSafeGcList);
+        }
+        destroyAll(threadSafeList);
+
+        GcList gcs;
+        std::swap(gcs, mGcList);
+        destroyAll(gcs);
     }
 }
 

filament/backend/src/vulkan/memory/ResourceManager.h

@@ -100,6 +100,12 @@ private:
     GcList mThreadSafeGcList;
     GcList mGcList;
 
+    struct FrameGcList {
+        GcList threadSafeGcList;
+        GcList gcList;
+    };
+    std::vector<FrameGcList> mFramesGcList;
+
     template<typename D>
     friend struct resource_ptr;
 

filament/src/MaterialDefinition.cpp

@@ -271,8 +271,8 @@ std::unique_ptr<MaterialDefinition> MaterialDefinition::create(FEngine& engine,
 
 void MaterialDefinition::terminate(FEngine& engine) {
     DriverApi& driver = engine.getDriverApi();
-    perViewDescriptorSetLayoutPcf.terminate(engine.getDescriptorSetLayoutFactory(), driver);
-    perViewDescriptorSetLayoutS2d.terminate(engine.getDescriptorSetLayoutFactory(), driver);
+    perViewDescriptorSetLayout.terminate(engine.getDescriptorSetLayoutFactory(), driver);
+    perViewDescriptorSetLayoutVsm.terminate(engine.getDescriptorSetLayoutFactory(), driver);
     descriptorSetLayout.terminate(engine.getDescriptorSetLayoutFactory(), driver);
 }
 
@@ -607,23 +607,23 @@ void MaterialDefinition::processDescriptorSets(FEngine& engine) {
             refractionMode == RefractionMode::SCREEN_SPACE;
     bool const hasFog = !(variantFilterMask & UserVariantFilterMask(UserVariantFilterBit::FOG));
 
-    this->perViewDescriptorSetLayoutPcfDescription = descriptor_sets::getPerViewDescriptorSetLayout(
+    this->perViewDescriptorSetLayoutDescription = descriptor_sets::getPerViewDescriptorSetLayout(
             materialDomain, isLit, isSSR, hasFog, false);
 
-    this->perViewDescriptorSetLayoutS2dDescription = descriptor_sets::getPerViewDescriptorSetLayout(
+    this->perViewDescriptorSetLayoutVsmDescription = descriptor_sets::getPerViewDescriptorSetLayout(
             materialDomain, isLit, isSSR, hasFog, true);
 
     // set the labels
     this->descriptorSetLayoutDescription.label = CString{ name }.append("_perMat");
-    this->perViewDescriptorSetLayoutPcfDescription.label = CString{ name }.append("_perView");
-    this->perViewDescriptorSetLayoutS2dDescription.label = CString{ name }.append("_perViewVsm");
+    this->perViewDescriptorSetLayoutDescription.label = CString{ name }.append("_perView");
+    this->perViewDescriptorSetLayoutVsmDescription.label = CString{ name }.append("_perViewVsm");
 
     // get the PER_RENDERABLE and PER_VIEW descriptor binding info
     for (auto&& [bindingPoint, dsl] : {
             std::pair{ DescriptorSetBindingPoints::PER_RENDERABLE,
                     descriptor_sets::getPerRenderableLayout() },
             std::pair{ DescriptorSetBindingPoints::PER_VIEW,
-                    this->perViewDescriptorSetLayoutPcfDescription }}) {
+                    this->perViewDescriptorSetLayoutDescription }}) {
         Program::DescriptorBindingsInfo& descriptors = programDescriptorBindings[+bindingPoint];
         descriptors.reserve(dsl.descriptors.size());
         for (auto const& entry: dsl.descriptors) {
@@ -636,17 +636,17 @@ void MaterialDefinition::processDescriptorSets(FEngine& engine) {
             descriptorSetLayoutFactory, driver,
             this->descriptorSetLayoutDescription };
 
-    this->perViewDescriptorSetLayoutPcf = {
+    this->perViewDescriptorSetLayout = {
             descriptorSetLayoutFactory, driver,
-            this->perViewDescriptorSetLayoutPcfDescription };
+            this->perViewDescriptorSetLayoutDescription };
 
-    this->perViewDescriptorSetLayoutS2d = {
+    this->perViewDescriptorSetLayoutVsm = {
             descriptorSetLayoutFactory, driver,
-            this->perViewDescriptorSetLayoutS2dDescription };
+            this->perViewDescriptorSetLayoutVsmDescription };
 }
 
 backend::DescriptorSetLayout const& MaterialDefinition::getPerViewDescriptorSetLayoutDescription(
-        Variant const variant, bool const useS2dDescriptorSetLayout) const noexcept {
+        Variant const variant, bool const useVsmDescriptorSetLayout) const noexcept {
     if (materialDomain == MaterialDomain::SURFACE) {
         if (Variant::isValidDepthVariant(variant)) {
             // Use the layout description used to create the per view depth variant layout.
@@ -657,10 +657,10 @@ backend::DescriptorSetLayout const& MaterialDefinition::getPerViewDescriptorSetL
             return descriptor_sets::getSsrVariantLayout();
         }
     }
-    if (useS2dDescriptorSetLayout) {
-        return perViewDescriptorSetLayoutS2dDescription;
+    if (useVsmDescriptorSetLayout) {
+        return perViewDescriptorSetLayoutVsmDescription;
     }
-    return perViewDescriptorSetLayoutPcfDescription;
+    return perViewDescriptorSetLayoutDescription;
 }
 
 Handle<HwProgram> MaterialDefinition::compileProgram(
@@ -689,7 +689,7 @@ Handle<HwProgram> MaterialDefinition::compileProgram(
     pb.descriptorLayout(+DescriptorSetBindingPoints::PER_VIEW,
             getPerViewDescriptorSetLayoutDescription(
                     specialization.variant,
-                    Variant::isShadowSampler2DVariant(specialization.variant)));
+                    Variant::isVSMVariant(specialization.variant)));
     pb.descriptorLayout(+DescriptorSetBindingPoints::PER_RENDERABLE,
             descriptor_sets::getPerRenderableLayout());
     pb.descriptorLayout(

filament/src/MaterialDefinition.h

@@ -94,17 +94,17 @@ struct MaterialDefinition {
             backend::ShaderModel const sm, bool isStereoSupported) const noexcept;
 
     backend::DescriptorSetLayout const& getPerViewDescriptorSetLayoutDescription(
-            Variant const variant, bool useS2dDescriptorSetLayout) const noexcept;
+            Variant const variant, bool useVsmDescriptorSetLayout) const noexcept;
 
     // Keep track of the definitions of the descriptor set layouts, as these
     // may be used by some backends in parallel compilation of programs.
-    backend::DescriptorSetLayout perViewDescriptorSetLayoutPcfDescription;
-    backend::DescriptorSetLayout perViewDescriptorSetLayoutS2dDescription;
+    backend::DescriptorSetLayout perViewDescriptorSetLayoutDescription;
+    backend::DescriptorSetLayout perViewDescriptorSetLayoutVsmDescription;
     backend::DescriptorSetLayout descriptorSetLayoutDescription;
 
     // try to order by frequency of use
-    filament::DescriptorSetLayout perViewDescriptorSetLayoutPcf;
-    filament::DescriptorSetLayout perViewDescriptorSetLayoutS2d;
+    filament::DescriptorSetLayout perViewDescriptorSetLayout;
+    filament::DescriptorSetLayout perViewDescriptorSetLayoutVsm;
     filament::DescriptorSetLayout descriptorSetLayout;
     backend::Program::DescriptorSetInfo programDescriptorBindings;
 

filament/src/PostProcessManager.cpp

@@ -802,7 +802,7 @@ FrameGraphId<FrameGraphTexture> PostProcessManager::ssr(FrameGraph& fg,
 
                 // use our special SSR variant, it can only be applied to object that have
                 // the SCREEN_SPACE ReflectionMode.
-                passBuilder.variant(Variant{ Variant::SPECIAL_SSR_VARIANT });
+                passBuilder.variant(Variant{ Variant::SPECIAL_SSR });
 
                 // generate all our drawing commands, except blended objects.
                 passBuilder.commandTypeFlags(RenderPass::CommandTypeFlags::SCREEN_SPACE_REFLECTIONS);

filament/src/RenderPass.cpp

@@ -568,7 +568,7 @@ RenderPass::Command* RenderPass::generateCommandsImpl(CommandTypeFlags extraFlag
     if constexpr (isDepthPass) {
         cmd.info.materialVariant = variant;
         cmd.info.rasterState = {};
-        cmd.info.rasterState.colorWrite = Variant::isPickingVariant(variant) || Variant::isDepthMomentsVariant(variant);
+        cmd.info.rasterState.colorWrite = Variant::isPickingVariant(variant) || Variant::isVSMVariant(variant);
         cmd.info.rasterState.depthWrite = true;
         cmd.info.rasterState.depthFunc = RasterState::DepthFunc::GE;
         cmd.info.rasterState.alphaToCoverage = false;
@@ -616,7 +616,7 @@ RenderPass::Command* RenderPass::generateCommandsImpl(CommandTypeFlags extraFlag
         bool const hasSkinningOrMorphing = hasSkinning || hasMorphing;
 
         // if we are already an SSR variant, the SRE bit is already set
-        static_assert(Variant::SPECIAL_SSR_VARIANT & Variant::SRE);
+        static_assert(Variant::SPECIAL_SSR & Variant::SRE);
         Variant renderableVariant{ variant };
 
         // we can't have SSR and shadowing together by construction

filament/src/details/Material.cpp

@@ -257,9 +257,9 @@ filament::DescriptorSetLayout const& FMaterial::getPerViewDescriptorSetLayout(
     }
     // mDefinition.perViewDescriptorSetLayout{Vsm} is already resolved for MaterialDomain
     if (useVsmDescriptorSetLayout) {
-        return mDefinition.perViewDescriptorSetLayoutS2d;
+        return mDefinition.perViewDescriptorSetLayoutVsm;
     }
-    return mDefinition.perViewDescriptorSetLayoutPcf;
+    return mDefinition.perViewDescriptorSetLayout;
 }
 
 void FMaterial::compile(CompilerPriorityQueue const priority,

filament/src/details/Material.h

@@ -139,7 +139,7 @@ public:
         // This is mostly intended to be used for post-process materials; but it's also useful for
         // Surface material that behave like post-process material (i.e. that don't really have variants or for
         // which VSM is nonsensical, like for unlit materials)
-        return mDefinition.perViewDescriptorSetLayoutPcf;
+        return mDefinition.perViewDescriptorSetLayout;
     }
 
     DescriptorSetLayout const& getPerViewDescriptorSetLayout(
@@ -378,11 +378,11 @@ private:
             Variant variant = {}) const noexcept;
 
     bool isSharedVariant(Variant const variant) const {
-        // HACK: The default material "should" have MNT | DEP, but then we'd have to compile it as a
+        // HACK: The default material "should" have VSM | DEP, but then we'd have to compile it as a
         // lit material, which would increase binary size. Perhaps we could specially compile it
         // with this variant, but with the shader program cache in active development, the days of
         // the default material are numbered anyway.
-        constexpr Variant::type_t vsmAndDep = Variant::MNT | Variant::DEP;
+        constexpr Variant::type_t vsmAndDep = Variant::VSM | Variant::DEP;
         return mDefinition.materialDomain == MaterialDomain::SURFACE && !mIsDefaultMaterial &&
                !mDefinition.hasCustomDepthShader && Variant::isValidDepthVariant(variant) &&
                (variant.key & vsmAndDep) != vsmAndDep;

filament/src/details/Renderer.cpp

@@ -969,7 +969,10 @@ void FRenderer::renderJob(DriverApi& driver, RootArenaScope& rootArenaScope, FVi
     variant.setDirectionalLighting(view.hasDirectionalLighting());
     variant.setDynamicLighting(view.hasDynamicLighting());
     variant.setFog(view.hasFog());
-    variant.setShadowSampler2D(view.hasShadowing() && view.getShadowType() != ShadowType::PCF);
+    // The VSM bit has a different meaning for STANDARD_VARIANT (as opposed to DEPTH_VARIANT),
+    // In the STANDARD_VARIANT case, we are *using* the shadow-map, and the VSM only decides which
+    // type of sampler is used (samplerShadow or sampler2D).
+    variant.setVsm(view.hasShadowing() && view.getShadowType() != ShadowType::PCF);
     variant.setStereo(view.hasStereo());
 
     /*
@@ -978,7 +981,10 @@ void FRenderer::renderJob(DriverApi& driver, RootArenaScope& rootArenaScope, FVi
 
     if (view.needsShadowMap()) {
         Variant shadowVariant(Variant::DEPTH_VARIANT);
-        shadowVariant.setDepthMoments(view.getShadowType() == ShadowType::VSM);
+        // The VSM bit has a different meaning for DEPTH_VARIANT (as opposed to STANDARD_VARIANT),
+        // In the DEPTH_VARIANT case, we are *generating* the shadow-map, and some computations
+        // are handled differently. In addition, the color buffer is used.
+        shadowVariant.setVsm(view.getShadowType() == ShadowType::VSM);
 
         auto shadows = view.renderShadowMaps(engine, fg, cameraInfo, mShaderUserTime,
                 RenderPassBuilder{ commandArena }

libs/filabridge/include/private/filament/DescriptorSets.h

@@ -36,7 +36,7 @@ backend::DescriptorSetLayout const& getPerRenderableLayout() noexcept;
 backend::DescriptorSetLayout getPerViewDescriptorSetLayout(
         MaterialDomain domain,
         bool isLit, bool isSSR, bool hasFog,
-        bool isShadowSampler2D) noexcept;
+        bool isVSM) noexcept;
 
 backend::DescriptorSetLayout getPerViewDescriptorSetLayoutWithVariant(
         Variant variant,

libs/filabridge/include/private/filament/Variant.h

@@ -48,21 +48,20 @@ struct Variant {
     // SRE: Shadow Receiver
     // SKN: Skinning
     // DEP: Depth only
-    // FOG: Fog (standard)
-    // PCK: Picking (depth)
-    // MNT: Output depth moments (depth)
-    // S2D: Sampler type for shadows (0: samplerShadowArray, 1: sampler2DArray) (standard)
+    // FOG: Fog
+    // PCK: Picking (depth variant only)
+    // VSM: Variance shadow maps (depth) / sampler type (standard)
     // STE: Instanced stereo rendering
     //
     //   X: either 1 or 0
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
-    // Variant              | STE | S2D | FOG | DEP | SKN | SRE | DYN | DIR |   256
+    // Variant              | STE | VSM | FOG | DEP | SKN | SRE | DYN | DIR |   256
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
-    //                              MNT   PCK
+    //                                    PCK
     //
     // Standard variants:
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
-    //                      | STE | S2D | FOG |  0  | SKN | SRE | DYN | DIR |    128 - 44 = 84
+    //                      | STE | VSM | FOG |  0  | SKN | SRE | DYN | DIR |    128 - 44 = 84
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
     //      Vertex shader      X     0     0     0     X     X     X     X
     //    Fragment shader      0     X     X     0     0     X     X     X
@@ -73,7 +72,7 @@ struct Variant {
     //
     // Depth variants:
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
-    //                      | STE | MNT | PCK |  1  | SKN |  0  |  0  |  0  |   16 - 4 = 12
+    //                      | STE | VSM | PCK |  1  | SKN |  0  |  0  |  0  |   16 - 4 = 12
     //                      +-----+-----+-----+-----+-----+-----+-----+-----+
     //       Vertex depth      X     X     0     1     X     0     0     0
     //     Fragment depth      0     0     X     1     0     0     0     0
@@ -97,15 +96,13 @@ struct Variant {
     static constexpr type_t DEP   = 0x10; // depth only variants
     static constexpr type_t FOG   = 0x20; // fog (standard)
     static constexpr type_t PCK   = 0x20; // picking (depth)
-    static constexpr type_t S2D   = 0x40; // sampler type
-    static constexpr type_t MNT   = 0x40; // variance shadow maps
+    static constexpr type_t VSM   = 0x40; // variance shadow maps / sampler type
     static constexpr type_t STE   = 0x80; // instanced stereo
 
     static constexpr type_t NO_VARIANT         = 0u;
 
     // special variants (variants that use the reserved space)
-    static constexpr type_t SPECIAL_SSR_VARIANT=       S2D |       SRE            ;
-    static constexpr type_t SPECIAL_SSR_MASK   = STE | S2D | DEP | SRE | DYN | DIR;
+    static constexpr type_t SPECIAL_SSR   = VSM | SRE; // screen-space reflections variant
 
     static constexpr type_t STANDARD_MASK      = DEP;
     static constexpr type_t STANDARD_VARIANT   = 0u;
@@ -130,30 +127,29 @@ struct Variant {
     void setSkinning(bool v) noexcept            { set(v, SKN); }
     void setFog(bool v) noexcept                 { set(v, FOG); }
     void setPicking(bool v) noexcept             { set(v, PCK); }
-    void setShadowSampler2D(bool v) noexcept     { set(v, S2D); }
-    void setDepthMoments(bool v) noexcept        { set(v, MNT); }
+    void setVsm(bool v) noexcept                 { set(v, VSM); }
     void setStereo(bool v) noexcept              { set(v, STE); }
 
     static constexpr bool isValidDepthVariant(Variant variant) noexcept {
         // Can't have VSM and PICKING together with DEPTH variants
-        constexpr type_t RESERVED_MASK  = MNT | PCK | DEP | SRE | DYN | DIR;
-        constexpr type_t RESERVED_VALUE = MNT | PCK | DEP;
+        constexpr type_t RESERVED_MASK  = VSM | PCK | DEP | SRE | DYN | DIR;
+        constexpr type_t RESERVED_VALUE = VSM | PCK | DEP;
         return ((variant.key & DEPTH_MASK) == DEPTH_VARIANT) &&
                ((variant.key & RESERVED_MASK) != RESERVED_VALUE);
    }
 
     static constexpr bool isValidStandardVariant(Variant variant) noexcept {
         // can't have shadow receiver if we don't have any lighting
-        constexpr type_t RESERVED0_MASK  = S2D | FOG | SRE | DYN | DIR;
-        constexpr type_t RESERVED0_VALUE = S2D | FOG | SRE;
+        constexpr type_t RESERVED0_MASK  = VSM | FOG | SRE | DYN | DIR;
+        constexpr type_t RESERVED0_VALUE = VSM | FOG | SRE;
 
         // can't have shadow receiver if we don't have any lighting
-        constexpr type_t RESERVED1_MASK  = S2D | SRE | DYN | DIR;
+        constexpr type_t RESERVED1_MASK  = VSM | SRE | DYN | DIR;
         constexpr type_t RESERVED1_VALUE = SRE;
 
         // can't have VSM without shadow receiver
-        constexpr type_t RESERVED2_MASK  = S2D | SRE;
-        constexpr type_t RESERVED2_VALUE = S2D;
+        constexpr type_t RESERVED2_MASK  = VSM | SRE;
+        constexpr type_t RESERVED2_VALUE = VSM;
 
         return ((variant.key & STANDARD_MASK) == STANDARD_VARIANT) &&
                ((variant.key & RESERVED0_MASK) != RESERVED0_VALUE) &&
@@ -178,15 +174,11 @@ struct Variant {
     }
 
     static constexpr bool isSSRVariant(Variant variant) noexcept {
-        return (variant.key & SPECIAL_SSR_MASK) == SPECIAL_SSR_VARIANT;
+        return (variant.key & (STE | VSM | DEP | SRE | DYN | DIR)) == (VSM | SRE);
     }
 
-    static constexpr bool isShadowSampler2DVariant(Variant variant) noexcept {
-        return !isSSRVariant(variant) && ((variant.key & (S2D | DEP)) == S2D);
-    }
-
-    static constexpr bool isDepthMomentsVariant(Variant variant) noexcept {
-        return !isSSRVariant(variant) && ((variant.key & (MNT | DEP)) == (MNT | DEP));
+    static constexpr bool isVSMVariant(Variant variant) noexcept {
+        return !isSSRVariant(variant) && ((variant.key & VSM) == VSM);
     }
 
     static constexpr bool isShadowReceiverVariant(Variant variant) noexcept {
@@ -210,13 +202,13 @@ struct Variant {
         // vertex shader.
         if ((variant.key & STANDARD_MASK) == STANDARD_VARIANT) {
             if (isSSRVariant(variant)) {
-                variant.key &= ~SPECIAL_SSR_VARIANT;
+                variant.key &= ~(VSM | SRE);
             }
             return variant & (STE | SKN | SRE | DYN | DIR);
         }
         if ((variant.key & DEPTH_MASK) == DEPTH_VARIANT) {
-            // Only MNT, skinning, and stereo affect the vertex shader's DEPTH variant
-            return variant & (STE | MNT | SKN | DEP);
+            // Only VSM, skinning, and stereo affect the vertex shader's DEPTH variant
+            return variant & (STE | VSM | SKN | DEP);
         }
         return {};
     }
@@ -225,11 +217,11 @@ struct Variant {
         // filter out fragment variants that are not needed. For e.g. skinning doesn't
         // affect the fragment shader.
         if ((variant.key & STANDARD_MASK) == STANDARD_VARIANT) {
-            return variant & (S2D | FOG | SRE | DYN | DIR);
+            return variant & (VSM | FOG | SRE | DYN | DIR);
         }
         if ((variant.key & DEPTH_MASK) == DEPTH_VARIANT) {
             // Only VSM & PICKING affects the fragment shader's DEPTH variant
-            return variant & (MNT | PCK | DEP);
+            return variant & (VSM | PCK | DEP);
         }
         return {};
     }
@@ -238,8 +230,8 @@ struct Variant {
         // special case for depth variant
         if (isValidDepthVariant(variant)) {
             if (!isLit) {
-                // if we're unlit, we never need the MNT variant
-                return variant & ~MNT;
+                // if we're unlit, we never need the VSM variant
+                return variant & ~VSM;
             }
             return variant;
         }
@@ -250,9 +242,9 @@ struct Variant {
             // when the shading mode is unlit, remove all the lighting variants
             return variant & UNLIT_MASK;
         }
-        // if shadow receiver is disabled, we pick the shadow sampler
+        // if shadow receiver is disabled, turn off VSM
         if (!(variant.key & SRE)) {
-            return variant & ~S2D;
+            return variant & ~VSM;
         }
         return variant;
     }

libs/filabridge/src/DescriptorSets.cpp

@@ -216,7 +216,7 @@ utils::CString getDescriptorName(DescriptorSetBindingPoints const set,
 DescriptorSetLayout getPerViewDescriptorSetLayout(
         MaterialDomain const domain,
         bool const isLit, bool const isSSR, bool const hasFog,
-        bool const isShadowSampler2D) noexcept {
+        bool const isVSM) noexcept {
 
     switch (domain) {
         case MaterialDomain::SURFACE: {
@@ -255,7 +255,7 @@ DescriptorSetLayout getPerViewDescriptorSetLayout(
             }
 
             // change the SHADOW_MAP descriptor type for VSM
-            if (isShadowSampler2D) {
+            if (isVSM) {
                 auto const pos = std::find_if(layout.descriptors.begin(), layout.descriptors.end(),
                         [](auto const& v) {
                             return v.binding == PerViewBindingPoints::SHADOW_MAP;
@@ -285,7 +285,8 @@ DescriptorSetLayout getPerViewDescriptorSetLayoutWithVariant(
         return ssrVariantDescriptorSetLayout;
     }
     // We need to filter out all the descriptors not included in the "resolved" layout below
-    return getPerViewDescriptorSetLayout(domain, isLit, isSSR, hasFog, Variant::isShadowSampler2DVariant(variant));
+    return getPerViewDescriptorSetLayout(domain, isLit, isSSR, hasFog,
+            Variant::isVSMVariant(variant));
 }
 
 DescriptorType getDescriptorType(SamplerType const type, SamplerFormat const format) {

libs/filabridge/src/Variant.cpp

@@ -32,46 +32,41 @@ namespace filament {
 Variant Variant::filterUserVariant(
         Variant variant, UserVariantFilterMask filterMask) noexcept {
     // these are easy to filter by just removing the corresponding bit
-    if (filterMask & uint32_t(UserVariantFilterBit::DIRECTIONAL_LIGHTING)) {
+    if (filterMask & (uint32_t)UserVariantFilterBit::DIRECTIONAL_LIGHTING) {
         variant.key &= ~DIR;
     }
-
-    if (filterMask & uint32_t(UserVariantFilterBit::DYNAMIC_LIGHTING)) {
+    if (filterMask & (uint32_t)UserVariantFilterBit::DYNAMIC_LIGHTING) {
         variant.key &= ~DYN;
     }
-
-    if (filterMask & uint32_t(UserVariantFilterBit::SKINNING)) {
+    if (filterMask & (uint32_t)UserVariantFilterBit::SKINNING) {
         variant.key &= ~SKN;
     }
-
-    if (filterMask & uint32_t(UserVariantFilterBit::STE)) {
+    if (filterMask & (uint32_t)UserVariantFilterBit::STE) {
         variant.key &= ~(filterMask & STE);
     }
-
-    if (isValidDepthVariant(variant)) {
-        // depth variants can have their MNT bit filtered
-        if (filterMask & uint32_t(UserVariantFilterBit::VSM)) {
-            variant.key &= ~MNT;
-        }
-    } else {
+    if (!isValidDepthVariant(variant)) {
         // we can't remove FOG from depth variants, this would, in fact, remove picking
-        if (filterMask & uint32_t(UserVariantFilterBit::FOG)) {
+        if (filterMask & (uint32_t)UserVariantFilterBit::FOG) {
             variant.key &= ~FOG;
         }
+    } else {
+        // depth variants can have their VSM bit filtered
+        if (filterMask & (uint32_t)UserVariantFilterBit::VSM) {
+            variant.key &= ~VSM;
+        }
     }
-
     if (!isSSRVariant(variant)) {
         // SSR variant needs to be handled separately
-        if (filterMask & uint32_t(UserVariantFilterBit::SHADOW_RECEIVER)) {
+        if (filterMask & (uint32_t)UserVariantFilterBit::SHADOW_RECEIVER) {
             variant.key &= ~SRE;
         }
-        if (filterMask & uint32_t(UserVariantFilterBit::VSM)) {
-            variant.key &= ~S2D;
+        if (filterMask & (uint32_t)UserVariantFilterBit::VSM) {
+            variant.key &= ~VSM;
         }
     } else {
         // see if we need to filter out the SSR variants
-        if (filterMask & uint32_t(UserVariantFilterBit::SSR)) {
-            variant.key &= ~SPECIAL_SSR_VARIANT;
+        if (filterMask & (uint32_t)UserVariantFilterBit::SSR) {
+            variant.key &= ~SPECIAL_SSR;
         }
     }
     return variant;

libs/filamat/src/shaders/ShaderGenerator.cpp

@@ -60,8 +60,7 @@ void ShaderGenerator::generateSurfaceMaterialVariantDefines(io::sstream& out,
     CodeGenerator::generateDefine(out, "VARIANT_HAS_SHADOWING",
             litVariants && filament::Variant::isShadowReceiverVariant(variant));
     CodeGenerator::generateDefine(out, "VARIANT_HAS_VSM",
-            filament::Variant::isShadowSampler2DVariant(variant) ||
-            filament::Variant::isDepthMomentsVariant(variant));
+            filament::Variant::isVSMVariant(variant));
     CodeGenerator::generateDefine(out, "VARIANT_HAS_STEREO",
             hasStereo(variant, featureLevel));
     CodeGenerator::generateDefine(out, "VARIANT_DEPTH",

libs/filamat/src/shaders/SibGenerator.cpp

@@ -49,7 +49,7 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
     // reason we name them "unused*" to ensure we're not using them by mistake (type/format don't
     // matter).
 
-    static SamplerInterfaceBlock const sibShadowSamplerPcf{ SamplerInterfaceBlock::Builder()
+    static SamplerInterfaceBlock const sibPcf{ SamplerInterfaceBlock::Builder()
             .name("sampler0")
             .stageFlags(backend::ShaderStageFlags::FRAGMENT)
             .add(  {{ "shadowMap",   +PerViewBindingPoints::SHADOW_MAP,     Type::SAMPLER_2D_ARRAY, Format::SHADOW, Precision::MEDIUM, FILTERABLE, !MULTISAMPLE, ALL_STAGES },
@@ -62,7 +62,7 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
             )
             .build() };
 
-    static SamplerInterfaceBlock const sibShadowSampler2D{ SamplerInterfaceBlock::Builder()
+    static SamplerInterfaceBlock const sibVsm{ SamplerInterfaceBlock::Builder()
             .name("sampler0")
             .stageFlags(backend::ShaderStageFlags::FRAGMENT)
             .add(  {{ "shadowMap",   +PerViewBindingPoints::SHADOW_MAP,     Type::SAMPLER_2D_ARRAY, Format::FLOAT,  Precision::HIGH,   FILTERABLE, !MULTISAMPLE, ALL_STAGES },
@@ -85,10 +85,10 @@ SamplerInterfaceBlock const& SibGenerator::getPerViewSib(Variant variant) noexce
 
     if (Variant::isSSRVariant(variant)) {
         return sibSsr;
-    } else if (Variant::isShadowSampler2DVariant(variant)) {
-        return sibShadowSampler2D;
+    } else if (Variant::isVSMVariant(variant)) {
+        return sibVsm;
     } else {
-        return sibShadowSamplerPcf;
+        return sibPcf;
     }
 }
 

libs/matdbg/src/CommonWriter.cpp

@@ -48,11 +48,10 @@ std::string formatVariantString(Variant variant, MaterialDomain domain) noexcept
         if (variant.key & Variant::DEP) variantString += "DEP|";
         if (variant.key & Variant::DEP) {
             if (variant.key & Variant::PCK) variantString += "PCK|";
-            if (variant.key & Variant::MNT) variantString += "MNT|";
         } else {
             if (variant.key & Variant::FOG) variantString += "FOG|";
-            if (variant.key & Variant::S2D) variantString += "S2D|";
         }
+        if (variant.key & Variant::VSM) variantString += "VSM|";
         if (variant.key & Variant::STE) variantString += "STE|";
         variantString = variantString.substr(0, variantString.length() - 1);
     }

test/renderdiff/README.md

@@ -76,7 +76,7 @@ into **branch** of `filament-assets`. This branch is paired with a PR or commit
 
 As an example, imagine I am working on a PR, and I've uploaded my change, which is in a
 branch called `my-pr-branch`, to `filament`. This PR requires updating the golden. We would do
-it in the following fashion on a macOS machine:
+it in the following fashion
 
 ### Using a script to update the golden repo