vk: delay destruction

And fix other python bugs

RDIFF_ACCEPT_NEW_GOLDENS

NEW_RELEASE_NOTES.md

@@ -6,3 +6,6 @@
 appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md).
 
 ## Release notes for next branch cut
+
+- engine: fix crash when using variance shadow maps
+- materials: better shadow normal-bias calculations [⚠️ **New Material Version**]

docs/dup/intro.html

@@ -181,7 +181,7 @@ important for <code>matc</code> (material compiler).</p>
 }
 
 dependencies {
-    implementation 'com.google.android.filament:filament-android:1.69.3'
+    implementation 'com.google.android.filament:filament-android:1.69.4'
 }
 </code></pre>
 <p>Here are all the libraries available in the group <code>com.google.android.filament</code>:</p>
@@ -195,7 +195,7 @@ dependencies {
 </div>
 <h3 id="ios"><a class="header" href="#ios">iOS</a></h3>
 <p>iOS projects can use CocoaPods to install the latest release:</p>
-<pre><code class="language-shell">pod 'Filament', '~&gt; 1.69.3'
+<pre><code class="language-shell">pod 'Filament', '~&gt; 1.69.4'
 </code></pre>
 <h2 id="documentation"><a class="header" href="#documentation">Documentation</a></h2>
 <ul>

docs/dup/test_ci_renderdiff.html

@@ -228,6 +228,7 @@ branch called <code>my-pr-branch</code>, to <code>filament</code>. This PR requi
 it in the following fashion</p>
 <h3 id="using-a-script-to-update-the-golden-repo"><a class="header" href="#using-a-script-to-update-the-golden-repo">Using a script to update the golden repo</a></h3>
 <ul>
+<li>Make sure you've completed the steps in 'Setting up python'</li>
 <li>Run interactive mode in the <code>update_golden.py</code> script.
 <pre><code>python3 test/renderdiff/src/update_golden.py
 </code></pre>

filament/backend/src/opengl/OpenGLContext.h

@@ -91,7 +91,7 @@ public:
         GLenum getIndicesType() const noexcept {
             return indicesType;
         }
-    } gl;
+    };
 
     static bool queryOpenGLVersion(GLint* major, GLint* minor) noexcept;
 

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/ShadowMap.cpp

@@ -213,12 +213,14 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine,
      * Final shadow map transform
      */
 
-    // Final shadow transform (focused warped light-space)
+    // Final shadow transform (focused warped light-space), world space to clip space
     const mat4f S = F * (W * LMpMv);
 
     // Computes St the transform to use in the shader to access the shadow map texture
     // i.e. it transforms a world-space vertex to a texture coordinate in the shadowmap
     const auto [Mt, Mn] = getTextureCoordsMapping(shadowMapInfo, getViewport());
+
+    // world space to texture atlas space
     const mat4f St = highPrecisionMultiply(Mt, S);
 
     ShaderParameters shaderParameters;
@@ -226,14 +228,12 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine,
 
     // note: in texelSizeWorldSpace() below, we can use Mb * Mt * F * W because
     // L * Mp * Mv is a rigid transform for directional lights, and doesn't matter.
-    // if Wp[3][1] is 0, then LISPSM was cancelled.
+    // if Wp[3][1] is 0, then LiSPSM was canceled.
     if (useLispsm && Wp[3][1] != 0.0f) {
-        shaderParameters.texelSizeAtOneMeterWs =
-                texelSizeWorldSpace(Wp, mat4f(Mt * F), shadowMapInfo.shadowDimension);
+        shaderParameters.texelSizeAtOneMeterWs = texelSizeWorldSpace(S, shadowMapInfo.shadowDimension);
     } else {
         // We know we're using an ortho projection
-        shaderParameters.texelSizeAtOneMeterWs =
-                texelSizeWorldSpace(St.upperLeft(), shadowMapInfo.shadowDimension);
+        shaderParameters.texelSizeAtOneMeterWs = texelSizeWorldSpace(S.upperLeft(), shadowMapInfo.shadowDimension);
     }
     if (!shadowMapInfo.vsm) {
         shaderParameters.lightSpace = St;
@@ -1117,84 +1117,67 @@ bool ShadowMap::intersectSegmentWithPlanarQuad(float3& UTILS_RESTRICT p,
     return hit;
 }
 
-float ShadowMap::texelSizeWorldSpace(const mat3f& worldToShadowTexture,
-        uint16_t shadowDimension) noexcept {
+float2 ShadowMap::texelSizeWorldSpace(const mat3f& clipFromWorld, uint16_t shadowDimension) noexcept {
     // The Jacobian of the transformation from texture-to-world is the matrix itself for
-    // orthographic projections. We just need to inverse worldToShadowTexture,
+    // orthographic projections. We just need to inverse shadowMapFromWorld,
     // which is guaranteed to be orthographic.
     // The two first columns give us how a texel maps in world-space.
-    const float ures = 1.0f / float(shadowDimension);
-    const float vres = 1.0f / float(shadowDimension);
-    const mat3f shadowTextureToWorld(inverse(worldToShadowTexture));
-    const float3 Jx = shadowTextureToWorld[0];
-    const float3 Jy = shadowTextureToWorld[1];
-    const float s = std::max(length(Jx) * ures, length(Jy) * vres);
+    float const oneTexel = 2.0f / float(shadowDimension);
+    mat3f const worldFromClip(inverse(clipFromWorld));
+    float3 const Jx = worldFromClip[0];
+    float3 const Jy = worldFromClip[1];
+    float2 const s = float2{ length(Jx), length(Jy) } * oneTexel;
     return s;
 }
 
-float ShadowMap::texelSizeWorldSpace(const mat4f& Wp, const mat4f& MbMtF,
-        uint16_t shadowDimension) noexcept {
-    // Here we compute the Jacobian of inverse(MbMtF * Wp).
-    // The expression below has been computed with Mathematica. However, it's not very hard,
-    // albeit error-prone, to do it by hand because MbMtF is a linear transform.
-    // So we really only need to calculate the Jacobian of inverse(Wp) at inverse(MbMtF).
-    //
-    // Because we're only interested in the length of the columns of the Jacobian, we can use
-    // Mb * Mt * F * Wp instead of the full expression Mb * Mt * F * Wp * Wv * L * Mp * Mv,
-    // because Wv * L * Mp * Mv is a rigid transform, which doesn't affect the length of
-    // the Jacobian's column vectors.
+/**
+ * Calculates the Jacobian matrix J = ∂(u,v,d)/∂(x,y,z) for a 4x4 perspective projection.
+ *
+ * Given a view-space point P = [x, y, z, 1]^T and a projection matrix M,
+ * the projected homogeneous coordinates are [X, Y, Z, W]^T = M * P.
+ * The resulting NDC coordinates are u = X/W, v = Y/W, and depth d = Z/W.
+ *
+ * To find the Jacobian on the CPU, we apply the quotient rule to each component:
+ * ∂(X/W) / ∂xi = ( (∂X/∂xi) * W - X * (∂W/∂xi) ) / W^2
+ *
+ * In matrix form, this can be expressed as:
+ * J = (1/W) * [ M_sub - (1/W) * (T ⊗ w_grad) ]
+ *
+ * Where:
+ * - W: The homogeneous w-component after projection (usually -z for standard mats).
+ * - M_sub: The top-left 3x3 submatrix of M.
+ * - T: The column vector [X, Y, Z]^T (pre-perspective divide).
+ * - w_grad: The row vector [m30, m31, m32] (the first three elements of M's last row).
+ * - ⊗: The outer product, resulting in a 3x3 matrix.
+ *
+ * This Jacobian describes the local "stretch" of the projection. For LiSPSM or
+ * shadow mapping, the inverse Jacobian J^-1 provides the world-space footprint
+ * of a shadow texel, which is essential for calculating an accurate,
+ * non-constant depth bias to eliminate shadow acne.
+ *
+ * @param M The 4x4 projection matrix.
+ * @param p The 3D point in view-space where the Jacobian is evaluated.
+ * @return A 3x3 matrix representing the partial derivatives of (u,v,d) w.r.t (x,y,z).
+ */
+static mat3f jacobian(mat4f const& M, float3 const& p) noexcept {
+    float4 const T = M * p;
+    mat3f const M_sub = M.upperLeft();
+    float3 const w_grad = { M[0].w, M[1].w, M[2].w };
+    mat3f const t_cross_w{
+        w_grad.x * T.xyz,
+        w_grad.y * T.xyz,
+        w_grad.z * T.xyz
+    };
+    return (M_sub - t_cross_w / T.w) / T.w;
+}
 
+float2 ShadowMap::texelSizeWorldSpace(mat4f const& S, uint16_t const shadowDimension) noexcept {
     // The Jacobian is not constant, so we evaluate it in the center of the shadow-map texture.
     // It might be better to do this computation in the vertex shader.
-    float3 const p = { 0.5f, 0.5f, 0.0f };
-
-    const float ures = 1.0f / float(shadowDimension);
-    const float vres = 1.0f / float(shadowDimension);
-    const float dres = 1.0f / 65536.0f;
-
-    constexpr bool JACOBIAN_ESTIMATE = false;
-    if constexpr (JACOBIAN_ESTIMATE) {
-        // This estimates the Jacobian -- this is a lot heavier. This is mostly for reference
-        // and testing.
-        const mat4f Si(inverse(MbMtF * Wp));
-        const float3 p0 = mat4f::project(Si, p);
-        const float3 p1 = mat4f::project(Si, p + float3{ 1, 0, 0 } * ures);
-        const float3 p2 = mat4f::project(Si, p + float3{ 0, 1, 0 } * vres);
-        const float3 p3 = mat4f::project(Si, p + float3{ 0, 0, 1 } * dres);
-        const float3 Jx = p1 - p0;
-        const float3 Jy = p2 - p0;
-        const float3 UTILS_UNUSED Jz = p3 - p0;
-        const float s = std::max(length(Jx), length(Jy));
-        return s;
-    }
-
-    const float n = Wp[0][0];
-    const float A = Wp[1][1];
-    const float B = Wp[3][1];
-    const float sx = MbMtF[0][0];
-    const float sy = MbMtF[1][1];
-    const float sz = MbMtF[2][2];
-    const float ox = MbMtF[3][0];
-    const float oy = MbMtF[3][1];
-    const float oz = MbMtF[3][2];
-
-    const float X = p.x - ox;
-    const float Y = p.y - oy;
-    const float Z = p.z - oz;
-
-    const float dz = A * sy - Y;
-    const float nsxsz = n * sx * sz;
-    const float j = -(B * sy) / (nsxsz * dz * dz);
-    const mat3f J(mat3f::row_major_init{
-            j * dz * sz,    j * X * sz,     0.0f,
-            0.0f,           j * nsxsz,      0.0f,
-            0.0f,           j * Z * sx,     j * dz * sx
-    });
-
-    float3 const Jx = J[0] * ures;
-    float3 const Jy = J[1] * vres;
-    UTILS_UNUSED float3 const Jz = J[2] * dres;
-    const float s = std::max(length(Jx), length(Jy));
+    float3 const p = { 0.0f, 0.0f, 0.0f }; // clip-space
+    float const oneTexel = 2.0f / float(shadowDimension);
+    mat3f const J = jacobian(inverse(S), p);
+    float2 const s = float2{ length(J[0]), length(J[1]) } * oneTexel;
     return s;
 }
 

filament/src/ShadowMap.h

@@ -135,7 +135,7 @@ public:
         math::mat4f lightSpace{};
         math::float4 lightFromWorldZ{};
         math::float4 scissorNormalized{};
-        float texelSizeAtOneMeterWs{};
+        math::float2 texelSizeAtOneMeterWs{};
     };
 
     // Call once per frame if the light, scene (or visible layers) or camera changes.
@@ -319,11 +319,8 @@ private:
 
     math::float4 getClampToEdgeCoords(ShadowMapInfo const& shadowMapInfo) const noexcept;
 
-    static float texelSizeWorldSpace(const math::mat3f& worldToShadowTexture,
-            uint16_t shadowDimension) noexcept;
-
-    static float texelSizeWorldSpace(const math::mat4f& W, const math::mat4f& MbMtF,
-            uint16_t shadowDimension) noexcept;
+    static math::float2 texelSizeWorldSpace(const math::mat3f& clipFromWorld, uint16_t shadowDimension) noexcept;
+    static math::float2 texelSizeWorldSpace(const math::mat4f& clipFromWorld, uint16_t shadowDimension) noexcept;
 
     static constexpr Segment sBoxSegments[12] = {
             { 0, 1 }, { 1, 3 }, { 3, 2 }, { 2, 0 },

filament/src/ShadowMapManager.cpp

@@ -742,17 +742,16 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng
 
                 // Texel size is constant for directional light (although that's not true when LISPSM
                 // is used, but in that case we're pretending it is).
-                const float wsTexelSize = shaderParameters.texelSizeAtOneMeterWs;
+                float2 const wsTexelSize = shaderParameters.texelSizeAtOneMeterWs;
 
                 auto& s = mShadowUb.edit();
                 s.shadows[shadowIndex].layer = shadowMap.getLayer();
                 s.shadows[shadowIndex].lightFromWorldMatrix = shaderParameters.lightSpace;
                 s.shadows[shadowIndex].scissorNormalized = shaderParameters.scissorNormalized;
-                s.shadows[shadowIndex].normalBias = normalBias * wsTexelSize;
-                s.shadows[shadowIndex].texelSizeAtOneMeter = wsTexelSize;
+                s.shadows[shadowIndex].normalBias = wsTexelSize * normalBias;
                 s.shadows[shadowIndex].elvsm = options.vsm.elvsm;
                 s.shadows[shadowIndex].bulbRadiusLs =
-                        mSoftShadowOptions.penumbraScale * options.shadowBulbRadius / wsTexelSize;
+                        mSoftShadowOptions.penumbraScale * options.shadowBulbRadius / length(wsTexelSize);
 
                 shadowTechnique |= ShadowTechnique::SHADOW_MAP;
                 cascadeHasVisibleShadows |= 0x1u << i;
@@ -833,7 +832,7 @@ void ShadowMapManager::prepareSpotShadowMap(ShadowMap& shadowMap, FEngine& engin
     // and if we need to generate it, update all the UBO data
     if (shadowMap.hasVisibleShadows()) {
         const size_t shadowIndex = shadowMap.getShadowIndex();
-        const float wsTexelSizeAtOneMeter = shaderParameters.texelSizeAtOneMeterWs;
+        const float2 wsTexelSizeAtOneMeter = shaderParameters.texelSizeAtOneMeterWs;
         // note: normalBias is set to zero for VSM
         const float normalBias = shadowMapInfo.vsm ? 0.0f : options->normalBias;
 
@@ -845,12 +844,11 @@ void ShadowMapManager::prepareSpotShadowMap(ShadowMap& shadowMap, FEngine& engin
         s.shadows[shadowIndex].scissorNormalized = shaderParameters.scissorNormalized;
         s.shadows[shadowIndex].normalBias = normalBias * wsTexelSizeAtOneMeter;
         s.shadows[shadowIndex].lightFromWorldZ = shaderParameters.lightFromWorldZ;
-        s.shadows[shadowIndex].texelSizeAtOneMeter = wsTexelSizeAtOneMeter;
         s.shadows[shadowIndex].nearOverFarMinusNear = float(n / (f - n));
         s.shadows[shadowIndex].elvsm = options->vsm.elvsm;
         s.shadows[shadowIndex].bulbRadiusLs =
                 mSoftShadowOptions.penumbraScale * options->shadowBulbRadius
-                        / wsTexelSizeAtOneMeter;
+                        / length(wsTexelSizeAtOneMeter);
 
     }
 }
@@ -926,7 +924,7 @@ void ShadowMapManager::preparePointShadowMap(ShadowMap& shadowMap,
     // and if we need to generate it, update all the UBO data
     if (shadowMap.hasVisibleShadows()) {
         const size_t shadowIndex = shadowMap.getShadowIndex();
-        const float wsTexelSizeAtOneMeter = shaderParameters.texelSizeAtOneMeterWs;
+        const float2 wsTexelSizeAtOneMeter = shaderParameters.texelSizeAtOneMeterWs;
         // note: normalBias is set to zero for VSM
         const float normalBias = shadowMapInfo.vsm ? 0.0f : options->normalBias;
 
@@ -938,12 +936,11 @@ void ShadowMapManager::preparePointShadowMap(ShadowMap& shadowMap,
         s.shadows[shadowIndex].scissorNormalized = shaderParameters.scissorNormalized;
         s.shadows[shadowIndex].normalBias = normalBias * wsTexelSizeAtOneMeter;
         s.shadows[shadowIndex].lightFromWorldZ = shaderParameters.lightFromWorldZ;
-        s.shadows[shadowIndex].texelSizeAtOneMeter = wsTexelSizeAtOneMeter;
         s.shadows[shadowIndex].nearOverFarMinusNear = float(n / (f - n));
         s.shadows[shadowIndex].elvsm = options->vsm.elvsm;
         s.shadows[shadowIndex].bulbRadiusLs =
                 mSoftShadowOptions.penumbraScale * options->shadowBulbRadius
-                        / wsTexelSizeAtOneMeter;
+                        / length(wsTexelSizeAtOneMeter);
     }
 }
 

filament/src/details/View.cpp

@@ -143,7 +143,7 @@ FView::FView(FEngine& engine)
 #ifndef NDEBUG
     // This can fail if another view has already registered this data source
     mDebugState->owner = debugRegistry.registerDataSource("d.view.frame_info",
-            [weak = std::weak_ptr(mDebugState)]() -> DebugRegistry::DataSource {
+            [weak = std::weak_ptr<DebugState>(mDebugState)]() -> DebugRegistry::DataSource {
                 // the View could have been destroyed by the time we do this
                 auto const state = weak.lock();
                 if (!state) {

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

@@ -299,10 +299,9 @@ struct ShadowUib { // NOLINT(cppcoreguidelines-pro-type-member-init)
         math::mat4f lightFromWorldMatrix;       // 64
         math::float4 lightFromWorldZ;           // 16
         math::float4 scissorNormalized;         // 16
-        float texelSizeAtOneMeter;              //  4
         float bulbRadiusLs;                     //  4
         float nearOverFarMinusNear;             //  4
-        float normalBias;                       //  4
+        math::float2 normalBias;                //  4
         bool elvsm;                             //  4
         uint32_t layer;                         //  4
         uint32_t reserved1;                     //  4

shaders/src/surface_getters.fs

@@ -111,7 +111,7 @@ highp vec4 getSpotLightSpacePosition(int index, highp vec3 dir, highp float zLig
     highp mat4 lightFromWorldMatrix = shadowUniforms.shadows[index].lightFromWorldMatrix;
 
     // for spotlights, the bias depends on z
-    float bias = shadowUniforms.shadows[index].normalBias * zLight;
+    highp vec2 bias = shadowUniforms.shadows[index].normalBias * zLight;
 
     return computeLightSpacePosition(getWorldPosition(), getWorldGeometricNormalVector(),
             dir, bias, lightFromWorldMatrix);

shaders/src/surface_shadowing.glsl

@@ -12,12 +12,51 @@
  */
 
 highp vec4 computeLightSpacePosition(highp vec3 p, const highp vec3 n,
-        const highp vec3 dir, const float b, highp_mat4 lightFromWorldMatrix) {
+        const highp vec3 dir, const highp vec2 b, highp_mat4 lightFromWorldMatrix) {
 
 #if !defined(VARIANT_HAS_VSM)
-    highp float cosTheta = saturate(dot(n, dir));
-    highp float sinTheta = sqrt(1.0 - cosTheta * cosTheta);
-    p += n * (sinTheta * b);
+    // --------------------------------------------------------------------------------------
+    // Anisotropic Normal Bias for Shadow Mapping
+    // --------------------------------------------------------------------------------------
+    // To prevent shadow acne, we must push the geometry along its normal to clear the
+    // quantization steps of the shadow map's discrete depth grid. The exact physical depth
+    // error we must clear is proportional to the shadow texel's world-space dimensions.
+    //
+    // This implementation computes the exact geometric projection of the rectangular
+    // shadow map texel onto the surface normal.
+    //
+    // 1. Coordinate Space Transition:
+    //    We project the world-space normal onto the light's X and Y basis vectors (L_right,
+    //    L_up). This gives us the lateral components of the normal in Light Space (n_Lx, n_Ly).
+    //
+    // 2. The Implicit sin(theta) Slope Scale:
+    //    Because the normal is a unit vector, the magnitude of its lateral components in
+    //    light space inherently equals sin(theta), where theta is the angle of incidence.
+    //    This perfectly and automatically scales the bias from 0.0 (top-down, flat surface)
+    //    to maximum (grazing angle).
+    //
+    // 3. Exact Anisotropic Footprint (The L1 Norm):
+    //    Shadow texels are rarely perfectly square due to Cascaded Shadow Maps (CSM) or
+    //    Light Space Perspective Shadow Maps (LiSPSM). Jx and Jy are the physical world-space
+    //    dimensions of the texel.
+    //    By evaluating `abs(n_Lx * Jx) + abs(n_Ly * Jy)`, we compute the exact scalar
+    //    projection of the rectangular texel footprint.
+    //      - It is superior to `max(Jx, Jy)` which assumes a massive square and causes Peter Panning.
+    //      - It is superior to `length()` which assumes an ellipse and under-biases the corners.
+    // --------------------------------------------------------------------------------------
+
+    // Extract the first row (Light's Right vector in World Space)
+    highp vec3 L_right = vec3(lightFromWorldMatrix[0][0], lightFromWorldMatrix[1][0], lightFromWorldMatrix[2][0]);
+
+    // Extract the second row (Light's Up vector in World Space)
+    highp vec3 L_up    = vec3(lightFromWorldMatrix[0][1], lightFromWorldMatrix[1][1], lightFromWorldMatrix[2][1]);
+
+    // Project the world normal onto the shadow map's 2D grid
+    highp float n_Lx = dot(n, L_right);
+    highp float n_Ly = dot(n, L_up);
+
+    // Apply the anisotropic normal bias
+    p += n * (abs(n_Lx * b.x) + abs(n_Ly * b.y));
 #endif
 
     return mulMat4x4Float3(lightFromWorldMatrix, p);

shaders/src/surface_types.glsl

@@ -5,10 +5,9 @@ struct ShadowData {
     highp mat4 lightFromWorldMatrix;
     highp vec4 lightFromWorldZ;
     highp vec4 scissorNormalized;
-    mediump float texelSizeAtOneMeter;
     mediump float bulbRadiusLs;
     mediump float nearOverFarMinusNear;
-    mediump float normalBias;
+    highp vec2 normalBias;
     bool elvsm;
     mediump uint layer;
     mediump uint reserved1;

test/renderdiff/README.md

@@ -80,6 +80,7 @@ it in the following fashion
 
 ### Using a script to update the golden repo
 
+- Make sure you've completed the steps in 'Setting up python'
 - Run interactive mode in the `update_golden.py` script.
   ```
   python3 test/renderdiff/src/update_golden.py

test/renderdiff/src/golden_manager.py

@@ -16,6 +16,7 @@ import os
 import shutil
 import re
 import sys
+import tempfile
 
 from utils import execute, ArgParseImpl, mkdir_p
 
@@ -99,10 +100,11 @@ class GoldenManager:
     code, old_commit = execute(f'git log --format=%B -n 1', cwd=assets_dir)
     if tag and len(tag) > 0:
       old_commit += f'\nFILAMENT={tag}'
-      COMMIT_FILE = '/tmp/golden_commit.txt'
-      with open(COMMIT_FILE, 'w') as f:
+      with tempfile.NamedTemporaryFile('w', delete=False) as f:
         f.write(old_commit)
-      self._git_exec(f'commit --amend -F {COMMIT_FILE}')
+        commit_file = f.name
+      self._git_exec(f'commit --amend -F {commit_file}')
+      os.remove(commit_file)
 
     # Do the actual merge
     self._git_exec(f'checkout main')
@@ -139,11 +141,11 @@ class GoldenManager:
           os.path.join(rdiff_dir, f))
         self._git_exec(f'add {os.path.join(GOLDENS_DIR, f)}')
 
-    TMP_GOLDEN_COMMIT_FILE = '/tmp/golden_commit.txt'
-
-    with open(TMP_GOLDEN_COMMIT_FILE, 'w') as f:
+    with tempfile.NamedTemporaryFile('w', delete=False) as f:
       f.write(commit_msg)
-    self._git_exec(f'commit -a -F {TMP_GOLDEN_COMMIT_FILE}')
+      tmp_golden_commit_file = f.name
+    self._git_exec(f'commit -a -F {tmp_golden_commit_file}')
+    os.remove(tmp_golden_commit_file)
     if push_to_remote and \
        (self.access_token_ or self.access_type_ == ACCESS_TYPE_SSH):
       self._git_exec(f'push -f origin {branch}')

test/renderdiff/src/update_golden.py

@@ -64,19 +64,19 @@ def _do_update(golden_manager, config):
 
 def _same_image_diffimg(diffimg_path, img1, img2):
   cmd = [diffimg_path, img1, img2]
-  try:
-    result = subprocess.run(cmd, capture_output=True, text=True)
-    output = result.stdout.strip()
-    if not output:
-      return False
+  result = subprocess.run(cmd, capture_output=True, text=True)
+  if result.returncode != 0:
+    raise RuntimeError(f"diffimg failed with return code {result.returncode}:\n{result.stderr}")
 
-    try:
-      res_json = json.loads(output)
-      return res_json.get('passed', False)
-    except json.JSONDecodeError:
-      return False
-  except Exception:
-    return False
+  output = result.stdout.strip()
+  if not output:
+    raise RuntimeError("diffimg produced no output")
+
+  try:
+    res_json = json.loads(output)
+    return res_json.get('passed', False)
+  except json.JSONDecodeError as e:
+    raise RuntimeError(f"Failed to parse diffimg output: {e}\nOutput: {output}")
 
 def _get_deletes_updates(update_dir, golden_dir, diffimg_path):
   ret_delete = []
@@ -93,13 +93,13 @@ def _get_deletes_updates(update_dir, golden_dir, diffimg_path):
     # However, update_golden typically only updates/adds based on the new render set.
     # But strict sync might imply deleting missing ones.
     # The original logic was: delete = list(base - new).
-    delete = list(base - new)
+    delete_files = list(base_files - new_files)
 
     # Files in new but not in base are definitely updates (additions)
-    update = list(new - base)
+    update_files = list(new_files - base_files)
 
     # Files in both need comparison
-    for fpath in base.intersection(new_files):
+    for fpath in base_files.intersection(new_files):
       base_fpath = os.path.join(golden_dir, fpath)
       new_fpath = os.path.join(update_dir, fpath)
 
@@ -110,10 +110,10 @@ def _get_deletes_updates(update_dir, golden_dir, diffimg_path):
           is_different = _file_as_str(new_fpath) != _file_as_str(base_fpath)
 
       if is_different:
-        update.append(fpath)
+        ret_update.append(fpath)
 
-    ret_update += update
-    ret_delete += delete
+    ret_update += update_files
+    ret_delete += delete_files
 
   return ret_delete, ret_update
 
@@ -179,7 +179,7 @@ if __name__ == "__main__":
   parser = ArgParseImpl()
   parser.add_argument('--branch', help='Branch of the golden repo to write to')
   parser.add_argument('--golden-repo-token', help='Access token for the golden repo')
-  parser.add_argument('--push-to-remote', action="store_true", help='Access token for the golden repo')
+  parser.add_argument('--push-to-remote', action="store_true", help='Push the golden repo changes to remote')
   parser.add_argument('--diffimg', help='Path to the diffimg tool',
                       default='./out/cmake-release/tools/diffimg/diffimg')
 

test/renderdiff/src/utils.py

@@ -18,6 +18,7 @@ import shutil
 import argparse
 import sys
 import pathlib
+import shlex
 
 def execute(cmd,
             cwd=None,
@@ -25,7 +26,7 @@ def execute(cmd,
             stdin=None,
             env=None,
             raise_errors=False):
-  in_env = os.environ
+  in_env = os.environ.copy()
   in_env.update(env if env else {})
   home = os.environ['HOME']
   if f'{home}/bin' not in in_env['PATH']:
@@ -45,11 +46,11 @@ def execute(cmd,
       'universal_newlines': True
   }
   if capture_output:
-    process = subprocess.Popen(cmd.split(' '), **kwargs)
+    process = subprocess.Popen(shlex.split(cmd), **kwargs)
     output, err_output = process.communicate()
     return_code = process.returncode
   else:
-    return_code = subprocess.call(cmd.split(' '), **kwargs)
+    return_code = subprocess.call(shlex.split(cmd), **kwargs)
 
   if return_code:
     # Error