vk: delay destruction

And fix other python bugs

RDIFF_ACCEPT_NEW_GOLDENS

.github/actions/renderdiff-generate/action.yml

@@ -0,0 +1,26 @@
+name: 'Renderdiff Generate'
+description: 'Sets up prerequisites and runs the generate script for renderdiff'
+runs:
+  using: "composite"
+  steps:
+    - uses: ./.github/actions/mac-prereq
+    - uses: ./.github/actions/get-gltf-assets
+    - uses: ./.github/actions/get-mesa
+    - uses: ./.github/actions/get-vulkan-sdk
+    - name: Prerequisites
+      run: |
+        # Must have at least clang-16 for a webgpu/dawn build.
+        sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer
+      shell: bash
+    - name: Generate images
+      run: |
+        TEST_DIR=test/renderdiff
+        source ${TEST_DIR}/src/preamble.sh
+        set -eux
+        bash ${TEST_DIR}/generate.sh
+        set +eux
+      shell: bash
+    - name: Build diffimg tool
+      run: |
+        ./build.sh release diffimg
+      shell: bash

.github/workflows/postsubmit-main.yml

@@ -10,16 +10,27 @@ jobs:
   # a branch on filament-assets.
   update-renderdiff-goldens:
     name: update-renderdiff-goldens
-    runs-on: 'ubuntu-24.04-4core'
+    runs-on: 'macos-14-xlarge'
     steps:
       - uses: actions/checkout@v4.1.6
         with:
           fetch-depth: 0
-      - uses: ./.github/actions/linux-prereq
       - id: get_commit_msg
         uses: ./.github/actions/get-commit-msg
-      - name: Build diffimg
+      - name: Check if accepting new goldens
-        run: ./build.sh release diffimg
+        id: check_accept
+        env:
+          COMMIT_MESSAGE: ${{ steps.get_commit_msg.outputs.msg }}
+        run: |
+          if echo "${COMMIT_MESSAGE}" | python3 test/renderdiff/src/commit_msg.py --mode=accept_new_goldens; then
+            echo "accept=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "accept=false" >> "$GITHUB_OUTPUT"
+          fi
+        shell: bash
+      - name: Renderdiff Generate for new goldens
+        if: steps.check_accept.outputs.accept == 'true'
+        uses: ./.github/actions/renderdiff-generate
       - name: Run update script
         env:
           GH_TOKEN: ${{ secrets.FILAMENTBOT_TOKEN }}
@@ -27,10 +38,24 @@ jobs:
         run: |
           GOLDEN_BRANCH=$(echo "${COMMIT_MESSAGE}" | python3 test/renderdiff/src/commit_msg.py)
           COMMIT_HASH="${{ steps.get_commit_msg.outputs.hash }}"
-          if [[ "${GOLDEN_BRANCH}" != "main" ]]; then
+
           git config --global user.email "filament.bot@gmail.com"
           git config --global user.name "Filament Bot"
           git config --global credential.helper cache
+
+          if [[ "${{ steps.check_accept.outputs.accept }}" == "true" ]]; then
+              SHORT_HASH="${COMMIT_HASH:0:8}"
+              GOLDEN_BRANCH="accept-goldens-${SHORT_HASH}"
+              echo "Generating new goldens for branch ${GOLDEN_BRANCH}"
+              python3 test/renderdiff/src/update_golden.py \
+                  --branch=${GOLDEN_BRANCH} \
+                  --source=$(pwd)/out/renderdiff/renders \
+                  --commit-msg="Auto-update goldens from ${COMMIT_HASH}" \
+                  --push-to-remote \
+                  --golden-repo-token=${GH_TOKEN}
+          fi
+
+          if [[ "${GOLDEN_BRANCH}" != "main" ]]; then
               echo "branch==${GOLDEN_BRANCH}"
               echo "hash==${COMMIT_HASH}"
               python3 test/renderdiff/src/update_golden.py --branch=${GOLDEN_BRANCH} \

.github/workflows/presubmit.yml

@@ -126,18 +126,24 @@ jobs:
       - uses: actions/checkout@v4.1.6
         with:
           fetch-depth: 0
-      - uses: ./.github/actions/mac-prereq
-      - uses: ./.github/actions/get-gltf-assets
-      - uses: ./.github/actions/get-mesa
-      - uses: ./.github/actions/get-vulkan-sdk
       - id: get_commit_msg
         uses: ./.github/actions/get-commit-msg
-      - name: Prerequisites
+      - name: Check if accepting new goldens
+        id: check_accept
+        env:
+          COMMIT_MESSAGE: ${{ steps.get_commit_msg.outputs.msg }}
         run: |
-          # Must have at least clang-16 for a webgpu/dawn build.
+          if echo "${COMMIT_MESSAGE}" | python3 test/renderdiff/src/commit_msg.py --mode=accept_new_goldens; then
-          sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer
+            echo "accept=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "accept=false" >> "$GITHUB_OUTPUT"
+          fi
         shell: bash
+      - name: Renderdiff Generate
+        if: steps.check_accept.outputs.accept != 'true'
+        uses: ./.github/actions/renderdiff-generate
       - name: Render and compare
+        if: steps.check_accept.outputs.accept != 'true'
         id: render_compare
         env:
           COMMIT_MESSAGE: ${{ steps.get_commit_msg.outputs.msg }}
@@ -146,9 +152,6 @@ jobs:
           source ${TEST_DIR}/src/preamble.sh
           set -eux
           GOLDEN_BRANCH=$(echo "${COMMIT_MESSAGE}" | python3 ${TEST_DIR}/src/commit_msg.py)
-          bash ${TEST_DIR}/generate.sh
-          # Build diffimg tool
-          ./build.sh release diffimg
 
           python3 ${TEST_DIR}/src/golden_manager.py \
                   --branch=${GOLDEN_BRANCH} \
@@ -172,10 +175,12 @@ jobs:
           fi
         shell: bash
       - uses: actions/upload-artifact@v4
+        if: steps.check_accept.outputs.accept != 'true'
         with:
           name: presubmit-renderdiff-result
           path: ./out/renderdiff
       - name: Compare result
+        if: steps.check_accept.outputs.accept != 'true'
         run: |
           ERROR_STR="${{ steps.render_compare.outputs.err }}"
           if [ -n "${ERROR_STR}" ]; then

NEW_RELEASE_NOTES.md

@@ -8,3 +8,4 @@ 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**]

README.md

@@ -31,7 +31,7 @@ repositories {
 }
 
 dependencies {
-    implementation 'com.google.android.filament:filament-android:1.69.3'
+    implementation 'com.google.android.filament:filament-android:1.69.4'
 }
 ```
 
@@ -50,7 +50,7 @@ Here are all the libraries available in the group `com.google.android.filament`:
 iOS projects can use CocoaPods to install the latest release:
 
 ```shell
-pod 'Filament', '~> 1.69.3'
+pod 'Filament', '~> 1.69.4'
 ```
 
 ## Documentation

RELEASE_NOTES.md

@@ -7,6 +7,10 @@ A new header is inserted each time a *tag* is created.
 Instead, if you are authoring a PR for the main branch, add your release note to
 [NEW_RELEASE_NOTES.md](./NEW_RELEASE_NOTES.md).
 
+## v1.69.5
+
+- engine: fix crash when using variance shadow maps
+
 ## v1.69.4
 
 

android/gradle.properties

@@ -1,5 +1,5 @@
 GROUP=com.google.android.filament
-VERSION_NAME=1.69.3
+VERSION_NAME=1.69.4
 
 POM_DESCRIPTION=Real-time physically based rendering engine for Android.
 

build/common/upload-release-assets/package-lock.json

@@ -235,9 +235,9 @@
       "license": "MIT"
     },
     "node_modules/minimatch": {
-      "version": "3.1.2",
+      "version": "3.1.3",
-      "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz",
+      "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.3.tgz",
-      "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==",
+      "integrity": "sha512-M2GCs7Vk83NxkUyQV1bkABc4yxgz9kILhHImZiBPAZ9ybuvCb0/H7lEl5XvIg3g+9d4eNotkZA5IWwYl0tibaA==",
       "dependencies": {
         "brace-expansion": "^1.1.7"
       },
@@ -429,9 +429,9 @@
       "integrity": "sha512-nGqtvLrj5w0naR6tDPfB4cUmYCqouzyQiz6C5y/LtcDllJdrcc6WaWW6iXyIIOErTa/XRybj28aasdn4LkVk6Q=="
     },
     "minimatch": {
-      "version": "3.1.2",
+      "version": "3.1.3",
-      "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz",
+      "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.3.tgz",
-      "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==",
+      "integrity": "sha512-M2GCs7Vk83NxkUyQV1bkABc4yxgz9kILhHImZiBPAZ9ybuvCb0/H7lEl5XvIg3g+9d4eNotkZA5IWwYl0tibaA==",
       "requires": {
         "brace-expansion": "^1.1.7"
       }

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>
@@ -263,6 +264,27 @@ branch of the golden repo (i.e. <code>my-pr-branch-golden</code>).</li>
 <li>If the PR is merged, then there is another workflow that will merge <code>my-pr-branch-golden</code>
 to the <code>main</code> branch of the golden repo.</li>
 </ul>
+<h3 id="automated-update-via-commit-message"><a class="header" href="#automated-update-via-commit-message">Automated update via commit message</a></h3>
+<p>Alternatively, if you are confident in your changes and want the CI to handle the update
+for you, you can use the following tag in your commit message:</p>
+<ul>
+<li>In the commit message of your working branch on <code>filament</code>, add the following line:
+<pre><code>RDIFF_ACCEPT_NEW_GOLDENS
+</code></pre>
+</li>
+</ul>
+<p>This has the following effects:</p>
+<ul>
+<li>The presubmit test <code>test-renderdiff</code> will be bypassed (it will not perform rendering or
+comparison).</li>
+<li>When the PR is merged, the postsubmit CI will automatically:
+<ol>
+<li>Build Filament and generate the new images.</li>
+<li>Upload them to a temporary branch in <code>filament-assets</code>.</li>
+<li>Merge that branch into <code>main</code>.</li>
+</ol>
+</li>
+</ul>
 <h2 id="viewing-test-results"><a class="header" href="#viewing-test-results">Viewing test results</a></h2>
 <p>We provide a viewer for looking at the result of a test run. The viewer is a webapp that can
 be used by pointing your browser to a localhost port. If you input the viewer with a PR or a

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/VulkanContext.h

@@ -36,6 +36,7 @@ VK_DEFINE_HANDLE(VmaPool)
 namespace filament::backend {
 
 struct VulkanCommandBuffer;
+struct VulkanRenderPass;
 struct VulkanRenderTarget;
 struct VulkanSwapChain;
 struct VulkanTexture;
@@ -58,11 +59,11 @@ struct VulkanAttachment {
     VkImageSubresourceRange getSubresourceRange() const;
 };
 
-struct VulkanRenderPass {
+struct VulkanRenderPassContext {
     // Between the begin and end command render pass we cache the command buffer
     VulkanCommandBuffer* commandBuffer;
     fvkmemory::resource_ptr<VulkanRenderTarget> renderTarget;
-    VkRenderPass renderPass;
+    fvkmemory::resource_ptr<VulkanRenderPass> renderPass;
     RenderPassParams params;
     int currentSubpass;
 };

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,9 +444,27 @@ 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 {
+        fvkmemory::resource_ptr<VulkanDescriptorSetLayout> layout) const {
-    // TODO: fix the size for better memory management
+    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({
@@ -462,7 +476,9 @@ void VulkanDescriptorSetCache::copySet(VkDescriptorSet srcSet, VkDescriptorSet d
                 .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/VulkanDriver.cpp

@@ -1975,17 +1975,19 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
     rpkey.initialDepthLayout = currentDepthLayout;
     rpkey.subpassMask = uint8_t(params.subpassMask);
 
-    VkRenderPass renderPass = mFramebufferCache.getRenderPass(rpkey);
+    fvkmemory::resource_ptr<VulkanRenderPass> renderPass =
+            mFramebufferCache.getRenderPass(rpkey, &mResourceManager);
     mPipelineCache.bindRenderPass(renderPass, 0);
 
     // Create the VkFramebuffer or fetch it from cache.
     VulkanFboCache::FboKey fbkey = rt->getFboKey();
-    fbkey.renderPass = renderPass;
+    fbkey.renderPass = renderPass->getVkRenderPass();
     fbkey.layers = 1;
 
     rt->emitBarriersBeginRenderPass(*commandBuffer);
 
-    VkFramebuffer vkfb = mFramebufferCache.getFramebuffer(fbkey);
+    fvkmemory::resource_ptr<VulkanFramebuffer> vkfb =
+            mFramebufferCache.getFramebuffer(fbkey, &mResourceManager);
 
 // Assign a label to the framebuffer for debugging purposes.
 #if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS | FVK_DEBUG_DEBUG_UTILS)
@@ -1998,12 +2000,14 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
 
     // The current command buffer now has references to the render target and its attachments.
     commandBuffer->acquire(rt);
+    commandBuffer->acquire(renderPass);
+    commandBuffer->acquire(vkfb);
 
     // Populate the structures required for vkCmdBeginRenderPass.
     VkRenderPassBeginInfo renderPassInfo {
         .sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO,
-        .renderPass = renderPass,
+        .renderPass = renderPass->getVkRenderPass(),
-        .framebuffer = vkfb,
+        .framebuffer = vkfb->getVkFramebuffer(),
 
         // The renderArea field constrains the LoadOp, but scissoring does not.
         // Therefore, we do not set the scissor rect here, we only need it in draw().
@@ -2058,7 +2062,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
     mCurrentRenderPass = {
         .commandBuffer = commandBuffer,
         .renderTarget = rt,
-        .renderPass = renderPassInfo.renderPass,
+        .renderPass = renderPass,
         .params = params,
         .currentSubpass = 0,
     };
@@ -2078,8 +2082,7 @@ void VulkanDriver::endRenderPass(int) {
     rt->emitBarriersEndRenderPass(*mCurrentRenderPass.commandBuffer);
 
     mCurrentRenderPass.renderTarget = {};
-    mCurrentRenderPass.renderPass = VK_NULL_HANDLE;
+    mCurrentRenderPass.renderPass = {};
-
     mCurrentRenderPass.commandBuffer = nullptr;
 }
 
@@ -2244,7 +2247,7 @@ void VulkanDriver::resolve(
         Handle<HwTexture> src, uint8_t dstLevel, uint8_t dstLayer) {
     FVK_SYSTRACE_SCOPE();
 
-    FILAMENT_CHECK_PRECONDITION(mCurrentRenderPass.renderPass == VK_NULL_HANDLE)
+    FILAMENT_CHECK_PRECONDITION(!mCurrentRenderPass.renderPass)
             << "resolve() cannot be invoked inside a render pass.";
 
     auto srcTexture = resource_ptr<VulkanTexture>::cast(&mResourceManager, src);
@@ -2287,7 +2290,7 @@ void VulkanDriver::blit(
         math::uint2 size) {
     FVK_SYSTRACE_SCOPE();
 
-    FILAMENT_CHECK_PRECONDITION(mCurrentRenderPass.renderPass == VK_NULL_HANDLE)
+    FILAMENT_CHECK_PRECONDITION(!mCurrentRenderPass.renderPass)
             << "blit() cannot be invoked inside a render pass.";
 
     auto srcTexture = resource_ptr<VulkanTexture>::cast(&mResourceManager, src);
@@ -2329,7 +2332,7 @@ void VulkanDriver::blitDEPRECATED(TargetBufferFlags buffers,
 
     // Note: blitDEPRECATED is only used for Renderer::copyFrame()
 
-    FILAMENT_CHECK_PRECONDITION(mCurrentRenderPass.renderPass == VK_NULL_HANDLE)
+    FILAMENT_CHECK_PRECONDITION(!mCurrentRenderPass.renderPass)
             << "blitDEPRECATED() cannot be invoked inside a render pass.";
 
     FILAMENT_CHECK_PRECONDITION(buffers == TargetBufferFlags::COLOR0)

filament/backend/src/vulkan/VulkanDriver.h

@@ -139,7 +139,7 @@ private:
 
     resource_ptr<VulkanSwapChain> mCurrentSwapChain;
     resource_ptr<VulkanRenderTarget> mDefaultRenderTarget;
-    VulkanRenderPass mCurrentRenderPass = {};
+    VulkanRenderPassContext mCurrentRenderPass = {};
     VmaAllocator mAllocator = VK_NULL_HANDLE;
     VkDebugReportCallbackEXT mDebugCallback = VK_NULL_HANDLE;
 

filament/backend/src/vulkan/VulkanFboCache.cpp

@@ -17,6 +17,7 @@
 #include "VulkanFboCache.h"
 
 #include "VulkanConstants.h"
+#include "VulkanHandles.h"
 #include "vulkan/utils/Image.h"
 
 #include <utils/Panic.h>
@@ -69,9 +70,10 @@ VulkanFboCache::~VulkanFboCache() {
             << "Please explicitly call terminate() while the VkDevice is still alive.";
 }
 
-VkFramebuffer VulkanFboCache::getFramebuffer(FboKey const& config) noexcept {
+fvkmemory::resource_ptr<VulkanFramebuffer> VulkanFboCache::getFramebuffer(
+        FboKey const& config, fvkmemory::ResourceManager* resManager) noexcept {
     FboMap::iterator iter = mFramebufferCache.find(config);
-    if (UTILS_LIKELY(iter != mFramebufferCache.end() && iter->second.handle != VK_NULL_HANDLE)) {
+    if (UTILS_LIKELY(iter != mFramebufferCache.end())) {
         iter.value().timestamp = mCurrentTime;
         return iter->second.handle;
     }
@@ -117,13 +119,16 @@ VkFramebuffer VulkanFboCache::getFramebuffer(FboKey const& config) noexcept {
     VkResult error = vkCreateFramebuffer(mDevice, &info, VKALLOC, &framebuffer);
     FILAMENT_CHECK_POSTCONDITION(error == VK_SUCCESS) << "Unable to create framebuffer."
                                                      << " error=" << static_cast<int32_t>(error);
-    mFramebufferCache[config] = {framebuffer, mCurrentTime};
+    fvkmemory::resource_ptr<VulkanFramebuffer> fbh =
-    return framebuffer;
+        fvkmemory::resource_ptr<VulkanFramebuffer>::construct(resManager, mDevice, framebuffer);
+    mFramebufferCache[config] = {fbh, mCurrentTime};
+    return fbh;
 }
 
-VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey const& config) noexcept {
+fvkmemory::resource_ptr<VulkanRenderPass> VulkanFboCache::getRenderPass(
+        RenderPassKey const& config, fvkmemory::ResourceManager* resManager) noexcept {
     auto iter = mRenderPassCache.find(config);
-    if (UTILS_LIKELY(iter != mRenderPassCache.end() && iter->second.handle != VK_NULL_HANDLE)) {
+    if (UTILS_LIKELY(iter != mRenderPassCache.end())) {
         iter.value().timestamp = mCurrentTime;
         return iter->second.handle;
     }
@@ -326,7 +331,9 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey const& config) noexcept
     VkResult error = vkCreateRenderPass(mDevice, &renderPassInfo, VKALLOC, &renderPass);
     FILAMENT_CHECK_POSTCONDITION(error == VK_SUCCESS) << "Unable to create render pass."
                                                       << " error=" << error;
-    mRenderPassCache[config] = {renderPass, mCurrentTime};
+    fvkmemory::resource_ptr<VulkanRenderPass> rph =
+        fvkmemory::resource_ptr<VulkanRenderPass>::construct(resManager, mDevice, renderPass);
+    mRenderPassCache[config] = {rph, mCurrentTime};
 
 #if FVK_ENABLED(FVK_DEBUG_FBO_CACHE)
     FVK_LOGD << "Created render pass " << renderPass << " with ";
@@ -343,13 +350,12 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey const& config) noexcept
              << "colorAttachmentCount[0] = " << subpasses[0].colorAttachmentCount;
 #endif
 
-    return renderPass;
+    return rph;
 }
 
 void VulkanFboCache::resetFramebuffers() noexcept {
     for (const auto& pair: mFramebufferCache) {
         mRenderPassRefCount[pair.first.renderPass]--;
-        vkDestroyFramebuffer(mDevice, pair.second.handle, VKALLOC);
     }
     mFramebufferCache.clear();
 }
@@ -357,9 +363,6 @@ void VulkanFboCache::resetFramebuffers() noexcept {
 void VulkanFboCache::terminate() noexcept {
     resetFramebuffers();
 
-    for (const auto& pair: mRenderPassCache) {
-        vkDestroyRenderPass(mDevice, pair.second.handle, VKALLOC);
-    }
     mRenderPassRefCount.clear();
     mRenderPassCache.clear();
 }
@@ -380,8 +383,6 @@ void VulkanFboCache::gc() noexcept {
         const FboVal fbo = iter->second;
         if (fbo.timestamp < evictTime && fbo.handle) {
             mRenderPassRefCount[iter->first.renderPass]--;
-            vkDestroyFramebuffer(mDevice, fbo.handle, VKALLOC);
-            iter.value().handle = VK_NULL_HANDLE;
 
             // erase(iterator) returns the iterator to the next element.
             iter = mFramebufferCache.erase(iter);
@@ -391,11 +392,8 @@ void VulkanFboCache::gc() noexcept {
     }
 
     for (RenderPassMap::iterator iter = mRenderPassCache.begin(); iter != mRenderPassCache.end(); ) {
-        const VkRenderPass handle = iter->second.handle;
+        const VkRenderPass handle = iter->second.handle->getVkRenderPass();
         if (iter->second.timestamp < evictTime && handle && mRenderPassRefCount[handle] == 0) {
-            vkDestroyRenderPass(mDevice, handle, VKALLOC);
-            iter.value().handle = VK_NULL_HANDLE;
-
             // erase(iterator) returns the iterator to the next element.
             iter = mRenderPassCache.erase(iter);
             mRenderPassRefCount.erase(handle);

filament/backend/src/vulkan/VulkanFboCache.h

@@ -18,6 +18,9 @@
 #define TNT_FILAMENT_BACKEND_VULKANFBOCACHE_H
 
 #include "VulkanContext.h"
+#include "vulkan/memory/Resource.h"
+#include "vulkan/memory/ResourceManager.h"
+#include "vulkan/memory/ResourcePointer.h"
 
 #include <utils/Hash.h>
 
@@ -27,6 +30,9 @@
 
 namespace filament::backend {
 
+struct VulkanFramebuffer;
+struct VulkanRenderPass;
+
 // Simple manager for VkFramebuffer and VkRenderPass objects.
 //
 // Note that a VkFramebuffer is just a binding between a render pass and a set of image views. So,
@@ -57,7 +63,7 @@ public:
         uint8_t padding[2];
     };
     struct RenderPassVal {
-        VkRenderPass handle;
+        fvkmemory::resource_ptr<VulkanRenderPass> handle;
         uint32_t timestamp;
     };
     static_assert(0 == MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT % 8);
@@ -83,7 +89,7 @@ public:
         VkImageView depth; // 8 bytes
     };
     struct FboVal {
-        VkFramebuffer handle;
+        fvkmemory::resource_ptr<VulkanFramebuffer> handle;
         uint32_t timestamp;
     };
     static_assert(sizeof(VkRenderPass) == 8, "VkRenderPass has unexpected size.");
@@ -98,10 +104,12 @@ public:
     ~VulkanFboCache();
 
     // Retrieves or creates a VkFramebuffer handle.
-    VkFramebuffer getFramebuffer(FboKey const& config) noexcept;
+    fvkmemory::resource_ptr<VulkanFramebuffer> getFramebuffer(
+        FboKey const& config, fvkmemory::ResourceManager* resManager) noexcept;
 
     // Retrieves or creates a VkRenderPass handle.
-    VkRenderPass getRenderPass(RenderPassKey const& config) noexcept;
+    fvkmemory::resource_ptr<VulkanRenderPass> getRenderPass(
+        RenderPassKey const& config, fvkmemory::ResourceManager* resManager) noexcept;
 
     // Evicts old unused Vulkan objects. Call this once per frame.
     void gc() noexcept;

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) {
@@ -565,7 +565,7 @@ void VulkanRenderTarget::transformViewportToPlatform(VkViewport* bounds) const {
     flipVertically(bounds, getExtent().height);
 }
 
-uint8_t VulkanRenderTarget::getColorTargetCount(const VulkanRenderPass& pass) const {
+uint8_t VulkanRenderTarget::getColorTargetCount(const VulkanRenderPassContext& pass) const {
     if (!mOffscreen) {
         return 1;
     }
@@ -715,4 +715,18 @@ VulkanRenderPrimitive::VulkanRenderPrimitive(PrimitiveType pt,
       vertexBuffer(vb),
       indexBuffer(ib) {}
 
+VulkanFramebuffer::VulkanFramebuffer(VkDevice device, VkFramebuffer framebuffer)
+    : mDevice(device), mFramebuffer(framebuffer) {}
+
+VulkanFramebuffer::~VulkanFramebuffer() {
+    vkDestroyFramebuffer(mDevice, mFramebuffer, VKALLOC);
+}
+
+VulkanRenderPass::VulkanRenderPass(VkDevice device, VkRenderPass renderPass)
+    : mDevice(device), mRenderPass(renderPass) {}
+
+VulkanRenderPass::~VulkanRenderPass() {
+    vkDestroyRenderPass(mDevice, mRenderPass, VKALLOC);
+}
+
 } // namespace filament::backend

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
@@ -419,7 +420,7 @@ struct VulkanRenderTarget : private HwRenderTarget, fvkmemory::Resource {
         return mInfo->fbkey.samples;
     }
 
-    uint8_t getColorTargetCount(VulkanRenderPass const& pass) const;
+    uint8_t getColorTargetCount(VulkanRenderPassContext const& pass) const;
 
     inline bool hasDepth() const { return mInfo->depthIndex != Auxiliary::UNDEFINED_INDEX; }
 
@@ -585,6 +586,32 @@ struct VulkanRenderPrimitive : public HwRenderPrimitive, fvkmemory::Resource {
     fvkmemory::resource_ptr<VulkanIndexBuffer> indexBuffer;
 };
 
+struct VulkanFramebuffer : public fvkmemory::Resource {
+    VulkanFramebuffer(VkDevice device, VkFramebuffer framebuffer);
+    ~VulkanFramebuffer();
+
+    inline VkFramebuffer getVkFramebuffer() const noexcept {
+        return mFramebuffer;
+    }
+
+private:
+    VkDevice mDevice;
+    VkFramebuffer mFramebuffer;
+};
+
+struct VulkanRenderPass : public fvkmemory::Resource {
+    VulkanRenderPass(VkDevice device, VkRenderPass renderPass);
+    ~VulkanRenderPass();
+
+    inline VkRenderPass getVkRenderPass() const noexcept {
+        return mRenderPass;
+    }
+
+private:
+    VkDevice mDevice;
+    VkRenderPass mRenderPass;
+};
+
 } // namespace filament::backend
 
 #endif // TNT_FILAMENT_BACKEND_VULKANHANDLES_H

filament/backend/src/vulkan/VulkanPipelineCache.cpp

@@ -459,8 +459,10 @@ void VulkanPipelineCache::bindStencilState(StencilState const& stencilState) noe
     mPipelineRequirements.stencilState = stencilState;
 }
 
-void VulkanPipelineCache::bindRenderPass(VkRenderPass renderPass, int subpassIndex) noexcept {
+void VulkanPipelineCache::bindRenderPass(
-    mPipelineRequirements.renderPass = renderPass;
+        fvkmemory::resource_ptr<VulkanRenderPass> renderPass,
+        int subpassIndex) noexcept {
+    mPipelineRequirements.renderPass = renderPass->getVkRenderPass();
     mPipelineRequirements.subpassIndex = subpassIndex;
 }
 

filament/backend/src/vulkan/VulkanPipelineCache.h

@@ -123,7 +123,8 @@ public:
     void bindProgram(fvkmemory::resource_ptr<VulkanProgram> program) noexcept;
     void bindRasterState(RasterState const& rasterState) noexcept;
     void bindStencilState(StencilState const& stencilState) noexcept;
-    void bindRenderPass(VkRenderPass renderPass, int subpassIndex) noexcept;
+    void bindRenderPass(fvkmemory::resource_ptr<VulkanRenderPass> renderPass,
+            int subpassIndex) noexcept;
     void bindPrimitiveTopology(VkPrimitiveTopology topology) noexcept;
     void bindVertexArray(VkVertexInputAttributeDescription const* attribDesc,
             VkVertexInputBindingDescription const* bufferDesc, uint8_t count);

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 |
+            dstStage,
-                    VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
             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/Resource.cpp

@@ -42,6 +42,8 @@ template ResourceType getTypeEnum<VulkanSync>() noexcept;
 template ResourceType getTypeEnum<VulkanMemoryMappedBuffer>() noexcept;
 template ResourceType getTypeEnum<VulkanSemaphore>() noexcept;
 template ResourceType getTypeEnum<VulkanStream>() noexcept;
+template ResourceType getTypeEnum<VulkanFramebuffer>() noexcept;
+template ResourceType getTypeEnum<VulkanRenderPass>() noexcept;
 
 template<typename D>
 ResourceType getTypeEnum() noexcept {
@@ -108,6 +110,12 @@ ResourceType getTypeEnum() noexcept {
     if constexpr (std::is_same_v<D, VulkanStream>) {
         return ResourceType::STREAM;
     }
+    if constexpr (std::is_same_v<D, VulkanFramebuffer>) {
+        return ResourceType::FRAMEBUFFER;
+    }
+    if constexpr (std::is_same_v<D, VulkanRenderPass>) {
+        return ResourceType::RENDER_PASS;
+    }
     return ResourceType::UNDEFINED_TYPE;
 }
 
@@ -155,6 +163,10 @@ std::string_view getTypeStr(ResourceType type) {
             return "Semaphore";
         case ResourceType::STREAM:
             return "VulkanStream";
+        case ResourceType::FRAMEBUFFER:
+            return "Framebuffer";
+        case ResourceType::RENDER_PASS:
+            return "RenderPass";
         case ResourceType::UNDEFINED_TYPE:
             return "";
     }

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

@@ -56,7 +56,9 @@ enum class ResourceType : uint8_t {
     MEMORY_MAPPED_BUFFER = 18,
     SEMAPHORE = 19,
     STREAM = 20,
-    UNDEFINED_TYPE = 21,    // Must be the last enum because we use it for iterating over the enums.
+    FRAMEBUFFER = 21,
+    RENDER_PASS = 22,
+    UNDEFINED_TYPE = 23,    // Must be the last enum because we use it for iterating over the enums.
 };
 
 template<typename D>

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(frameGc.gcList, mGcList);
-    std::swap(gcs, mGcList);
+
-    destroyAll(gcs);
+    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()) {
+    auto destroyAll = [this](GcList& list) {
-        gc();
+        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);
     }
 }
 
@@ -126,6 +158,12 @@ void ResourceManager::destroyWithType(ResourceType type, HandleId id) {
         case ResourceType::STREAM:
             destruct<VulkanStream>(Handle<VulkanStream>(id));
             break;
+        case ResourceType::FRAMEBUFFER:
+            destruct<VulkanFramebuffer>(Handle<VulkanFramebuffer>(id));
+            break;
+        case ResourceType::RENDER_PASS:
+            destruct<VulkanRenderPass>(Handle<VulkanRenderPass>(id));
+            break;
         case ResourceType::UNDEFINED_TYPE:
             break;
     }

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/backend/test/test_Autoresolve.cpp

@@ -18,6 +18,7 @@
 
 #include "Shader.h"
 #include "SharedShaders.h"
+#include "Skip.h"
 #include "TrianglePrimitive.h"
 
 #include <backend/PixelBufferDescriptor.h>
@@ -31,6 +32,9 @@ using namespace filament::backend;
 using namespace filament::math;
 
 TEST_F(BackendTest, AutoresolveDifferingSampleCounts) {
+    SKIP_IF(SkipEnvironment(OperatingSystem::CI, Backend::OPENGL), "see b/486954356");
+    SKIP_IF(SkipEnvironment(OperatingSystem::CI, Backend::VULKAN), "see b/486954356");
+
     auto& api = getDriverApi();
     constexpr int kRenderTargetSize = 512;
 

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 =
+        shaderParameters.texelSizeAtOneMeterWs = texelSizeWorldSpace(S, shadowMapInfo.shadowDimension);
-                texelSizeWorldSpace(Wp, mat4f(Mt * F), shadowMapInfo.shadowDimension);
     } else {
         // We know we're using an ortho projection
-        shaderParameters.texelSizeAtOneMeterWs =
+        shaderParameters.texelSizeAtOneMeterWs = texelSizeWorldSpace(S.upperLeft(), shadowMapInfo.shadowDimension);
-                texelSizeWorldSpace(St.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,
+float2 ShadowMap::texelSizeWorldSpace(const mat3f& clipFromWorld, uint16_t shadowDimension) noexcept {
-        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);
+    float const oneTexel = 2.0f / float(shadowDimension);
-    const float vres = 1.0f / float(shadowDimension);
+    mat3f const worldFromClip(inverse(clipFromWorld));
-    const mat3f shadowTextureToWorld(inverse(worldToShadowTexture));
+    float3 const Jx = worldFromClip[0];
-    const float3 Jx = shadowTextureToWorld[0];
+    float3 const Jy = worldFromClip[1];
-    const float3 Jy = shadowTextureToWorld[1];
+    float2 const s = float2{ length(Jx), length(Jy) } * oneTexel;
-    const float s = std::max(length(Jx) * ures, length(Jy) * vres);
     return s;
 }
 
-float ShadowMap::texelSizeWorldSpace(const mat4f& Wp, const mat4f& MbMtF,
+/**
-        uint16_t shadowDimension) noexcept {
+ * Calculates the Jacobian matrix J = ∂(u,v,d)/∂(x,y,z) for a 4x4 perspective projection.
-    // Here we compute the Jacobian of inverse(MbMtF * Wp).
+ *
-    // The expression below has been computed with Mathematica. However, it's not very hard,
+ * Given a view-space point P = [x, y, z, 1]^T and a projection matrix M,
-    // albeit error-prone, to do it by hand because MbMtF is a linear transform.
+ * the projected homogeneous coordinates are [X, Y, Z, W]^T = M * P.
-    // So we really only need to calculate the Jacobian of inverse(Wp) at inverse(MbMtF).
+ * The resulting NDC coordinates are u = X/W, v = Y/W, and depth d = Z/W.
-    //
+ *
-    // Because we're only interested in the length of the columns of the Jacobian, we can use
+ * To find the Jacobian on the CPU, we apply the quotient rule to each component:
-    // Mb * Mt * F * Wp instead of the full expression Mb * Mt * F * Wp * Wv * L * Mp * Mv,
+ * ∂(X/W) / ∂xi = ( (∂X/∂xi) * W - X * (∂W/∂xi) ) / W^2
-    // because Wv * L * Mp * Mv is a rigid transform, which doesn't affect the length of
+ *
-    // the Jacobian's column vectors.
+ * 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 };
+    float3 const p = { 0.0f, 0.0f, 0.0f }; // clip-space
-
+    float const oneTexel = 2.0f / float(shadowDimension);
-    const float ures = 1.0f / float(shadowDimension);
+    mat3f const J = jacobian(inverse(S), p);
-    const float vres = 1.0f / float(shadowDimension);
+    float2 const s = float2{ length(J[0]), length(J[1]) } * oneTexel;
-    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));
     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,
+    static math::float2 texelSizeWorldSpace(const math::mat3f& clipFromWorld, uint16_t shadowDimension) noexcept;
-            uint16_t shadowDimension) noexcept;
+    static math::float2 texelSizeWorldSpace(const math::mat4f& clipFromWorld, uint16_t shadowDimension) noexcept;
-
-    static float texelSizeWorldSpace(const math::mat4f& W, const math::mat4f& MbMtF,
-            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].normalBias = wsTexelSize * normalBias;
-                s.shadows[shadowIndex].texelSizeAtOneMeter = wsTexelSize;
                 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/Engine.cpp

@@ -798,6 +798,12 @@ void FEngine::submitFrame() {
 void FEngine::flush() {
     // flush the command buffer
     flushCommandBuffer(mCommandBufferQueue);
+    
+    // In single-threaded mode, we have to call execute() to drain the command
+    // buffer to really free up space
+    if constexpr (!UTILS_HAS_THREADING) {
+        execute();
+    }
 }
 
 void FEngine::flushAndWait() {

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) {

ios/CocoaPods/Filament.podspec

@@ -1,12 +1,12 @@
 Pod::Spec.new do |spec|
   spec.name = "Filament"
-  spec.version = "1.69.3"
+  spec.version = "1.69.4"
   spec.license = { :type => "Apache 2.0", :file => "LICENSE" }
   spec.homepage = "https://google.github.io/filament"
   spec.authors = "Google LLC."
   spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL."
   spec.platform = :ios, "11.0"
-  spec.source = { :http => "https://github.com/google/filament/releases/download/v1.69.3/filament-v1.69.3-ios.tgz" }
+  spec.source = { :http => "https://github.com/google/filament/releases/download/v1.69.4/filament-v1.69.4-ios.tgz" }
 
   spec.libraries = 'c++'
 

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

libs/gltfio/src/FFilamentAsset.h

@@ -60,11 +60,13 @@
 #define GLTFIO_WARN(msg) slog.w << msg << io::endl
 #endif
 
+#ifndef GLTFIO_USE_FILESYSTEM
 #if defined(__EMSCRIPTEN__) || defined(__ANDROID__) || defined(FILAMENT_IOS)
 #define GLTFIO_USE_FILESYSTEM 0
 #else
 #define GLTFIO_USE_FILESYSTEM 1
 #endif
+#endif
 
 namespace utils {
     class NameComponentManager;

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);
+    // Anisotropic Normal Bias for Shadow Mapping
-    p += n * (sinTheta * b);
+    // --------------------------------------------------------------------------------------
+    // 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
@@ -114,6 +115,24 @@ Doing the above has multiple effects:
 - If the PR is merged, then there is another workflow that will merge `my-pr-branch-golden`
   to the `main` branch of the golden repo.
 
+### Automated update via commit message
+
+Alternatively, if you are confident in your changes and want the CI to handle the update
+for you, you can use the following tag in your commit message:
+
+- In the commit message of your working branch on `filament`, add the following line:
+  ```
+  RDIFF_ACCEPT_NEW_GOLDENS
+  ```
+
+This has the following effects:
+- The presubmit test `test-renderdiff` will be bypassed (it will not perform rendering or
+  comparison).
+- When the PR is merged, the postsubmit CI will automatically:
+    1. Build Filament and generate the new images.
+    2. Upload them to a temporary branch in `filament-assets`.
+    3. Merge that branch into `main`.
+
 ## Viewing test results
 
 We provide a viewer for looking at the result of a test run. The viewer is a webapp that can

test/renderdiff/src/commit_msg.py

@@ -18,6 +18,7 @@ import re
 from utils import execute, ArgParseImpl
 
 RDIFF_UPDATE_GOLDEN_STR = 'RDIFF_BRANCH'
+RDIFF_ACCEPT_NEW_GOLDENS_STR = 'RDIFF_ACCEPT_NEW_GOLDENS'
 
 def _parse_commit(commit_str):
   lines = commit_str.split('\n')
@@ -42,9 +43,11 @@ def _parse_commit(commit_str):
 
 if __name__ == "__main__":
   RE_STR = rf"{RDIFF_UPDATE_GOLDEN_STR}(?:\s)?\=(?:\s)?([a-zA-Z0-9\s\-\/]+)"
+  RE_ACCEPT = rf"^{RDIFF_ACCEPT_NEW_GOLDENS_STR}$"
 
   parser = ArgParseImpl()
   parser.add_argument('--file', help='A file containing the commit message')
+  parser.add_argument('--mode', choices=['branch', 'accept_new_goldens'], default='branch', help='Mode of operation')
   args, _ = parser.parse_known_args(sys.argv[1:])
 
   if not args.file:
@@ -53,14 +56,21 @@ if __name__ == "__main__":
     with open(args.file, 'r') as f:
       msg = f.read()
 
-  to_update = []
   commit, title, description = _parse_commit(msg)
+
+  if args.mode == 'accept_new_goldens':
+    for line in description:
+      if re.match(RE_ACCEPT, line):
+        sys.exit(0)
+    sys.exit(1)
+
+  # args.mode == 'branch'
   for line in description:
     m = re.match(RE_STR, line)
     if not m:
       continue
     print(m.group(1))
-    exit(0)
+    sys.exit(0)
 
   # Always default to the main branch
   print('main')

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 tempfile.NamedTemporaryFile('w', delete=False) as f:
-      with open(COMMIT_FILE, 'w') 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 tempfile.NamedTemporaryFile('w', delete=False) as f:
-
-    with open(TMP_GOLDEN_COMMIT_FILE, 'w') 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)
+  if result.returncode != 0:
+    raise RuntimeError(f"diffimg failed with return code {result.returncode}:\n{result.stderr}")
+
   output = result.stdout.strip()
   if not output:
-      return False
+    raise RuntimeError("diffimg produced no output")
 
   try:
     res_json = json.loads(output)
     return res_json.get('passed', False)
-    except json.JSONDecodeError:
+  except json.JSONDecodeError as e:
-      return False
+    raise RuntimeError(f"Failed to parse diffimg output: {e}\nOutput: {output}")
-  except Exception:
-    return False
 
 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_update += update_files
-    ret_delete += delete
+    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

web/filament-js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "filament",
-  "version": "1.69.3",
+  "version": "1.69.4",
   "description": "Real-time physically based rendering engine",
   "main": "filament.js",
   "module": "filament.js",