Dawn change to enable writeBuffer for size != multiple of 4

Co-authored-by: Andy Hovingh <6198728+AndyHovingh@users.noreply.github.com>

.github/actions/dep-versions/action.yml

@@ -3,5 +3,5 @@ runs:
   using: "composite"
   steps:
     - name: Set up dependency versions
-      shell: bash
+      shell: bash    
       run: cat ./build/common/versions >> $GITHUB_ENV

.github/actions/web-prereq/action.yml

@@ -1,16 +0,0 @@
-name: 'Web Preqrequisites'
-runs:
-  using: "composite"
-  steps:
-    - uses: ./.github/actions/dep-versions
-    - name: Cache EMSDK
-      id: emsdk-cache
-      uses: actions/cache@v4 # Use a specific version
-      with:
-        path: emsdk
-        key: ${{ runner.os }}-emsdk-${{ env.GITHUB_EMSDK_VERSION }}
-    - name: Install Web Prerequisites
-      shell: bash
-      run: |
-        bash ./build/common/get-emscripten.sh
-        echo "EMSDK=$PWD/emsdk" >> $GITHUB_ENV

.github/workflows/presubmit.yml

@@ -96,7 +96,6 @@ jobs:
         with:
           fetch-depth: 0
       - uses: ./.github/actions/linux-prereq
-      - uses: ./.github/actions/web-prereq
       - name: Run build script
         run: |
           cd build/web && printf "y" | ./build.sh presubmit
@@ -124,15 +123,13 @@ jobs:
       - uses: ./.github/actions/mac-prereq
       - name: Cache Mesa and deps
         id: mesa-cache
-        uses: actions/cache@v4
+        uses: actions/cache@v4 # Use a specific version
         with:
           path: mesa
           key: ${{ runner.os }}-mesa-deps-2-${{ vars.MESA_VERSION }}
-      - name: Prerequisites
-        id: prereqs
-        run: |
-          bash test/utils/get_mesa.sh
-          pip install tifffile numpy
+      - name: Get Mesa
+        id: mesa-prereq
+        run: bash test/utils/get_mesa.sh
       - name: Run Test
         run: bash test/renderdiff/test.sh
       - uses: actions/upload-artifact@v4
@@ -153,7 +150,7 @@ jobs:
       - name: Run test
         run: ./out/cmake-debug/libs/filamat/test_filamat --gtest_filter=MaterialCompiler.Wgsl*
 
-  code-correctness:
+  code-correcteness:
     name: code-correctness
     runs-on: 'macos-14-xlarge'
     steps:

.github/workflows/release.yml

@@ -65,9 +65,13 @@ jobs:
 
   build-mac:
     name: build-mac
-    runs-on: macos-14-xlarge
+    runs-on: ${{ matrix.os }}
     if: github.event_name == 'release' || github.event.inputs.platform == 'desktop'
 
+    strategy:
+      matrix:
+        os: [macos-14-xlarge, ubuntu-22.04-32core]
+
     steps:
       - name: Decide Git ref
         id: git_ref
@@ -114,7 +118,6 @@ jobs:
         with:
           ref: ${{ steps.git_ref.outputs.ref }}
       - uses: ./.github/actions/linux-prereq
-      - uses: ./.github/actions/web-prereq
       - name: Run build script
         env:
           TAG: ${{ steps.git_ref.outputs.tag }}

.github/workflows/web-continuous.yml

@@ -17,7 +17,6 @@ jobs:
         with:
           fetch-depth: 0
       - uses: ./.github/actions/linux-prereq
-      - uses: ./.github/actions/web-prereq
       - name: Run build script
         run: |
           cd build/web && printf "y" | ./build.sh continuous

BUILDING.md

@@ -363,8 +363,6 @@ python ./emsdk.py activate latest
 source ./emsdk_env.sh
 ```
 
-Alternatively, you can try running the script `build/common/get-emscripten.sh`.
-
 After this you can invoke the [easy build](#easy-build) script as follows:
 
 ```shell

README.md

@@ -31,7 +31,7 @@ repositories {
 }
 
 dependencies {
-    implementation 'com.google.android.filament:filament-android:1.59.5'
+    implementation 'com.google.android.filament:filament-android:1.59.4'
 }
 ```
 
@@ -51,7 +51,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.59.5'
+pod 'Filament', '~> 1.59.4'
 ```
 
 ## Documentation

RELEASE_NOTES.md

@@ -7,11 +7,6 @@ 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.60.0
-
-- materials: remove dependence on per-view descset layout from filamat. [⚠️ **New Material Version**]
-- `ColorGrading::Builder::toneMapper` now takes a `shared_ptr<ToneMapper>`
-
 ## v1.59.5
 
 

android/filament-android/src/main/cpp/ColorGrading.cpp

@@ -18,7 +18,6 @@
 
 #include <filament/ColorGrading.h>
 #include <filament/ToneMapper.h>
-#include <memory>
 
 #include <math/vec3.h>
 #include <math/vec4.h>
@@ -68,9 +67,8 @@ extern "C" JNIEXPORT void JNICALL
 Java_com_google_android_filament_ColorGrading_nBuilderToneMapper(JNIEnv*, jclass,
         jlong nativeBuilder, jlong toneMapper_) {
     ColorGrading::Builder* builder = (ColorGrading::Builder*) nativeBuilder;
-    ToneMapper* toneMapper = reinterpret_cast<ToneMapper*>(toneMapper_);
-    std::shared_ptr<ToneMapper> toneMapperCopy(toneMapper->clone());
-    builder->toneMapper(toneMapperCopy);
+    const ToneMapper* toneMapper = (const ToneMapper*) toneMapper_;
+    builder->toneMapper(toneMapper);
 }
 
 #pragma clang diagnostic push

android/filament-android/src/main/java/com/google/android/filament/ColorGrading.java

@@ -214,9 +214,9 @@ public class ColorGrading {
          *
          * The default tone mapping operator is {@link ToneMapper.ACESLegacy}.
          *
-         * The copy of the specified tone mapper is set to this builder. It is safe to delete
-         * the original tone mapper object while the copied one is held by the built ColorGrading
-         * object.
+         * The specified tone mapper must have a lifecycle that exceeds the lifetime of
+         * this builder. Since the build(Engine&) method is synchronous, it is safe to
+         * delete the tone mapper object after that finishes executing.
          *
          * @param toneMapper The tone mapping operator to apply to the HDR color buffer
          *

android/gradle.properties

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

build.sh

@@ -151,7 +151,7 @@ function print_fgviewer_help {
 }
 
 # Unless explicitly specified, NDK version will be selected as highest available version within same major release chain
-FILAMENT_NDK_VERSION=${FILAMENT_NDK_VERSION:-$(cat `dirname $0`/build/common/versions | grep GITHUB_NDK_VERSION | cut -f 1 -d ".")}
+FILAMENT_NDK_VERSION=${FILAMENT_NDK_VERSION:-$(cat `dirname $0`/build/android/ndk.version | cut -f 1 -d ".")}
 
 # Requirements
 CMAKE_MAJOR=3

build/android/build.sh

@@ -1,6 +1,28 @@
 #!/bin/bash
 
-source `dirname $0`/../common/ci-check.sh
+# Usage: the first argument selects the build type:
+# - release, to build release only
+# - debug, to build debug only
+# - continuous, to build release and debug
+# - presubmit, for presubmit builds
+#
+# The default is release
+
+echo "This script is intended to run in a CI environment and may modify your current environment."
+echo "Please refer to BUILDING.md for more information."
+
+read -r -p "Do you wish to proceed (y/n)? " choice
+case "${choice}" in
+    y|Y)
+              echo "Build will proceed..."
+              ;;
+    n|N)
+          exit 0
+          ;;
+          *)
+        exit 0
+        ;;
+esac
 
 set -e
 set -x
@@ -8,6 +30,11 @@ set -x
 UNAME=`echo $(uname)`
 LC_UNAME=`echo $UNAME | tr '[:upper:]' '[:lower:]'`
 
+# build-common.sh will generate the following variables:
+#     $GENERATE_ARCHIVES
+#     $BUILD_DEBUG
+#     $BUILD_RELEASE
+source `dirname $0`/../common/ci-common.sh
 source `dirname $0`/../common/build-common.sh
 
 if [[ "$GITHUB_WORKFLOW" ]]; then

build/common/build-common.sh

@@ -1,20 +1,5 @@
 #!/bin/bash
 
-# build-common.sh will generate the following variables:
-#     $GENERATE_ARCHIVES
-#     $BUILD_DEBUG
-#     $BUILD_RELEASE
-
-# Typically a build script (build.sh) would source this script. For example,
-#   source `dirname $0`/../common/build-common.sh
-
-# Usage: the first argument selects the build type:
-# - release, to build release only
-# - debug, to build debug only
-# - continuous, to build release and debug
-# - presubmit, for presubmit builds
-#
-# The default is release
 if [[ ! "$TARGET" ]]; then
     if [[ "$1" ]]; then
         TARGET=$1

build/common/ci-check.sh

@@ -1,19 +0,0 @@
-echo "This script is intended to run in a CI environment and may modify your current environment."
-echo "Please refer to BUILDING.md for more information."
-
-read -r -p "Do you wish to proceed (y/n)? " choice
-case "${choice}" in
-    y|Y)
-	      echo "Build will proceed..."
-	      ;;
-    n|N)
-    	  exit 0
-    	  ;;
-	  *)
-        exit 0
-        ;;
-esac
-
-if [[ "$GITHUB_WORKFLOW" ]]; then
-    echo "Running workflow $GITHUB_WORKFLOW (event: $GITHUB_EVENT_NAME, action: $GITHUB_ACTION)"
-fi

build/common/ci-common.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+if [[ "$GITHUB_WORKFLOW" ]]; then
+    echo "Running workflow $GITHUB_WORKFLOW (event: $GITHUB_EVENT_NAME, action: $GITHUB_ACTION)"
+fi

build/common/get-emscripten.sh

@@ -1,22 +0,0 @@
-#!/bin/bash
-
-if [ -d "./emsdk" ]; then
-    echo "emsdk folder found. Assume emsdk has been installed."
-    cd emsdk
-    ./emsdk activate latest
-    source ./emsdk_env.sh
-    export EMSDK="$PWD"
-    cd ..
-    exit 0
-fi
-
-# Install emscripten.
-EMSDK_VERSION=${GITHUB_EMSDK_VERSION-3.1.60}
-curl -L https://github.com/emscripten-core/emsdk/archive/refs/tags/${EMSDK_VERSION}.zip > emsdk.zip
-unzip emsdk.zip ; mv emsdk-* emsdk ; cd emsdk
-./emsdk install latest
-./emsdk activate latest
-source ./emsdk_env.sh
-
-export EMSDK="$PWD"
-cd ..

build/common/versions

@@ -3,5 +3,4 @@ GITHUB_CMAKE_VERSION=3.19.5
 GITHUB_NINJA_VERSION=1.10.2
 GITHUB_MESA_VERSION=24.2.1
 GITHUB_LLVM_VERSION=16
-GITHUB_NDK_VERSION=27.0.11718014
-GITHUB_EMSDK_VERSION=3.1.60
+GITHUB_NDK_VERSION=27.0.11718014

build/ios/build.sh

@@ -1,11 +1,35 @@
 #!/bin/bash
 
-source `dirname $0`/../common/ci-check.sh
+# Usage: the first argument selects the build type:
+# - release, to build release only
+# - debug, to build debug only
+# - continuous, to build release and debug
+# - presubmit, for presubmit builds
+#
+# The default is release
+
+echo "This script is intended to run in a CI environment and may modify your current environment."
+echo "Please refer to BUILDING.md for more information."
+
+read -r -p "Do you wish to proceed (y/n)? " choice
+case "${choice}" in
+    y|Y)
+	      echo "Build will proceed..."
+	      ;;
+    n|N)
+    	  exit 0
+    	  ;;
+	  *)
+        exit 0
+        ;;
+esac
 
 set -e
 set -x
 
+source `dirname $0`/../common/ci-common.sh
 source `dirname $0`/../common/build-common.sh
+
 pushd `dirname $0`/../.. > /dev/null
 
 # If we're generating an archive for release or continuous builds, then we'll also build for the

build/linux/build.sh

@@ -1,11 +1,38 @@
 #!/bin/bash
 
-source `dirname $0`/../common/ci-check.sh
+# Usage: the first argument selects the build type:
+# - release, to build release only
+# - debug, to build debug only
+# - continuous, to build release and debug
+# - presubmit, for presubmit builds
+#
+# The default is release
+
+echo "This script is intended to run in a CI environment and may modify your current environment."
+echo "Please refer to BUILDING.md for more information."
+
+read -r -p "Do you wish to proceed (y/n)? " choice
+case "${choice}" in
+    y|Y)
+	      echo "Build will proceed..."
+	      ;;
+    n|N)
+    	  exit 0
+    	  ;;
+	  *)
+        exit 0
+        ;;
+esac
 
 set -e
 set -x
 
+# build-common.sh will generate the following variables:
+#     $GENERATE_ARCHIVES
+#     $BUILD_DEBUG
+#     $BUILD_RELEASE
+source `dirname $0`/../common/ci-common.sh
 source `dirname $0`/../common/build-common.sh
-pushd `dirname $0`/../.. > /dev/null
 
+pushd `dirname $0`/../.. > /dev/null
 ./build.sh -c $RUN_TESTS $GENERATE_ARCHIVES $BUILD_DEBUG $BUILD_RELEASE

build/mac/build.sh

@@ -1,11 +1,34 @@
 #!/bin/bash
 
-source `dirname $0`/../common/ci-check.sh
+# Usage: the first argument selects the build type:
+# - release, to build release only
+# - debug, to build debug only
+# - continuous, to build release and debug
+# - presubmit, for presubmit builds
+#
+# The default is release
+
+echo "This script is intended to run in a CI environment and may modify your current environment."
+echo "Please refer to BUILDING.md for more information."
+
+read -r -p "Do you wish to proceed (y/n)? " choice
+case "${choice}" in
+    y|Y)
+	      echo "Build will proceed..."
+	      ;;
+    n|N)
+    	  exit 0
+    	  ;;
+	  *)
+        exit 0
+        ;;
+esac
 
 set -e
 set -x
 
+source `dirname $0`/../common/ci-common.sh
 source `dirname $0`/../common/build-common.sh
-pushd `dirname $0`/../.. > /dev/null
 
+pushd `dirname $0`/../.. > /dev/null
 ./build.sh -c $RUN_TESTS $GENERATE_ARCHIVES $BUILD_DEBUG $BUILD_RELEASE

build/web/build.sh

@@ -1,10 +1,34 @@
 #!/bin/bash
 
-source `dirname $0`/../common/ci-check.sh
+# Usage: the first argument selects the build type:
+# - release, to build release only
+# - debug, to build debug only
+# - continuous, to build release and debug
+# - presubmit, for presubmit builds
+#
+# The default is release
+
+echo "This script is intended to run in a CI environment and may modify your current environment."
+echo "Please refer to BUILDING.md for more information."
+
+read -r -p "Do you wish to proceed (y/n)? " choice
+case "${choice}" in
+    y|Y)
+	      echo "Build will proceed..."
+	      ;;
+    n|N)
+    	  exit 0
+    	  ;;
+	  *)
+        exit 0
+        ;;
+esac
 
 set -e
 set -x
 
+source `dirname $0`/../common/ci-common.sh
+source `dirname $0`/ci-common.sh
 source `dirname $0`/../common/build-common.sh
 pushd `dirname $0`/../.. > /dev/null
 

build/web/ci-common.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+
+# Install emscripten.
+curl -L https://github.com/emscripten-core/emsdk/archive/refs/tags/3.1.60.zip > emsdk.zip
+unzip emsdk.zip ; mv emsdk-* emsdk ; cd emsdk
+./emsdk install latest
+./emsdk activate latest
+source ./emsdk_env.sh
+
+export EMSDK="$PWD"
+cd ..

docs_src/src/SUMMARY.md

@@ -18,7 +18,6 @@
     - [Metal](./notes/metal_debugging.md)
     - [Vulkan](./notes/vulkan_debugging.md)
     - [SPIR-V](./notes/spirv_debugging.md)
-    - [Running with ASAN and UBSAN](./notes/asan_ubsan.md)
   - [Libraries](./notes/libs.md)
     - [bluegl](./dup/bluegl.md)
     - [bluevk](./dup/bluevk.md)

docs_src/src/notes/asan_ubsan.md

@@ -1,41 +0,0 @@
-# Running with ASAN/UBSAN
-
-## Enabling
-
-When building though build.sh, pass the `-b` flag. This sets the cmake variable
-`FILAMENT_ENABLE_ASAN_UBSAN=ON` which eventually passes `"-fsanitize=address -fsanitize=undefined"`
-to all compile and link operations.
-
-If building through CMake directly, or an IDE like CLion that doesn't use build.sh, instead pass
-`-DFILAMENT_ENABLE_ASAN_UBSAN=ON` to cmake in order to get the same result.
-
-## Getting memory leak detection on Mac
-
-Memory leak detection isn't enabled by default on MacOS. There are two issues to address, first is
-using a version of clang that supports memory leak detection and second is enabling it at runtime.
-
-The version of clang distributed by Apple (with a version like "Apple clang version 16.0.0") doesn't
-currently support leak detection at all. Instead you will need to get or build a different LLVM,
-such as the one distributed through homebrew and get CMake to use that instead.
-
-Then during runtime you'll need to have the environment variable `ASAN_OPTIONS` include the option
-`detect_leaks=1`. Multiple `ASAN_OPTIONS` values are concatenated with `:`.
-
-## Getting memory leak output in CLion
-
-### Setting variables
-
-Under `Settings | Build, Execution, Deployment | Dynamic Analysis Tools | Sanitizers` there is an
-ASAN Settings field that overrides whatever other `ASAN_OPTIONS` you might set elsewhere, so you
-must use that instead of setting it through your Run/Debug Configuration.
-
-To pass `-DFILAMENT_ENABLE_ASAN_UBSAN=ON` to CMake you'll want to create a new CMake Profile and
-pass it as a CMake argument.
-
-### Avoiding losing output
-
-CMake will consume ASAN output and display it through a separate "Sanitizers" tab. Unfortunately
-certain leak detection errors that interrupt the executable seem to not show up in this tab, but are
-still removed from the user-visible console output. If this is happening and you need to see the
-unfiltered console output you'll need to go to `Settings | Build, Execution, Deployment | Dynamic
-Analysis Tools | Sanitizers` and uncheck "Use visual representation for Sanitizer's output".

filament/backend/CMakeLists.txt

@@ -5,6 +5,18 @@ set(TARGET backend)
 set(PUBLIC_HDR_DIR include)
 set(GENERATION_ROOT ${CMAKE_CURRENT_BINARY_DIR})
 
+# ==================================================================================================
+# Compilation options
+# ==================================================================================================
+#
+set(BACKEND_SANITIZATION "" CACHE STRING "Sanitization option")
+set_property(CACHE BACKEND_SANITIZATION PROPERTY STRINGS ";ASAN")
+
+set(BACKEND_SANITIZERS)
+if (BACKEND_SANITIZATION STREQUAL "ASAN")
+    set(BACKEND_SANITIZERS -fsanitize=address)
+endif()
+
 # ==================================================================================================
 # Sources and headers
 # ==================================================================================================
@@ -472,6 +484,7 @@ target_compile_options(${TARGET} PRIVATE
         ${OSMESA_COMPILE_FLAGS}
         $<$<CONFIG:Release>:${OPTIMIZATION_FLAGS}>
         $<$<AND:$<PLATFORM_ID:Darwin>,$<CONFIG:Release>>:${DARWIN_OPTIMIZATION_FLAGS}>
+        ${BACKEND_SANITIZERS}
 )
 
 if (FILAMENT_SUPPORTS_METAL)
@@ -482,6 +495,8 @@ if (FILAMENT_SUPPORTS_WEBGPU)
     target_compile_definitions(${TARGET} PRIVATE $<$<BOOL:${FILAMENT_WEBGPU_IMMEDIATE_ERROR_HANDLING}>:FILAMENT_WEBGPU_IMMEDIATE_ERROR_HANDLING>)
 endif()
 
+target_link_options(${TARGET} PRIVATE ${BACKEND_SANITIZERS})
+
 target_link_libraries(${TARGET} PRIVATE
         ${OSMESA_LINKER_FLAGS}
         $<$<AND:$<PLATFORM_ID:Linux>,$<CONFIG:Release>>:${LINUX_LINKER_OPTIMIZATION_FLAGS}>
@@ -551,6 +566,8 @@ if (APPLE AND NOT IOS)
          test/test_RenderExternalImage.cpp)
     add_library(backend_test STATIC ${BACKEND_TEST_SRC})
     target_link_libraries(backend_test PUBLIC ${BACKEND_TEST_LIBS})
+    target_compile_options(backend_test PRIVATE ${BACKEND_SANITIZERS})
+    target_link_options(backend_test PRIVATE ${BACKEND_SANITIZERS})
 
     set(BACKEND_TEST_DEPS
             OSDependent
@@ -589,6 +606,7 @@ if (APPLE AND NOT IOS)
        # linker from removing "unused" symbols.
        target_link_libraries(backend_test_mac PRIVATE -force_load backend_test)
        set_target_properties(backend_test_mac PROPERTIES FOLDER Tests)
+       target_link_options(backend_test_mac PRIVATE ${BACKEND_SANITIZERS})
 
        # This is needed after XCode 15.3
        set_target_properties(backend_test_mac PROPERTIES BUILD_WITH_INSTALL_RPATH TRUE)
@@ -598,6 +616,8 @@ endif()
 
 if (LINUX)
     add_executable(backend_test_linux test/linux_runner.cpp ${BACKEND_TEST_SRC})
+    target_compile_options(backend_test_linux PRIVATE ${BACKEND_SANITIZERS})
+    target_link_options(backend_test_linux PRIVATE ${BACKEND_SANITIZERS})
     target_link_libraries(backend_test_linux PRIVATE ${BACKEND_TEST_LIBS})
     set_target_properties(backend_test_linux PROPERTIES FOLDER Tests)
 endif()

filament/backend/include/backend/DriverEnums.h

@@ -30,7 +30,6 @@
 #include <utils/compiler.h>
 #include <utils/debug.h>
 #include <utils/ostream.h>
-#include <utils/StaticString.h>
 
 #include <math/vec4.h>
 
@@ -40,6 +39,7 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <utils/StaticString.h>
 
 /**
  * Types and enums used by filament's driver.

filament/backend/include/backend/Program.h

@@ -44,8 +44,8 @@ public:
 
     struct Descriptor {
         utils::CString name;
-        DescriptorType type;
-        descriptor_binding_t binding;
+        backend::DescriptorType type;
+        backend::descriptor_binding_t binding;
     };
 
     struct SpecializationConstant {

filament/backend/src/CommandStream.cpp

@@ -84,7 +84,7 @@ void CommandStream::execute(void* buffer) {
 
     Profiler profiler;
 
-    if (SYSTRACE_TAG) {
+    if constexpr (SYSTRACE_TAG) {
         if (UTILS_UNLIKELY(mUsePerformanceCounter)) {
             // we want to remove all this when tracing is completely disabled
             profiler.resetEvents(Profiler::EV_CPU_CYCLES  | Profiler::EV_BPU_MISSES);
@@ -100,7 +100,7 @@ void CommandStream::execute(void* buffer) {
         }
     });
 
-    if (SYSTRACE_TAG) {
+    if constexpr (SYSTRACE_TAG) {
         if (UTILS_UNLIKELY(mUsePerformanceCounter)) {
             // we want to remove all this when tracing is completely disabled
             profiler.stop();

filament/backend/src/metal/MetalEnums.h

@@ -185,7 +185,6 @@ inline MTLPixelFormat getMetalFormat(PixelDataFormat format, PixelDataType type)
     CONVERT(RGBA_INTEGER, UINT, RGBA32Uint);
     CONVERT(RGBA_INTEGER, INT, RGBA32Sint);
     CONVERT(RGBA, FLOAT, RGBA32Float);
-    CONVERT(DEPTH_COMPONENT, FLOAT, Depth32Float);
     #undef CONVERT
 
     return MTLPixelFormatInvalid;

filament/backend/src/vulkan/VulkanConstants.h

@@ -152,12 +152,12 @@ static_assert(FVK_ENABLED(FVK_DEBUG_VALIDATION));
 #elif FVK_ENABLED(FVK_DEBUG_SYSTRACE)
 
     #include <utils/Systrace.h>
-    
+
     #define FVK_SYSTRACE_CONTEXT()      SYSTRACE_CONTEXT()
     #define FVK_SYSTRACE_START(marker)  SYSTRACE_NAME_BEGIN(marker)
     #define FVK_SYSTRACE_END()          SYSTRACE_NAME_END()
-    #define FVK_SYSTRACE_SCOPE()        SYSTRACE_NAME(__func__)
-    #define FVK_PROFILE_MARKER(marker)  FVK_SYSTRACE_SCOPE()
+    #define FVK_SYSTRACE_SCOPE()        SYSTRACE_CALL()
+    #define FVK_PROFILE_MARKER(marker)  SYSTRACE_CALL()
 
 #else
     #define FVK_SYSTRACE_CONTEXT()

filament/backend/src/vulkan/VulkanContext.h

@@ -142,10 +142,6 @@ public:
         return mPortabilitySubsetFeatures.imageView2DOn3DImage == VK_TRUE;
     }
 
-    inline bool isUnifiedMemoryArchitecture() const noexcept {
-        return mIsUnifiedMemoryArchitecture;
-    }
-
 private:
     VkPhysicalDeviceMemoryProperties mMemoryProperties = {};
     VkPhysicalDeviceProperties2 mPhysicalDeviceProperties = {
@@ -168,7 +164,6 @@ private:
     bool mDebugUtilsSupported = false;
     bool mLazilyAllocatedMemorySupported = false;
     bool mProtectedMemorySupported = false;
-    bool mIsUnifiedMemoryArchitecture = false;
 
     fvkutils::VkFormatList mDepthStencilFormats;
     fvkutils::VkFormatList mBlittableDepthStencilFormats;

filament/backend/src/vulkan/platform/VulkanPlatform.cpp

@@ -631,21 +631,6 @@ fvkutils::VkFormatList findBlittableDepthStencilFormats(VkPhysicalDevice device)
     return ret;
 }
 
-/**
- *  Check if the GPU has a unified memory architecture.
- */
-bool hasUnifiedMemoryArchitecture(VkPhysicalDeviceMemoryProperties memoryProperties) noexcept {
-    // Try to identify if the platform is running on a Unified Memory Architecture by inspecting the
-    // memory heap flags, if they are all VK_MEMORY_HEAP_DEVICE_LOCAL_BIT it's UMA, otherwise not
-    // enough information to make a decision, so default to false.
-    for (uint32_t i = 0; i < memoryProperties.memoryHeapCount; ++i) {
-        if ((memoryProperties.memoryHeaps[i].flags & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) == 0) {
-            return false;
-        }
-    }
-    return true;
-}
-
 }// anonymous namespace
 
 using SwapChainPtr = VulkanPlatform::SwapChainPtr;
@@ -879,8 +864,6 @@ Driver* VulkanPlatform::createDriver(void* sharedContext,
         }
     }
 
-    context.mIsUnifiedMemoryArchitecture = hasUnifiedMemoryArchitecture(context.mMemoryProperties);
-
 #ifdef NDEBUG
     // If we are in release build, we should not have turned on debug extensions
     FILAMENT_CHECK_POSTCONDITION(!context.mDebugUtilsSupported && !context.mDebugMarkersSupported)

filament/backend/src/webgpu/WebGPUDriver.cpp

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include <utils/Hash.h>
+
 #include "webgpu/WebGPUDriver.h"
 
 #include "WebGPUPipelineCreation.h"
@@ -916,14 +916,6 @@ void WebGPUDriver::blit(
 }
 
 void WebGPUDriver::bindPipeline(PipelineState const& pipelineState) {
-    // TODO Investigate implications of this hash more closely. Vulkan has a whole class
-    // VulkanPipelineCache to handle this, may be missing nuance
-    static auto pipleineStateHasher = utils::hash::MurmurHashFn<filament::backend::PipelineState>();
-    auto hash = pipleineStateHasher(pipelineState);
-    if(mPipelineMap.find(hash) != mPipelineMap.end()){
-        mRenderPassEncoder.SetPipeline(mPipelineMap[hash]);
-        return;
-    }
     const auto* program = handleCast<WGPUProgram>(pipelineState.program);
     assert_invariant(program);
     assert_invariant(program->computeShaderModule == nullptr &&
@@ -960,7 +952,7 @@ void WebGPUDriver::bindPipeline(PipelineState const& pipelineState) {
             *vertexBufferInfo, layout, pipelineState.rasterState, pipelineState.stencilState,
             pipelineState.polygonOffset, pipelineState.primitiveType, mSwapChain->getColorFormat(),
             mSwapChain->getDepthFormat());
-    mPipelineMap[hash] = pipeline;
+    // TODO: uncomment once we have a valid pipeline to set
     mRenderPassEncoder.SetPipeline(pipeline);
 }
 

filament/backend/src/webgpu/WebGPUDriver.h

@@ -93,8 +93,6 @@ private:
     wgpu::RenderPassEncoder mRenderPassEncoder = nullptr;
     wgpu::CommandBuffer mCommandBuffer = nullptr;
     WGPURenderTarget* mDefaultRenderTarget = nullptr;
-
-    tsl::robin_map<uint32_t, wgpu::RenderPipeline> mPipelineMap;
     /*
      * Driver interface
      */

filament/backend/src/webgpu/WebGPUHandles.cpp

@@ -514,7 +514,7 @@ WGPUTexture::WGPUTexture(SamplerType target, uint8_t levels, TextureFormat forma
                     "the spec. See https://www.w3.org/TR/webgpu/#texture-creation or "
                     "https://gpuweb.github.io/gpuweb/#multisample-state");
     // First, the texture aspect, starting with the defaults/basic configuration
-    mUsage = fToWGPUTextureUsage(usage, device.HasFeature(wgpu::FeatureName::TransientAttachments));
+    mUsage = fToWGPUTextureUsage(usage);
     mFormat = fToWGPUTextureFormat(format);
     wgpu::TextureDescriptor textureDescriptor{
         .label = getUserTextureLabel(target),
@@ -566,8 +566,7 @@ WGPUTexture::WGPUTexture(WGPUTexture* src, uint8_t baseLevel, uint8_t levelCount
     mTexView = makeTextureView(baseLevel, levelCount, target);
 }
 
-wgpu::TextureUsage WGPUTexture::fToWGPUTextureUsage(const TextureUsage& fUsage,
-        const bool supportsTransientAttachment) {
+wgpu::TextureUsage WGPUTexture::fToWGPUTextureUsage(const TextureUsage& fUsage) {
     wgpu::TextureUsage retUsage = wgpu::TextureUsage::None;
 
     // Basing this mapping off of VulkanTexture.cpp's getUsage func and suggestions from Gemini
@@ -593,7 +592,6 @@ wgpu::TextureUsage WGPUTexture::fToWGPUTextureUsage(const TextureUsage& fUsage,
     // This is from Vulkan logic- if there are any issues try disabling this first, allows perf
     // benefit though
     const bool useTransientAttachment =
-            supportsTransientAttachment &&
             // Usage consists of attachment flags only.
             none(fUsage & ~TextureUsage::ALL_ATTACHMENTS) &&
             // Usage contains at least one attachment flag.

filament/backend/src/webgpu/WebGPUHandles.h

@@ -194,8 +194,7 @@ private:
     wgpu::TextureFormat mFormat = wgpu::TextureFormat::Undefined;
     uint32_t mArrayLayerCount = 1;
     wgpu::TextureView mTexView = nullptr;
-    wgpu::TextureUsage fToWGPUTextureUsage(const filament::backend::TextureUsage& fUsage,
-            const bool supportsTransientAttachment);
+    wgpu::TextureUsage fToWGPUTextureUsage(const filament::backend::TextureUsage& fUsage);
 };
 
 struct WGPURenderPrimitive : public HwRenderPrimitive {

filament/backend/src/webgpu/platform/WebGPUPlatform.cpp

@@ -100,11 +100,9 @@ wgpu::Adapter WebGPUPlatform::requestAdapter(wgpu::Surface const& surface) {
 wgpu::Device WebGPUPlatform::requestDevice(wgpu::Adapter const& adapter) {
     // TODO consider passing limits
     constexpr std::array optionalFeatures = { wgpu::FeatureName::DepthClipControl,
-        wgpu::FeatureName::Depth32FloatStencil8, wgpu::FeatureName::CoreFeaturesAndLimits,
-        wgpu::FeatureName::TransientAttachments };
+        wgpu::FeatureName::Depth32FloatStencil8, wgpu::FeatureName::CoreFeaturesAndLimits };
 
-    // Currently no required features, but logic to check them is available
-    constexpr std::array<wgpu::FeatureName, 0> requiredFeatures = {};
+    constexpr std::array requiredFeatures = { wgpu::FeatureName::TransientAttachments };
 
     wgpu::SupportedFeatures supportedFeatures;
     adapter.GetFeatures(&supportedFeatures);

filament/backend/test/BackendTest.cpp

@@ -45,7 +45,6 @@ namespace test {
 Backend BackendTest::sBackend = Backend::NOOP;
 OperatingSystem BackendTest::sOperatingSystem = OperatingSystem::OTHER;
 bool BackendTest::sIsMobilePlatform = false;
-std::vector<std::string> BackendTest::sFailedImages;
 
 void BackendTest::init(Backend backend, OperatingSystem operatingSystem, bool isMobilePlatform) {
     sBackend = backend;
@@ -64,12 +63,11 @@ BackendTest::~BackendTest() {
     flushAndWait();
     mImageExpectations->evaluate();
     // Note: Don't terminate the driver for OpenGL, as it wipes away the context and removes the buffer from the screen.
-    if (sBackend != Backend::OPENGL) {
-        driver->terminate();
-        delete driver;
+    if (sBackend == Backend::OPENGL) {
+        return;
     }
-
-    recordFailedImages();
+    driver->terminate();
+    delete driver;
 }
 
 void BackendTest::initializeDriver() {
@@ -169,24 +167,8 @@ bool BackendTest::matchesEnvironment(OperatingSystem operatingSystem) {
     return sOperatingSystem == operatingSystem;
 }
 
-void BackendTest::markImageAsFailure(std::string failedImageName) {
-    sFailedImages.emplace_back(std::move(failedImageName));
-}
-
-void BackendTest::recordFailedImages() {
-    if (!sFailedImages.empty()) {
-        std::string failedImages;
-        for (auto& failedTestImageName: sFailedImages) {
-            if (failedImages.empty()) {
-                failedImages = failedTestImageName;
-            } else {
-                failedImages.append(",");
-                failedImages.append(failedTestImageName);
-            }
-        }
-        RecordProperty("FailedImages", failedImages);
-    }
-    sFailedImages.clear();
+bool BackendTest::matchesEnvironment(OperatingSystem operatingSystem, Backend backend) {
+    return matchesEnvironment(operatingSystem) && matchesEnvironment(backend);
 }
 
 class Environment : public ::testing::Environment {

filament/backend/test/BackendTest.h

@@ -38,9 +38,6 @@ public:
     static OperatingSystem sOperatingSystem;
     static bool sIsMobilePlatform;
 
-    // Takes the name of the image that wasn't correct, without the .png suffix
-    static void markImageAsFailure(std::string failedImageName);
-
 protected:
 
     BackendTest();
@@ -76,13 +73,8 @@ protected:
 
     static bool matchesEnvironment(Backend backend);
     static bool matchesEnvironment(OperatingSystem operatingSystem);
+    static bool matchesEnvironment(OperatingSystem operatingSystem, Backend backend);
 private:
-    // Adds all the images that failed an ImageExpectation to the XML metadata for the current tests
-    // case. Add --gtest_output=xml as a command line argument to generate a test_detail.xml file in
-    // the directory where the tests are run.
-    static void recordFailedImages();
-
-    static std::vector<std::string> sFailedImages;
 
     filament::backend::Driver* driver = nullptr;
     filament::backend::CommandBufferQueue commandBufferQueue;

filament/backend/test/ImageExpectations.cpp

@@ -21,7 +21,6 @@
 #include "utils/Hash.h"
 #include <fstream>
 
-#include "BackendTest.h"
 #include "backend/PixelBufferDescriptor.h"
 #include "private/backend/DriverApi.h"
 
@@ -33,8 +32,6 @@
 
 #endif
 
-namespace test {
-
 ScreenshotParams::ScreenshotParams(int width, int height, std::string fileName,
         uint32_t expectedHash, bool isSrgb)
     : mWidth(width),
@@ -83,10 +80,6 @@ std::string ScreenshotParams::expectedFilePath() const {
     return absl::StrFormat("%s/%s", expectedDirectoryPath(), expectedFileName());
 }
 
-const std::string ScreenshotParams::filePrefix() const {
-    return mFileName;
-}
-
 ImageExpectation::ImageExpectation(const char* fileName, int lineNumber,
         filament::backend::DriverApi& api, ScreenshotParams params,
         filament::backend::RenderTargetHandle renderTarget)
@@ -120,11 +113,7 @@ void ImageExpectation::compareImage() const {
 #ifndef FILAMENT_IOS
         LoadedPng loadedImage(mParams.expectedFilePath());
         uint32_t loadedImageHash = loadedImage.hash();
-        auto compareToImageMatcher = testing::Eq(loadedImageHash);
-        if (!testing::Matches(compareToImageMatcher)(actualHash)) {
-            BackendTest::markImageAsFailure(mParams.filePrefix());
-        }
-        EXPECT_THAT(actualHash, compareToImageMatcher) << mParams.expectedFileName();
+        EXPECT_THAT(actualHash, testing::Eq(loadedImageHash)) << mParams.expectedFileName();
 #endif
         // For builds that can't load PNGs (currently iOS only) use the expected hash.
         EXPECT_THAT(actualHash, testing::Eq(mParams.expectedHash())) << mParams.expectedFileName();
@@ -248,5 +237,3 @@ uint32_t LoadedPng::hash() const {
 const std::vector<unsigned char>& LoadedPng::bytes() const {
     return mBytes;
 }
-
-} // namespace test

filament/backend/test/ImageExpectations.h

@@ -35,8 +35,6 @@ do {                                                                \
         screenshotParams);                                          \
 } while (0)
 
-namespace test {
-
 /**
  * Stores user-provided configuration values for an image expectation
  */
@@ -56,7 +54,6 @@ public:
     static std::string expectedDirectoryPath();
     std::string expectedFileName() const;
     std::string expectedFilePath() const;
-    const std::string filePrefix() const;
 
 private:
     int mWidth;
@@ -156,6 +153,4 @@ private:
     std::vector<std::unique_ptr<ImageExpectation>> mExpectations;
 };
 
-} // namespace test
-
 #endif //TNT_IMAGE_EXPECTATIONS_H

filament/backend/test/Shader.cpp

@@ -57,7 +57,7 @@ Shader::Shader(DriverApi& api, Cleanup& cleanup, ShaderConfig config) : mCleanup
 
     if (!kLayouts.empty()) {
         mDescriptorSetLayout =
-                cleanup.add(api.createDescriptorSetLayout(DescriptorSetLayout{ .bindings = kLayouts }));
+                cleanup.add(api.createDescriptorSetLayout(DescriptorSetLayout{ kLayouts }));
     }
 }
 

filament/backend/test/move_actual_images_to_expected.py

@@ -1,143 +1,62 @@
-import os, shutil, argparse, typing, xml.etree.ElementTree, subprocess, platform
+import os, shutil, argparse, typing
+
+def match_sufffix(file_name: str, suffix: str, accepted_prefixes: typing.List[str]) -> str:
+  """
+  Check if the file name is one of the searched for ones with the given suffix and if so return it.
+  :param accepted_prefixes: If None accepts any prefix
+  :return: file_name with the suffix removed or "" if it doesn't match. This does mean a string that
+  is just the suffix is considered to not match as it will return the empty string.
+  """
+  if file_name.endswith(suffix):
+    prefix = file_name.removesuffix(suffix)
+    if accepted_prefixes is None or prefix in accepted_prefixes:
+      return prefix
+  return ""
 
 
-class TestResults(object):
-    ACTUAL_SUFFIX = '_actual.png'
-    EXPECTED_SUFFIX = '.png'
-
-    def __init__(self, results_directory: str, source_expected_directory: str):
-        self.results_directory = results_directory
-        self.actual_directory = os.path.join(self.results_directory, 'images', 'actual_images')
-        self.expected_directory = os.path.join(self.results_directory, 'images', 'expected_images')
-        self.source_expected_directory = source_expected_directory
-
-    def get_latest_failed_images(self) -> typing.List[str]:
-        failed_images = []
-        xml_tree = xml.etree.ElementTree.parse(
-            os.path.join(self.results_directory, 'test_detail.xml'))
-        testsuites = xml_tree.getroot()
-        for testsuite in testsuites.findall('testsuite'):
-            for testcase in testsuite.findall('testcase'):
-                for properties in testcase.findall('properties'):
-                    for property in properties.findall('property'):
-                        if property.get('name') == 'FailedImages':
-                            failed_images.extend(property.get('value').split(','))
-
-        return failed_images
-
-    def handle_failed_image(self, failed_image):
-        self.show_images(failed_image)
-        print(f'Update {failed_image}\'s expected image? y/n')
-        while True:
-            user_input = input()
-            if user_input == 'y':
-                self.move_actual_to_source([failed_image])
-                break
-            elif user_input == 'n':
-                break
-
-    def handle_all_failed_images(self):
-        for failed_image in self.get_latest_failed_images():
-            self.handle_failed_image(failed_image)
-
-    def show_images(self, failed_image):
-        # TODO: Test more on non-mac systems
-        open_command: str
-        os_name = platform.system().lower()
-        if 'windows' in os_name:
-            open_command = 'start'
-        elif 'osx' in os_name or 'darwin' in os_name:
-            open_command = 'open'
-        else:
-            open_command = 'xdg-open'
-
-        subprocess.run(
-            [open_command,
-             os.path.join(self.actual_directory, failed_image + TestResults.ACTUAL_SUFFIX)])
-        subprocess.run(
-            [open_command,
-             os.path.join(self.expected_directory, failed_image + TestResults.EXPECTED_SUFFIX)])
-
-    def move_actual_to_source(self, file_prefixes: typing.List[str]):
-        replace_file_names(path=self.actual_directory, removed=TestResults.ACTUAL_SUFFIX,
-                           replacement=TestResults.EXPECTED_SUFFIX,
-                           output_path=self.source_expected_directory, prefixes=file_prefixes)
-
-    def batch_move(self, prefixes: typing.Optional[typing.List[str]] = None):
-        replace_file_names(path=self.actual_directory, removed=TestResults.ACTUAL_SUFFIX,
-                           replacement=TestResults.EXPECTED_SUFFIX,
-                           output_path=self.source_expected_directory, prefixes=prefixes)
-
-
-def match_suffix(file_name: str, suffix: str, accepted_prefixes: typing.List[str]) -> str:
-    """
-    Check if the file name is one of the searched for ones with the given suffix and if so return
-    it.
-    :param accepted_prefixes: If None accepts any prefix
-    :return: file_name with the suffix removed or "" if it doesn't match. This does mean a string
-    that is just the suffix is considered to not match as it will return the empty string.
-    """
-    if file_name.endswith(suffix):
-        prefix = file_name.removesuffix(suffix)
-        if accepted_prefixes is None or prefix in accepted_prefixes:
-            return prefix
-    return ''
-
-
-def replace_file_names(path: str, removed: str, replacement: str = '', output_path: str = '',
-                       prefixes: typing.Optional[typing.List[str]] = None):
-    if not output_path:
-        output_path = path
-    for file_name in os.listdir(path=path):
-        prefix = match_suffix(file_name, removed, prefixes)
-        if prefix:
-            # Remove the prefix from the list so that prefixes is the list of intended but not yet
-            # found files.
-            if prefixes is not None:
-                prefixes.remove(prefix)
-            new_file_name = prefix + replacement
-            new_file_path = os.path.join(output_path, new_file_name)
-            old_file_path = os.path.join(path, file_name)
-            print(f'{old_file_path} to {new_file_path}')
-            shutil.copyfile(old_file_path, new_file_path)
-    if prefixes is not None:
-        for unfound_prefix in prefixes:
-            print(f'Failed to find {unfound_prefix}_actual.png')
-
+def replace_file_names(path: str, removed: str, replacement: str = "", output_path: str = "",
+                       prefixes: typing.List[str] = None):
+  if not output_path:
+    output_path = path
+  for file_name in os.listdir(path=path):
+    prefix = match_sufffix(file_name, removed, prefixes)
+    if prefix:
+      # Remove the prefix from the list so that prefixes is the list of intended but not yet found
+      # files.
+      if prefixes is not None:
+        prefixes.remove(prefix)
+      new_file_name = prefix + replacement
+      new_file_path = os.path.join(output_path, new_file_name)
+      old_file_path = os.path.join(path, file_name)
+      print(f'{old_file_path} to {new_file_path}')
+      shutil.move(old_file_path, new_file_path)
+  if prefixes is not None:
+    for unfound_prefix in prefixes:
+      print(f'Failed to find {unfound_prefix}_actual.png')
 
 if __name__ == "__main__":
-    parser = argparse.ArgumentParser(prog='Backend Test File Renamer',
-                                     description='Moves actual generated test images to the '
-                                                 'expected images directory, to update the test '
-                                                 'requirements. test_cases accepts multiple '
-                                                 'arguments that should be the name of the '
-                                                 'expected image file without the .png suffix. '
-                                                 'Also --all can be passed to copy all images.\n'
-                                                 'Remember to sync CMake after running this to '
-                                                 'move the new expected images to the binary '
-                                                 'directory.')
-    parser.add_argument('-r', '--results_path')
-    parser.add_argument('-s', '--source_expected_path', default="./expected_images")
-    # The mutually exclusive options for how to process the actual images
-    parser.add_argument('-b', '--batch', action='extend', nargs='*')
-    parser.add_argument('-a', '--all', action='store_true')
-    parser.add_argument('-t', '--tests', action='store_true')
-    parser.add_argument('-c', '--compare', action='extend', nargs='*')
+  parser = argparse.ArgumentParser(prog='Backend Test File Renamer',
+                                   description='Moves actual generated test images to the expected '
+                                               'images directory, to update the test requirements. '
+                                               'test_cases accepts multiple arguments that should '
+                                               'be the name of the expected image file without the '
+                                               '.png suffix. Also --all can be passed to copy all '
+                                               'images.\n'
+                                               'Remember to sync CMake after running this to move '
+                                               'the new expected images to the binary directory.')
+  parser.add_argument('-i', '--input_path')
+  parser.add_argument('-o', '--output_path', default="./expected_images")
+  parser.add_argument('-t', '--test_cases', action='extend', nargs='*')
+  parser.add_argument('-a', '--all', action='store_true')
 
-    args = parser.parse_args()
-    if not args.results_path:
-        raise AssertionError("No result path provided")
-    results_path = args.results_path
+  args = parser.parse_args()
+  input_path = "."
+  if args.input_path:
+    input_path = args.input_path
 
-    results = TestResults(results_directory=results_path,
-                          source_expected_directory=args.source_expected_path)
+  prefixes = args.test_cases
+  if args.all:
+    prefixes = None
 
-    if args.all:
-        results.batch_move()
-    elif args.tests:
-        results.handle_all_failed_images()
-    elif args.compare:
-        for file_prefix in args.compare:
-            results.show_images(file_prefix)
-    else:
-        results.batch_move(args.batch)
+  replace_file_names(path=input_path, output_path=args.output_path, removed="_actual.png",
+                     replacement=".png", prefixes=prefixes)

filament/include/filament/ColorGrading.h

@@ -21,17 +21,13 @@
 
 #include <filament/FilamentAPI.h>
 #include <filament/ToneMapper.h>
-#include <filament/ColorSpace.h>
 
 #include <utils/compiler.h>
 
 #include <math/mathfwd.h>
-#include <math/vec3.h>
-#include <math/vec4.h>
 
 #include <stdint.h>
 #include <stddef.h>
-#include <memory>
 
 namespace filament {
 
@@ -205,14 +201,15 @@ public:
          *
          * The default tone mapping operator is ACESLegacyToneMapper.
          *
-         * The ownership of the specified tone mapper is shared with the builder and built
-         * ColorGrading object.
+         * The specified tone mapper must have a lifecycle that exceeds the lifetime of
+         * this builder. Since the build(Engine&) method is synchronous, it is safe to
+         * delete the tone mapper object after that finishes executing.
          *
          * @param toneMapper The tone mapping operator to apply to the HDR color buffer
          *
          * @return This Builder, for chaining calls
          */
-        Builder& toneMapper(std::shared_ptr<ToneMapper> toneMapper) noexcept;
+        Builder& toneMapper(ToneMapper const* UTILS_NULLABLE toneMapper) noexcept;
 
         /**
          * Selects the tone mapping operator to apply to the HDR color buffer as the last
@@ -490,118 +487,7 @@ public:
         friend class FColorGrading;
     };
 
-    /** Returns the quality level used to create this ColorGrading object. */
-    QualityLevel getQuality() const noexcept;
-
-    /** Returns the LUT format used to create this ColorGrading object. */
-    LutFormat getLutFormat() const noexcept;
-
-    /** Returns the LUT dimensions used to create this ColorGrading object. */
-    uint8_t getLutDimensions() const noexcept;
-
-    /** Returns the tone mapper used to create this ColorGrading object. */
-    const ToneMapper& getToneMapper() const noexcept;
-
-    /** Returns whether luminance scaling was enabled during creation. */
-    bool isLuminanceScalingEnabled() const noexcept;
-
-    /** Returns whether gamut mapping was enabled during creation. */
-    bool isGamutMappingEnabled() const noexcept;
-
-    /** Returns the exposure value used to create this ColorGrading object. */
-    float getExposure() const noexcept;
-
-    /** Returns the night adaptation value used to create this ColorGrading object. */
-    float getNightAdaptation() const noexcept;
-
-    /** Returns the white balance temperature used to create this ColorGrading object. */
-    float getWhiteBalanceTemperature() const noexcept;
-
-    /** Returns the white balance tint used to create this ColorGrading object. */
-    float getWhiteBalanceTint() const noexcept;
-
-    /** Returns the channel mixer output for the red channel. */
-    math::float3 getChannelMixerOutRed() const noexcept;
-
-    /** Returns the channel mixer output for the green channel. */
-    math::float3 getChannelMixerOutGreen() const noexcept;
-
-    /** Returns the channel mixer output for the blue channel. */
-    math::float3 getChannelMixerOutBlue() const noexcept;
-
-    /** Returns the shadows adjustment used to create this ColorGrading object. */
-    math::float3 getShadows() const noexcept;
-
-    /** Returns the midtones adjustment used to create this ColorGrading object. */
-    math::float3 getMidtones() const noexcept;
-
-    /** Returns the highlights adjustment used to create this ColorGrading object. */
-    math::float3 getHighlights() const noexcept;
-
-    /** Returns the shadow/midtones/highlights ranges used to create this ColorGrading object. */
-    math::float4 getShadowMidtonesHighlightsRanges() const noexcept;
-
-    /** Returns the slope adjustment used to create this ColorGrading object. */
-    math::float3 getSlope() const noexcept;
-
-    /** Returns the offset adjustment used to create this ColorGrading object. */
-    math::float3 getOffset() const noexcept;
-
-    /** Returns the power adjustment used to create this ColorGrading object. */
-    math::float3 getPower() const noexcept;
-
-    /** Returns the contrast value used to create this ColorGrading object. */
-    float getContrast() const noexcept;
-
-    /** Returns the vibrance value used to create this ColorGrading object. */
-    float getVibrance() const noexcept;
-
-    /** Returns the saturation value used to create this ColorGrading object. */
-    float getSaturation() const noexcept;
-
-    /** Returns the shadow gamma curve adjustment used to create this ColorGrading object. */
-    math::float3 getCurvesShadowGamma() const noexcept;
-
-    /** Returns the mid-point curve adjustment used to create this ColorGrading object. */
-    math::float3 getCurvesMidPoint() const noexcept;
-
-    /** Returns the highlight scale curve adjustment used to create this ColorGrading object. */
-    math::float3 getCurvesHighlightScale() const noexcept;
-
-    /** Returns the output color space used to create this ColorGrading object. */
-    const color::ColorSpace& getOutputColorSpace() const noexcept;
-
-protected :
-    struct Settings {
-        LutFormat lutFormat = LutFormat::INTEGER;
-        uint8_t lutDimensions = 32;
-        std::shared_ptr<ToneMapper> toneMapper = std::make_shared<ACESLegacyToneMapper>();
-        bool luminanceScaling = false;
-        bool gummapMapping = false;
-        float exposure = 0.0f;
-        float nightAdaptation = 0.0f;
-        float whiteBalanceTemperature = 0.0f;
-        float whiteBalanceTint = 0.0f;
-        math::float3 channelMixerOutRed{1.0f, 0.0f, 0.0f};
-        math::float3 channelMixerOutGreen{0.0f, 1.0f, 0.0f};
-        math::float3 channelMixerOutBlue{0.0f, 0.0f, 1.0f};
-        math::float3 shadows{1.0f, 1.0f, 1.0f};
-        math::float3 midtones{1.0f, 1.0f, 1.0f};
-        math::float3 highlights{1.0f, 1.0f, 1.0f};
-        math::float4 ShadowMidtonesHighlightsRanges{0.0f, 0.333f, 0.55f, 1.0f};
-        math::float3 slope{1.0f};
-        math::float3 offset{0.0f};
-        math::float3 power{1.0f};
-        float contrast = 1.0f;
-        float vibrance = 1.0f;
-        float saturation = 1.0f;
-        math::float3 curvesShadowGamma{1.0f, 1.0f, 1.0f};
-        math::float3 curvesMidPoint{1.0f, 1.0f, 1.0f};
-        math::float3 curvesHighlightScale{1.0f, 1.0f, 1.0f};
-        color::ColorSpace colorSpace = color::Rec709 - color::sRGB - color::D65;
-    };
-    Settings mSettings;
-
+protected:
     // prevent heap allocation
     ~ColorGrading() = default;
 };

filament/include/filament/ToneMapper.h

@@ -54,8 +54,6 @@ namespace filament {
 struct UTILS_PUBLIC ToneMapper {
     ToneMapper() noexcept;
     virtual ~ToneMapper() noexcept;
-    ToneMapper(ToneMapper const&) noexcept;
-    ToneMapper& operator=(ToneMapper const&) noexcept = default;
 
     /**
      * Maps an open domain (or "scene referred" values) color value to display
@@ -71,13 +69,6 @@ struct UTILS_PUBLIC ToneMapper {
      */
     virtual math::float3 operator()(math::float3 c) const noexcept = 0;
 
-    /**
-     * Creates a copy of this tone mapper instance.
-     *
-     * @return A pointer to a new ToneMapper instance that is a copy of this instance.
-     */
-    virtual ToneMapper* clone() const noexcept = 0;
-
     /**
      * If true, then this function holds that f(x) = vec3(f(x.r), f(x.g), f(x.b))
      *
@@ -101,10 +92,8 @@ struct UTILS_PUBLIC ToneMapper {
 struct UTILS_PUBLIC LinearToneMapper final : public ToneMapper {
     LinearToneMapper() noexcept;
     ~LinearToneMapper() noexcept final;
-    LinearToneMapper(LinearToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 c) const noexcept override;
-    LinearToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return true; }
     bool isLDR() const noexcept override { return true; }
 };
@@ -117,10 +106,8 @@ struct UTILS_PUBLIC LinearToneMapper final : public ToneMapper {
 struct UTILS_PUBLIC ACESToneMapper final : public ToneMapper {
     ACESToneMapper() noexcept;
     ~ACESToneMapper() noexcept final;
-    ACESToneMapper(ACESToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 c) const noexcept override;
-    ACESToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return false; }
     bool isLDR() const noexcept override { return false; }
 };
@@ -134,10 +121,8 @@ struct UTILS_PUBLIC ACESToneMapper final : public ToneMapper {
 struct UTILS_PUBLIC ACESLegacyToneMapper final : public ToneMapper {
     ACESLegacyToneMapper() noexcept;
     ~ACESLegacyToneMapper() noexcept final;
-    ACESLegacyToneMapper(ACESLegacyToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 c) const noexcept override;
-    ACESLegacyToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return false; }
     bool isLDR() const noexcept override { return false; }
 };
@@ -151,10 +136,8 @@ struct UTILS_PUBLIC ACESLegacyToneMapper final : public ToneMapper {
 struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper {
     FilmicToneMapper() noexcept;
     ~FilmicToneMapper() noexcept final;
-    FilmicToneMapper(FilmicToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 x) const noexcept override;
-    FilmicToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return true; }
     bool isLDR() const noexcept override { return false; }
 };
@@ -167,10 +150,8 @@ struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper {
 struct UTILS_PUBLIC PBRNeutralToneMapper final : public ToneMapper {
     PBRNeutralToneMapper() noexcept;
     ~PBRNeutralToneMapper() noexcept final;
-    PBRNeutralToneMapper(PBRNeutralToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 x) const noexcept override;
-    virtual PBRNeutralToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return false; }
     bool isLDR() const noexcept override { return false; }
 };
@@ -192,10 +173,8 @@ struct UTILS_PUBLIC AgxToneMapper final : public ToneMapper {
      */
     explicit AgxToneMapper(AgxLook look = AgxLook::NONE) noexcept;
     ~AgxToneMapper() noexcept final;
-    AgxToneMapper(AgxToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 x) const noexcept override;
-    virtual AgxToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return false; }
     bool isLDR() const noexcept override { return false; }
 
@@ -236,13 +215,12 @@ struct UTILS_PUBLIC GenericToneMapper final : public ToneMapper {
     ) noexcept;
     ~GenericToneMapper() noexcept final;
 
-    GenericToneMapper(GenericToneMapper const& rhs) noexcept;
-    GenericToneMapper& operator=(GenericToneMapper const& rhs) noexcept;
+    GenericToneMapper(GenericToneMapper const&) = delete;
+    GenericToneMapper& operator=(GenericToneMapper const&) = delete;
     GenericToneMapper(GenericToneMapper&& rhs)  noexcept;
     GenericToneMapper& operator=(GenericToneMapper&& rhs) noexcept;
 
     math::float3 operator()(math::float3 x) const noexcept override;
-    GenericToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return true; }
     bool isLDR() const noexcept override { return false; }
 
@@ -305,10 +283,8 @@ private:
 struct UTILS_PUBLIC DisplayRangeToneMapper final : public ToneMapper {
     DisplayRangeToneMapper() noexcept;
     ~DisplayRangeToneMapper() noexcept override;
-    DisplayRangeToneMapper(DisplayRangeToneMapper const&) noexcept;
 
     math::float3 operator()(math::float3 c) const noexcept override;
-    DisplayRangeToneMapper* clone() const noexcept override;
     bool isOneDimensional() const noexcept override { return false; }
     bool isLDR() const noexcept override { return false; }
 };

filament/src/MaterialParser.cpp

@@ -648,32 +648,44 @@ bool ChunkAttributeInfo::unflatten(Unflattener& unflattener,
 bool ChunkDescriptorBindingsInfo::unflatten(Unflattener& unflattener,
         MaterialParser::DescriptorBindingsContainer* container) {
 
-    static_assert(sizeof(DescriptorSetBindingPoints) == sizeof(uint8_t));
-
-    uint8_t descriptorCount;
-    if (!unflattener.read(&descriptorCount)) {
+    uint8_t setCount;
+    if (!unflattener.read(&setCount)) {
         return false;
     }
 
-    auto& descriptors = (*container)[+DescriptorSetBindingPoints::PER_MATERIAL];
-    descriptors.reserve(descriptorCount);
-    for (size_t i = 0; i < descriptorCount; i++) {
-        CString name;
-        if (!unflattener.read(&name)) {
+    for (size_t j = 0; j < setCount; j++) {
+        static_assert(sizeof(DescriptorSetBindingPoints) == sizeof(uint8_t));
+
+        DescriptorSetBindingPoints set;
+        if (!unflattener.read(reinterpret_cast<uint8_t*>(&set))) {
             return false;
         }
-        uint8_t type;
-        if (!unflattener.read(&type)) {
+
+        uint8_t descriptorCount;
+        if (!unflattener.read(&descriptorCount)) {
             return false;
         }
-        uint8_t binding;
-        if (!unflattener.read(&binding)) {
-            return false;
+
+        auto& descriptors = (*container)[+set];
+        descriptors.reserve(descriptorCount);
+        for (size_t i = 0; i < descriptorCount; i++) {
+            CString name;
+            if (!unflattener.read(&name)) {
+                return false;
+            }
+            uint8_t type;
+            if (!unflattener.read(&type)) {
+                return false;
+            }
+            uint8_t binding;
+            if (!unflattener.read(&binding)) {
+                return false;
+            }
+            descriptors.push_back({
+                    std::move(name),
+                    DescriptorType(type),
+                    descriptor_binding_t(binding)});
         }
-        descriptors.push_back({
-                std::move(name),
-                DescriptorType(type),
-                descriptor_binding_t(binding)});
     }
 
     return true;
@@ -681,40 +693,42 @@ bool ChunkDescriptorBindingsInfo::unflatten(Unflattener& unflattener,
 
 bool ChunkDescriptorSetLayoutInfo::unflatten(Unflattener& unflattener,
         MaterialParser::DescriptorSetLayoutContainer* container) {
-    uint8_t descriptorCount;
-    if (!unflattener.read(&descriptorCount)) {
-        return false;
-    }
-    auto& descriptors = container->bindings;
-    descriptors.reserve(descriptorCount);
-    for (size_t i = 0; i < descriptorCount; i++) {
-        uint8_t type;
-        if (!unflattener.read(&type)) {
+    for (size_t j = 0; j < 2; j++) {
+        uint8_t descriptorCount;
+        if (!unflattener.read(&descriptorCount)) {
             return false;
         }
-        uint8_t stageFlags;
-        if (!unflattener.read(&stageFlags)) {
-            return false;
+        auto& descriptors = (*container)[j].bindings;
+        descriptors.reserve(descriptorCount);
+        for (size_t i = 0; i < descriptorCount; i++) {
+            uint8_t type;
+            if (!unflattener.read(&type)) {
+                return false;
+            }
+            uint8_t stageFlags;
+            if (!unflattener.read(&stageFlags)) {
+                return false;
+            }
+            uint8_t binding;
+            if (!unflattener.read(&binding)) {
+                return false;
+            }
+            uint8_t flags;
+            if (!unflattener.read(&flags)) {
+                return false;
+            }
+            uint16_t count;
+            if (!unflattener.read(&count)) {
+                return false;
+            }
+            descriptors.push_back({
+                    DescriptorType(type),
+                    ShaderStageFlags(stageFlags),
+                    descriptor_binding_t(binding),
+                    DescriptorFlags(flags),
+                    count,
+            });
         }
-        uint8_t binding;
-        if (!unflattener.read(&binding)) {
-            return false;
-        }
-        uint8_t flags;
-        if (!unflattener.read(&flags)) {
-            return false;
-        }
-        uint16_t count;
-        if (!unflattener.read(&count)) {
-            return false;
-        }
-        descriptors.push_back({
-            DescriptorType(type),
-            ShaderStageFlags(stageFlags),
-            descriptor_binding_t(binding),
-            DescriptorFlags(flags),
-            count,
-        });
     }
     return true;
 }

filament/src/MaterialParser.h

@@ -93,7 +93,7 @@ public:
     using DescriptorBindingsContainer = backend::Program::DescriptorSetInfo;
     bool getDescriptorBindings(DescriptorBindingsContainer* container) const noexcept;
 
-    using DescriptorSetLayoutContainer = backend::DescriptorSetLayout;
+    using DescriptorSetLayoutContainer = std::array<backend::DescriptorSetLayout, 2>;
     bool getDescriptorSetLayout(DescriptorSetLayoutContainer* container) const noexcept;
 
     bool getDepthWriteSet(bool* value) const noexcept;

filament/src/ToneMapper.cpp

@@ -188,8 +188,7 @@ float3 ACES(float3 color, float brightness) noexcept {
 
 #define DEFAULT_CONSTRUCTORS(A) \
         A::A() noexcept = default; \
-        A::~A() noexcept = default; \
-        A::A(A const&) noexcept = default;
+        A::~A() noexcept = default;
 
 DEFAULT_CONSTRUCTORS(ToneMapper)
 
@@ -203,10 +202,6 @@ float3 LinearToneMapper::operator()(float3 const v) const noexcept {
     return saturate(v);
 }
 
-LinearToneMapper* LinearToneMapper::clone() const noexcept {
-    return new LinearToneMapper(*this);
-}
-
 //------------------------------------------------------------------------------
 // ACES tone mappers
 //------------------------------------------------------------------------------
@@ -217,20 +212,12 @@ float3 ACESToneMapper::operator()(float3 const c) const noexcept {
     return aces::ACES(c, 1.0f);
 }
 
-ACESToneMapper* ACESToneMapper::clone() const noexcept {
-    return new ACESToneMapper(*this);
-}
-
 DEFAULT_CONSTRUCTORS(ACESLegacyToneMapper)
 
 float3 ACESLegacyToneMapper::operator()(float3 const c) const noexcept {
     return aces::ACES(c, 1.0f / 0.6f);
 }
 
-ACESLegacyToneMapper* ACESLegacyToneMapper::clone() const noexcept {
-    return new ACESLegacyToneMapper(*this);
-}
-
 DEFAULT_CONSTRUCTORS(FilmicToneMapper)
 
 float3 FilmicToneMapper::operator()(float3 const x) const noexcept {
@@ -243,10 +230,6 @@ float3 FilmicToneMapper::operator()(float3 const x) const noexcept {
     return (x * (a * x + b)) / (x * (c * x + d) + e);
 }
 
-FilmicToneMapper* FilmicToneMapper::clone() const noexcept {
-    return new FilmicToneMapper(*this);
-}
-
 //------------------------------------------------------------------------------
 // PBR Neutral tone mapper
 //------------------------------------------------------------------------------
@@ -273,17 +256,12 @@ float3 PBRNeutralToneMapper::operator()(float3 color) const noexcept {
     return mix(color, float3(newPeak), g);
 }
 
-PBRNeutralToneMapper* PBRNeutralToneMapper::clone() const noexcept {
-    return new PBRNeutralToneMapper(*this);
-}
-
 //------------------------------------------------------------------------------
 // AgX tone mapper
 //------------------------------------------------------------------------------
 
 AgxToneMapper::AgxToneMapper(AgxLook const look) noexcept : look(look) {}
 AgxToneMapper::~AgxToneMapper() noexcept = default;
-AgxToneMapper::AgxToneMapper(AgxToneMapper const& rhs) noexcept = default;
 
 // These matrices taken from Blender's implementation of AgX, which works with Rec.2020 primaries.
 // https://github.com/EaryChow/AgX_LUT_Gen/blob/main/AgXBaseRec2020.py
@@ -378,10 +356,6 @@ float3 AgxToneMapper::operator()(float3 v) const noexcept {
     return v;
 }
 
-AgxToneMapper* AgxToneMapper::clone() const noexcept {
-    return new AgxToneMapper(*this);
-}
-
 //------------------------------------------------------------------------------
 // Display range tone mapper
 //------------------------------------------------------------------------------
@@ -420,10 +394,6 @@ float3 DisplayRangeToneMapper::operator()(float3 const c) const noexcept {
     return mix(debugColors[index], debugColors[index + 1], saturate(v - float(index)));
 }
 
-DisplayRangeToneMapper* DisplayRangeToneMapper::clone() const noexcept {
-    return new DisplayRangeToneMapper(*this);
-}
-
 //------------------------------------------------------------------------------
 // Generic tone mapper
 //------------------------------------------------------------------------------
@@ -484,19 +454,7 @@ GenericToneMapper::~GenericToneMapper() noexcept {
     delete mOptions;
 }
 
-GenericToneMapper::GenericToneMapper(GenericToneMapper const& rhs) noexcept {
-    mOptions = new Options(*rhs.mOptions);
-}
-
-GenericToneMapper& GenericToneMapper::operator=(GenericToneMapper const& rhs) noexcept {
-    if (this != &rhs) {
-        delete mOptions;
-        mOptions = new Options(*rhs.mOptions);
-    }
-    return *this;
-}
-
-GenericToneMapper::GenericToneMapper(GenericToneMapper&& rhs) noexcept : mOptions(rhs.mOptions) {
+GenericToneMapper::GenericToneMapper(GenericToneMapper&& rhs)  noexcept : mOptions(rhs.mOptions) {
     rhs.mOptions = nullptr;
 }
 
@@ -511,10 +469,6 @@ float3 GenericToneMapper::operator()(float3 x) const noexcept {
     return mOptions->outputScale * x / (x + mOptions->inputScale);
 }
 
-GenericToneMapper* GenericToneMapper::clone() const noexcept {
-    return new GenericToneMapper(*this);
-}
-
 float GenericToneMapper::getContrast() const noexcept { return  mOptions->contrast; }
 float GenericToneMapper::getMidGrayIn() const noexcept { return  mOptions->midGrayIn; }
 float GenericToneMapper::getMidGrayOut() const noexcept { return  mOptions->midGrayOut; }

filament/src/details/ColorGrading.cpp

@@ -49,7 +49,7 @@ using namespace backend;
 //------------------------------------------------------------------------------
 
 struct ColorGrading::BuilderDetails {
-    std::shared_ptr<ToneMapper> toneMapper = nullptr;
+    const ToneMapper* toneMapper = nullptr;
 
 #if defined(__clang__)
 #pragma clang diagnostic push
@@ -173,7 +173,7 @@ ColorGrading::Builder& ColorGrading::Builder::dimensions(uint8_t const dim) noex
     return *this;
 }
 
-ColorGrading::Builder& ColorGrading::Builder::toneMapper(std::shared_ptr<ToneMapper> toneMapper) noexcept {
+ColorGrading::Builder& ColorGrading::Builder::toneMapper(const ToneMapper* toneMapper) noexcept {
     mImpl->toneMapper = toneMapper;
     return *this;
 }
@@ -271,110 +271,6 @@ ColorGrading::Builder& ColorGrading::Builder::outputColorSpace(
     return *this;
 }
 
-ColorGrading::LutFormat ColorGrading::getLutFormat() const noexcept {
-    return mSettings.lutFormat;
-}
-
-uint8_t ColorGrading::getLutDimensions() const noexcept {
-    return mSettings.lutDimensions;
-}
-
-const ToneMapper& ColorGrading::getToneMapper() const noexcept {
-    return *mSettings.toneMapper;
-}
-
-bool ColorGrading::isLuminanceScalingEnabled() const noexcept {
-    return mSettings.luminanceScaling;
-}
-
-bool ColorGrading::isGamutMappingEnabled() const noexcept {
-    return mSettings.gummapMapping;
-}
-
-float ColorGrading::getExposure() const noexcept {
-    return mSettings.exposure;
-}
-
-float ColorGrading::getNightAdaptation() const noexcept {
-    return mSettings.nightAdaptation;
-}
-
-float ColorGrading::getWhiteBalanceTemperature() const noexcept {
-    return mSettings.whiteBalanceTemperature;
-}
-
-float ColorGrading::getWhiteBalanceTint() const noexcept {
-    return mSettings.whiteBalanceTint;
-}
-
-math::float3 ColorGrading::getChannelMixerOutRed() const noexcept {
-    return mSettings.channelMixerOutRed;
-}
-
-math::float3 ColorGrading::getChannelMixerOutGreen() const noexcept {
-    return mSettings.channelMixerOutGreen;
-}
-
-math::float3 ColorGrading::getChannelMixerOutBlue() const noexcept {
-    return mSettings.channelMixerOutBlue;
-}
-
-math::float3 ColorGrading::getShadows() const noexcept {
-    return mSettings.shadows;
-}
-
-math::float3 ColorGrading::getMidtones() const noexcept {
-    return mSettings.midtones;
-}
-
-math::float3 ColorGrading::getHighlights() const noexcept {
-    return mSettings.highlights;
-}
-
-math::float4 ColorGrading::getShadowMidtonesHighlightsRanges() const noexcept {
-    return mSettings.ShadowMidtonesHighlightsRanges;
-}
-
-math::float3 ColorGrading::getSlope() const noexcept {
-    return mSettings.slope;
-}
-
-math::float3 ColorGrading::getOffset() const noexcept {
-    return mSettings.offset;
-}
-
-math::float3 ColorGrading::getPower() const noexcept {
-    return mSettings.power;
-}
-
-float ColorGrading::getContrast() const noexcept {
-    return mSettings.contrast;
-}
-
-float ColorGrading::getVibrance() const noexcept {
-    return mSettings.vibrance;
-}
-
-float ColorGrading::getSaturation() const noexcept {
-    return mSettings.saturation;
-}
-
-math::float3 ColorGrading::getCurvesShadowGamma() const noexcept {
-    return mSettings.curvesShadowGamma;
-}
-
-math::float3 ColorGrading::getCurvesMidPoint() const noexcept {
-    return mSettings.curvesMidPoint;
-}
-
-math::float3 ColorGrading::getCurvesHighlightScale() const noexcept {
-    return mSettings.curvesHighlightScale;
-}
-
-const color::ColorSpace& ColorGrading::getOutputColorSpace() const noexcept {
-    return mSettings.colorSpace;
-}
-
 #if defined(__clang__)
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
@@ -391,25 +287,30 @@ ColorGrading* ColorGrading::Builder::build(Engine& engine) {
     if (needToneMapper) {
         switch (mImpl->toneMapping) {
             case ToneMapping::LINEAR:
-                mImpl->toneMapper = std::make_shared<LinearToneMapper>();
+                mImpl->toneMapper = new LinearToneMapper();
                 break;
             case ToneMapping::ACES_LEGACY:
-                mImpl->toneMapper = std::make_shared<ACESLegacyToneMapper>();
+                mImpl->toneMapper = new ACESLegacyToneMapper();
                 break;
             case ToneMapping::ACES:
-                mImpl->toneMapper = std::make_shared<ACESToneMapper>();
+                mImpl->toneMapper = new ACESToneMapper();
                 break;
             case ToneMapping::FILMIC:
-                mImpl->toneMapper = std::make_shared<FilmicToneMapper>();
+                mImpl->toneMapper = new FilmicToneMapper();
                 break;
             case ToneMapping::DISPLAY_RANGE:
-                mImpl->toneMapper = std::make_shared<DisplayRangeToneMapper>();
+                mImpl->toneMapper = new DisplayRangeToneMapper();
                 break;
         }
     }
 
     FColorGrading* colorGrading = downcast(engine).createColorGrading(*this);
 
+    if (needToneMapper) {
+        delete mImpl->toneMapper;
+        mImpl->toneMapper = nullptr;
+    }
+
     return colorGrading;
 }
 
@@ -985,9 +886,6 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) {
                 data, lutElementCount * elementSize, format, type,
                         [](void* buffer, size_t, void*) { free(buffer); }
                         });
-
-    // Initialize settings from builder
-    initializeSettings(builder, mSettings);
 }
 
 FColorGrading::~FColorGrading() noexcept = default;
@@ -997,33 +895,4 @@ void FColorGrading::terminate(FEngine& engine) {
     driver.destroyTexture(mLutHandle);
 }
 
-void FColorGrading::initializeSettings(const Builder& builder, Settings& settings) noexcept {
-    settings.lutFormat = builder->format;
-    settings.lutDimensions = builder->dimension;
-    settings.toneMapper = builder->toneMapper;
-    settings.luminanceScaling = builder->luminanceScaling;
-    settings.gummapMapping = builder->gamutMapping;
-    settings.exposure = builder->exposure;
-    settings.nightAdaptation = builder->nightAdaptation;
-    settings.whiteBalanceTemperature = builder->whiteBalance.x;
-    settings.whiteBalanceTint = builder->whiteBalance.y;
-    settings.channelMixerOutRed = builder->outRed;
-    settings.channelMixerOutGreen = builder->outGreen;
-    settings.channelMixerOutBlue = builder->outBlue;
-    settings.shadows = builder->shadows;
-    settings.midtones = builder->midtones;
-    settings.highlights = builder->highlights;
-    settings.ShadowMidtonesHighlightsRanges = builder->tonalRanges;
-    settings.slope = builder->slope;
-    settings.offset = builder->offset;
-    settings.power = builder->power;
-    settings.contrast = builder->contrast;
-    settings.vibrance = builder->vibrance;
-    settings.saturation = builder->saturation;
-    settings.curvesShadowGamma = builder->shadowGamma;
-    settings.curvesMidPoint = builder->midPoint;
-    settings.curvesHighlightScale = builder->highlightScale;
-    settings.colorSpace = builder->outputColorSpace;
-}
-
 } //namespace filament

filament/src/details/ColorGrading.h

@@ -51,8 +51,6 @@ private:
     uint32_t mDimension;
     bool mIsOneDimensional;
     bool mIsLDR;
-
-    static void initializeSettings(const Builder& builder, Settings& settings) noexcept;
 };
 
 FILAMENT_DOWNCAST(ColorGrading)

filament/src/details/Material.cpp

@@ -609,12 +609,9 @@ Program FMaterial::getProgramWithVariants(
         program.attributes(mAttributeInfo);
     }
 
-    program.descriptorBindings(+DescriptorSetBindingPoints::PER_VIEW,
-            mProgramDescriptorBindings[+DescriptorSetBindingPoints::PER_VIEW]);
-    program.descriptorBindings(+DescriptorSetBindingPoints::PER_RENDERABLE,
-            mProgramDescriptorBindings[+DescriptorSetBindingPoints::PER_RENDERABLE]);
-    program.descriptorBindings(+DescriptorSetBindingPoints::PER_MATERIAL,
-            mProgramDescriptorBindings[+DescriptorSetBindingPoints::PER_MATERIAL]);
+    program.descriptorBindings(0, mProgramDescriptorBindings[0]);
+    program.descriptorBindings(1, mProgramDescriptorBindings[1]);
+    program.descriptorBindings(2, mProgramDescriptorBindings[2]);
     program.specializationConstants(mSpecializationConstants);
 
     program.pushConstants(ShaderStage::VERTEX, mPushConstants[uint8_t(ShaderStage::VERTEX)]);
@@ -1143,45 +1140,37 @@ void FMaterial::precacheDepthVariants(FEngine& engine) {
 void FMaterial::processDescriptorSets(FEngine& engine, MaterialParser const* const parser) {
     UTILS_UNUSED_IN_RELEASE bool success;
 
+
     success = parser->getDescriptorBindings(&mProgramDescriptorBindings);
     assert_invariant(success);
 
-    backend::DescriptorSetLayout descriptorSetLayout;
+    std::array<backend::DescriptorSetLayout, 2> descriptorSetLayout;
     success = parser->getDescriptorSetLayout(&descriptorSetLayout);
     assert_invariant(success);
-    auto perMatLabel = mName;
-    perMatLabel.append("_perMat");
-    descriptorSetLayout.label = std::move(perMatLabel);
 
-    // get the PER_VIEW descriptor binding info
-    auto perViewDescriptorSetLayout =
-            descriptor_sets::getPerViewDescriptorSetLayout(mMaterialDomain, mVariantFilterMask,
-            mIsVariantLit || mHasShadowMultiplier, mReflectionMode, mRefractionMode);
-    auto perViewLabel = mName;
-    perViewLabel.append("_perView");
-    perViewDescriptorSetLayout.label = std::move(perViewLabel);
+    // Setup Labels for debugging
+    utils::CString namestr;
+    utils::StaticString perViewStr = "_perView";
+    utils::StaticString singleStr = "_single";
 
-    // get the PER_RENDERABLE and PER_VIEW descriptor binding info
-    for (auto&& [bindingPoint, descriptorSetLayout] : {
-            std::pair{ DescriptorSetBindingPoints::PER_RENDERABLE,
-                    descriptor_sets::getPerRenderableLayout() },
-            std::pair{ DescriptorSetBindingPoints::PER_VIEW,
-                    perViewDescriptorSetLayout }}) {
-        Program::DescriptorBindingsInfo& descriptors = mProgramDescriptorBindings[+bindingPoint];
-        descriptors.reserve(descriptorSetLayout.bindings.size());
-        for (auto const& entry: descriptorSetLayout.bindings) {
-            auto const& name = descriptor_sets::getDescriptorName(bindingPoint, entry.binding);
-            descriptors.push_back({ name, entry.type, entry.binding });
-        }
-    }
+    parser->getName(&namestr);
+
+    utils::CString singleLabel(namestr.c_str(), namestr.length() + singleStr.length());
+    memccpy(singleLabel.c_str() + namestr.length(), singleStr.c_str(), '\0', singleStr.length());
+
+    utils::CString perViewLabel(namestr.c_str(), namestr.length() + perViewStr.length());
+    memccpy(perViewLabel.c_str() + namestr.length(), perViewStr.c_str(), '\0', perViewStr.length());
+
+    descriptorSetLayout[0].label = namestr;
+    descriptorSetLayout[1].label = perViewLabel;
 
     mDescriptorSetLayout = {
             engine.getDescriptorSetLayoutFactory(),
-            engine.getDriverApi(), std::move(descriptorSetLayout) };
+            engine.getDriverApi(), std::move(descriptorSetLayout[0]) };
 
     mPerViewDescriptorSetLayout = {
             engine.getDescriptorSetLayoutFactory(),
-            engine.getDriverApi(), perViewDescriptorSetLayout };
+            engine.getDriverApi(), std::move(descriptorSetLayout[1]) };
 }
 
 descriptor_binding_t FMaterial::getSamplerBinding(

ios/CocoaPods/Filament.podspec

@@ -1,12 +1,12 @@
 Pod::Spec.new do |spec|
   spec.name = "Filament"
-  spec.version = "1.59.5"
+  spec.version = "1.59.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.59.5/filament-v1.59.5-ios.tgz" }
+  spec.source = { :http => "https://github.com/google/filament/releases/download/v1.59.4/filament-v1.59.4-ios.tgz" }
 
   # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon.
   spec.pod_target_xcconfig = {

ios/samples/README.md

@@ -102,8 +102,7 @@ project in Xcode to see changes take effect.
 
 ## Building iOS Samples with ASan / UBSan
 
-1. Turn on ASan / UBSan in Filament's top-level CMakeLists.txt by passing
-   `-DFILAMENT_ENABLE_ASAN_UBSAN=1` to trigger the following line:
+1. Turn on ASan / UBSan in Filament's top-level CMakeLists.txt by uncommenting the following line:
 
 ```
 set(EXTRA_SANITIZE_OPTIONS "-fsanitize=undefined -fsanitize=address")

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

@@ -20,7 +20,6 @@
 #include <backend/DriverEnums.h>
 
 #include <private/filament/EngineEnums.h>
-#include <private/filament/Variant.h>
 
 #include <filament/MaterialEnums.h>
 
@@ -40,16 +39,8 @@ backend::DescriptorSetLayout getPerViewDescriptorSetLayout(
         ReflectionMode reflectionMode,
         RefractionMode refractionMode) noexcept;
 
-backend::DescriptorSetLayout getPerViewDescriptorSetLayoutWithVariant(
-        Variant variant,
-        MaterialDomain domain,
-        UserVariantFilterMask variantFilter,
-        bool isLit,
-        ReflectionMode reflectionMode,
-        RefractionMode refractionMode) noexcept;
-
 utils::CString getDescriptorName(
-        DescriptorSetBindingPoints set,
+        filament::DescriptorSetBindingPoints set,
         backend::descriptor_binding_t binding) noexcept;
 
 } // namespace filament::descriptor_sets

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

@@ -23,8 +23,6 @@
 #include <utils/CString.h>
 #include <utils/FixedCapacityVector.h>
 
-#include <private/filament/DescriptorSets.h>
-
 #include <initializer_list>
 #include <unordered_map>
 #include <string_view>
@@ -130,9 +128,6 @@ public:
 
     static utils::CString generateUniformName(const char* group, const char* sampler) noexcept;
 
-    static SamplerInfoList filterSamplerList(SamplerInfoList list,
-            backend::DescriptorSetLayout const& descriptorSetLayout);
-
 private:
     friend class Builder;
 

libs/filabridge/src/DescriptorSets.cpp

@@ -17,7 +17,6 @@
 #include "private/filament/DescriptorSets.h"
 
 #include <private/filament/EngineEnums.h>
-#include <private/filament/Variant.h>
 
 #include <filament/MaterialEnums.h>
 
@@ -27,35 +26,38 @@
 #include <utils/debug.h>
 
 #include <algorithm>
-#include <initializer_list>
-#include <string_view>
 #include <unordered_map>
+#include <string_view>
 
 namespace filament::descriptor_sets {
 
 using namespace backend;
 
-static constexpr std::initializer_list<DescriptorSetLayoutBinding> postProcessDescriptorSetLayoutList = {
+static DescriptorSetLayout const postProcessDescriptorSetLayout{
+    utils::StaticString("postProcess"),{
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FRAME_UNIFORMS },
-};
+}};
 
-static constexpr std::initializer_list<DescriptorSetLayoutBinding> depthVariantDescriptorSetLayoutList = {
+static DescriptorSetLayout const depthVariantDescriptorSetLayout{
+    utils::StaticString("depthVariant"),{
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FRAME_UNIFORMS },
-};
+}};
 
 // ssrVariantDescriptorSetLayout must match perViewDescriptorSetLayout's vertex stage. This is
 // because the SSR variant is always using the "standard" vertex shader (i.e. there is no
 // dedicated SSR vertex shader), which uses perViewDescriptorSetLayout.
 // This means that PerViewBindingPoints::SHADOWS must be in the layout even though it's not used
 // by the SSR variant.
-static constexpr std::initializer_list<DescriptorSetLayoutBinding> ssrVariantDescriptorSetLayoutList = {
-    { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FRAME_UNIFORMS },
-    { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SHADOWS        },
-    { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::STRUCTURE      },
-    { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SSR            },
-};
+static DescriptorSetLayout const ssrVariantDescriptorSetLayout{
+    utils::StaticString("ssrVariant"),{
+   { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FRAME_UNIFORMS },
+   { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SHADOWS        },
+   { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::STRUCTURE      },
+   { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SSR            },
+}};
 
-static constexpr std::initializer_list<DescriptorSetLayoutBinding> perViewDescriptorSetLayoutList = {
+static DescriptorSetLayout perViewDescriptorSetLayout = {
+    utils::StaticString("perView"),{
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FRAME_UNIFORMS },
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SHADOWS        },
     { DescriptorType::UNIFORM_BUFFER,                            ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::LIGHTS         },
@@ -68,37 +70,17 @@ static constexpr std::initializer_list<DescriptorSetLayoutBinding> perViewDescri
     { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SSAO           },
     { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::SSR            },
     { DescriptorType::SAMPLER,                                   ShaderStageFlags::FRAGMENT,  +PerViewBindingPoints::FOG            },
-};
+}};
 
-static constexpr std::initializer_list<DescriptorSetLayoutBinding> perRenderableDescriptorSetLayoutList = {
+static DescriptorSetLayout perRenderableDescriptorSetLayout = {
+    utils::StaticString("perRenderable"),{
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerRenderableBindingPoints::OBJECT_UNIFORMS, DescriptorFlags::DYNAMIC_OFFSET },
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerRenderableBindingPoints::BONES_UNIFORMS,  DescriptorFlags::DYNAMIC_OFFSET },
     { DescriptorType::UNIFORM_BUFFER, ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT,  +PerRenderableBindingPoints::MORPHING_UNIFORMS         },
     { DescriptorType::SAMPLER,        ShaderStageFlags::VERTEX                             ,  +PerRenderableBindingPoints::MORPH_TARGET_POSITIONS    },
     { DescriptorType::SAMPLER,        ShaderStageFlags::VERTEX                             ,  +PerRenderableBindingPoints::MORPH_TARGET_TANGENTS     },
     { DescriptorType::SAMPLER,        ShaderStageFlags::VERTEX                             ,  +PerRenderableBindingPoints::BONES_INDICES_AND_WEIGHTS },
-};
-
-// used for post-processing passes
-static DescriptorSetLayout const postProcessDescriptorSetLayout{ utils::StaticString("postProcess"),
-    postProcessDescriptorSetLayoutList };
-
-// used to generate shadow-maps
-static DescriptorSetLayout const depthVariantDescriptorSetLayout{
-    utils::StaticString("depthVariant"), depthVariantDescriptorSetLayoutList
-};
-
-static DescriptorSetLayout const ssrVariantDescriptorSetLayout{ utils::StaticString("ssrVariant"),
-    ssrVariantDescriptorSetLayoutList };
-
-// Used for generating the color pass (i.e. the main pass). This is in fact a template that gets
-// declined into 8 different layouts, based on variants.
-static DescriptorSetLayout perViewDescriptorSetLayout = { utils::StaticString("perView"),
-    perViewDescriptorSetLayoutList };
-
-static DescriptorSetLayout perRenderableDescriptorSetLayout = {
-    utils::StaticString("perRenderable"), perRenderableDescriptorSetLayoutList
-};
+}};
 
 DescriptorSetLayout const& getPostProcessLayout() noexcept {
     return postProcessDescriptorSetLayout;
@@ -116,8 +98,8 @@ DescriptorSetLayout const& getPerRenderableLayout() noexcept {
     return perRenderableDescriptorSetLayout;
 }
 
-utils::CString getDescriptorName(DescriptorSetBindingPoints const set,
-        descriptor_binding_t const binding) noexcept {
+utils::CString getDescriptorName(DescriptorSetBindingPoints set,
+        descriptor_binding_t binding) noexcept {
     using namespace std::literals;
 
     static std::unordered_map<descriptor_binding_t, std::string_view> const set0{{
@@ -163,11 +145,11 @@ utils::CString getDescriptorName(DescriptorSetBindingPoints const set,
 }
 
 DescriptorSetLayout getPerViewDescriptorSetLayout(
-        MaterialDomain const domain,
-        UserVariantFilterMask const variantFilter,
-        bool const isLit,
-        ReflectionMode const reflectionMode,
-        RefractionMode const refractionMode) noexcept {
+        MaterialDomain domain,
+        UserVariantFilterMask variantFilter,
+        bool isLit,
+        ReflectionMode reflectionMode,
+        RefractionMode refractionMode) noexcept {
 
     bool const ssr = reflectionMode == ReflectionMode::SCREEN_SPACE ||
                      refractionMode == RefractionMode::SCREEN_SPACE;
@@ -210,65 +192,11 @@ DescriptorSetLayout getPerViewDescriptorSetLayout(
             return layout;
         }
         case MaterialDomain::POST_PROCESS:
-            return postProcessDescriptorSetLayout;
+            return descriptor_sets::getPostProcessLayout();
         case MaterialDomain::COMPUTE:
             // TODO: what's the layout for compute?
-            return postProcessDescriptorSetLayout;
+            return descriptor_sets::getPostProcessLayout();
     }
 }
 
-DescriptorSetLayout getPerViewDescriptorSetLayoutWithVariant(
-        Variant const variant,
-        MaterialDomain domain,
-        UserVariantFilterMask const variantFilter,
-        bool const isLit,
-        ReflectionMode const reflectionMode,
-        RefractionMode const refractionMode) noexcept {
-    if (Variant::isValidDepthVariant(variant)) {
-        return depthVariantDescriptorSetLayout;
-    }
-    if (Variant::isSSRVariant(variant)) {
-        return ssrVariantDescriptorSetLayout;
-    }
-    // We need to filter out all the descriptors not included in the "resolved" layout below
-    return getPerViewDescriptorSetLayout(domain, variantFilter,
-            isLit, reflectionMode, refractionMode);
-}
-
-
-
-template<class ITERATOR, class PREDICATE>
-constexpr static ITERATOR find_if(ITERATOR first, ITERATOR last, PREDICATE pred) {
-    for (; first != last; ++first)
-        if (pred(*first)) break;
-    return first;
-}
-
-constexpr static bool checkConsistency() noexcept {
-    // check that all descriptors that apply to the vertex stage in perViewDescriptorSetLayout
-    // are present in ssrVariantDescriptorSetLayout; meaning that the latter is compatible
-    // with the former.
-    for (auto const& r: perViewDescriptorSetLayoutList) {
-        if (hasShaderType(r.stageFlags, ShaderStage::VERTEX)) {
-            auto const pos = find_if(
-                    ssrVariantDescriptorSetLayoutList.begin(),
-                    ssrVariantDescriptorSetLayoutList.end(),
-                    [r](auto const& l) {
-                        return l.count == r.count &&
-                               l.type == r.type &&
-                               l.binding == r.binding &&
-                               l.flags == r.flags &&
-                               l.stageFlags == r.stageFlags;
-                    });
-            if (pos == ssrVariantDescriptorSetLayoutList.end()) {
-                return false;
-            }
-        }
-    }
-    return true;
-}
-
-static_assert(checkConsistency(), "ssrVariantDescriptorSetLayout is not compatible with "
-        "perViewDescriptorSetLayout");
-
 } // namespace filament::descriptor_sets

libs/filabridge/src/SamplerInterfaceBlock.cpp

@@ -16,7 +16,6 @@
 
 #include "private/filament/SamplerInterfaceBlock.h"
 
-#include <private/filament/DescriptorSets.h>
 
 #include <backend/DriverEnums.h>
 
@@ -103,7 +102,7 @@ const SamplerInterfaceBlock::SamplerInfo* SamplerInterfaceBlock::getSamplerInfo(
     return &mSamplersInfoList[pos->second];
 }
 
-CString SamplerInterfaceBlock::generateUniformName(const char* group, const char* sampler) noexcept {
+utils::CString SamplerInterfaceBlock::generateUniformName(const char* group, const char* sampler) noexcept {
     char uniformName[256];
 
     // sampler interface block name
@@ -118,27 +117,9 @@ CString SamplerInterfaceBlock::generateUniformName(const char* group, const char
             std::min(sizeof(uniformName) / 2 - 2, strlen(sampler)),
             prefix + 1);
     *last++ = 0; // null terminator
-    assert_invariant(last <= std::end(uniformName));
+    assert(last <= std::end(uniformName));
 
     return CString{ uniformName, size_t(last - uniformName) - 1u };
 }
 
-SamplerInterfaceBlock::SamplerInfoList SamplerInterfaceBlock::filterSamplerList(
-        SamplerInfoList list, backend::DescriptorSetLayout const& descriptorSetLayout) {
-    // remove all the samplers that are not included in the descriptor-set layout
-    list.erase(
-            std::remove_if(list.begin(), list.end(),
-                    [&](auto const& entry) {
-                        auto pos = std::find_if(
-                                descriptorSetLayout.bindings.begin(),
-                                descriptorSetLayout.bindings.end(),
-                                [&entry](const auto& item) {
-                                    return item.binding == entry.binding;
-                                });
-                        return pos == descriptorSetLayout.bindings.end();
-                    }), list.end());
-
-    return list;
-}
-
 } // namespace filament

libs/filamat/include/filamat/MaterialBuilder.h

@@ -603,7 +603,7 @@ public:
      * extension will be derived from the shader stage. For example, mymaterial_0x0e.frag,
      * mymaterial_0x18.vert, etc.
      */
-    MaterialBuilder& saveRawVariants(bool saveRawVariants) noexcept;
+    MaterialBuilder& saveRawVariants(bool saveVariants) noexcept;
 
     //! If true, will include debugging information in generated SPIRV.
     MaterialBuilder& generateDebugInfo(bool generateDebugInfo) noexcept;
@@ -635,7 +635,7 @@ public:
      * Build the material. If you are using the Filament engine with this library, you should use
      * the job system provided by Engine.
      */
-    Package build(utils::JobSystem& jobSystem);
+    Package build(utils::JobSystem& jobSystem) noexcept;
 
 public:
     // The methods and types below are for internal use
@@ -809,7 +809,7 @@ private:
     // Multiple calls to findProperties accumulate the property sets across fragment
     // and vertex shaders in mProperties.
     bool findProperties(filament::backend::ShaderStage type,
-            MaterialBuilder::PropertyList const& allProperties,
+            MaterialBuilder::PropertyList& allProperties,
             CodeGenParams const& semanticCodeGenParams) noexcept;
 
     bool runSemanticAnalysis(MaterialInfo* inOutInfo,

libs/filamat/src/GLSLPostProcessor.cpp

@@ -34,19 +34,14 @@
 
 #include "MetalArgumentBuffer.h"
 #include "SpirvFixup.h"
+#include "utils/ostream.h"
 
 #include <filament/MaterialEnums.h>
 
-#include <utils/compiler.h>
-#include <utils/debug.h>
 #include <utils/Log.h>
-#include <utils/ostream.h>
 
-#include <algorithm>
-#include <optional>
 #include <sstream>
 #include <unordered_map>
-#include <utility>
 #include <vector>
 
 #ifdef FILAMENT_SUPPORTS_WEBGPU
@@ -156,16 +151,21 @@ DescriptorSetLayout getPerMaterialDescriptorSet(SamplerInterfaceBlock const& sib
     return layout;
 }
 
-static void collectDescriptorsForSet(DescriptorSetBindingPoints set,
+static void collectDescriptorsForSet(filament::DescriptorSetBindingPoints set,
         const GLSLPostProcessor::Config& config, DescriptorSetInfo& descriptors) {
     const MaterialInfo& material = *config.materialInfo;
 
-    // get the descriptor set layout for the given pinding point
-    DescriptorSetLayout const descriptorSetLayout = [&] {
+    DescriptorSetLayout const info = [&]() {
         switch (set) {
             case DescriptorSetBindingPoints::PER_VIEW: {
-                return descriptor_sets::getPerViewDescriptorSetLayoutWithVariant(
-                        config.variant, config.domain, config.variantFilter,
+                if (filament::Variant::isValidDepthVariant(config.variant)) {
+                    return descriptor_sets::getDepthVariantLayout();
+                }
+                if (filament::Variant::isSSRVariant(config.variant)) {
+                    return descriptor_sets::getSsrVariantLayout();
+                }
+                return descriptor_sets::getPerViewDescriptorSetLayout(config.domain,
+                        config.variantFilter,
                         material.isLit || material.hasShadowMultiplier,
                         material.reflectionMode,
                         material.refractionMode);
@@ -179,8 +179,7 @@ static void collectDescriptorsForSet(DescriptorSetBindingPoints set,
         }
     }();
 
-    // get the sampler list for this binding point
-    auto samplerList = [&] {
+    auto samplerList = [&]() {
         switch (set) {
             case DescriptorSetBindingPoints::PER_VIEW:
                 return SibGenerator::getPerViewSib(config.variant).getSamplerInfoList();
@@ -193,34 +192,42 @@ static void collectDescriptorsForSet(DescriptorSetBindingPoints set,
         }
     }();
 
-    // filter the list with the descriptor set layout
-    auto const descriptorSetSamplerList =
-            SamplerInterfaceBlock::filterSamplerList(std::move(samplerList), descriptorSetLayout);
+    // remove all the samplers that are not included in the descriptor-set layout
+    samplerList.erase(std::remove_if(samplerList.begin(), samplerList.end(),
+                              [&info](auto const& entry) {
+                                  auto pos = std::find_if(info.bindings.begin(),
+                                          info.bindings.end(), [&entry](const auto& item) {
+                                              return item.binding == entry.binding;
+                                          });
+                                  return pos == info.bindings.end();
+                              }),
+            samplerList.end());
 
-    // helper to get the name of a descriptor for this set, given a binding.
-    auto getDescriptorName = [set, &descriptorSetSamplerList](descriptor_binding_t binding) {
+    auto getDescriptorName = [&](DescriptorSetBindingPoints set, descriptor_binding_t binding) {
         if (set == DescriptorSetBindingPoints::PER_MATERIAL) {
-            auto pos = std::find_if(descriptorSetSamplerList.begin(), descriptorSetSamplerList.end(),
+            auto pos = std::find_if(samplerList.begin(), samplerList.end(),
                     [&](const auto& entry) { return entry.binding == binding; });
-            if (pos == descriptorSetSamplerList.end()) {
+            if (pos == samplerList.end()) {
                 return descriptor_sets::getDescriptorName(set, binding);
             }
-            return pos->uniformName;
+            SamplerInterfaceBlock::SamplerInfo& sampler = *pos;
+            return sampler.uniformName;
         }
         return descriptor_sets::getDescriptorName(set, binding);
     };
 
-    for (auto descriptor : descriptorSetLayout.bindings) {
-        descriptor_binding_t binding = descriptor.binding;
-        auto name = getDescriptorName(binding);
-        if (descriptor.type == DescriptorType::SAMPLER ||
-            descriptor.type == DescriptorType::SAMPLER_EXTERNAL) {
-            auto pos = std::find_if(descriptorSetSamplerList.begin(), descriptorSetSamplerList.end(),
+    for (size_t i = 0; i < info.bindings.size(); i++) {
+        backend::descriptor_binding_t binding = info.bindings[i].binding;
+        auto name = getDescriptorName(set, binding);
+        if (info.bindings[i].type == DescriptorType::SAMPLER ||
+            info.bindings[i].type == DescriptorType::SAMPLER_EXTERNAL) {
+            auto pos = std::find_if(samplerList.begin(), samplerList.end(),
                     [&](const auto& entry) { return entry.binding == binding; });
-            assert_invariant(pos != descriptorSetSamplerList.end());
-            descriptors.emplace_back(name, descriptor, *pos);
+            assert_invariant(pos != samplerList.end());
+            SamplerInterfaceBlock::SamplerInfo& sampler = *pos;
+            descriptors.emplace_back(name, info.bindings[i], sampler);
         } else {
-            descriptors.emplace_back(name, descriptor, std::nullopt);
+            descriptors.emplace_back(name, info.bindings[i], std::nullopt);
         }
     }
 
@@ -358,7 +365,7 @@ static std::string stringifySpvOptimizerMessage(spv_message_level_t level, const
 }
 
 void GLSLPostProcessor::spirvToMsl(const SpirvBlob* spirv, std::string* outMsl,
-        ShaderStage stage, ShaderModel shaderModel,
+        filament::backend::ShaderStage stage, filament::backend::ShaderModel shaderModel,
         bool useFramebufferFetch, const DescriptorSets& descriptorSets,
         const ShaderMinifier* minifier) {
     using namespace msl;
@@ -667,7 +674,7 @@ bool GLSLPostProcessor::process(const std::string& inputShader, Config const& co
         //        SpvRules should be enough.
         //        I think this could cause the compilation to fail on gl_VertexID.
         using Type = std::underlying_type_t<EShMessages>;
-        msg = EShMessages(Type(msg) | Type(EShMsgVulkanRules));
+        msg = EShMessages(Type(msg) | Type(EShMessages::EShMsgVulkanRules));
     }
 
     bool const ok = tShader.parse(&DefaultTBuiltInResource, internalConfig.langVersion, false, msg);
@@ -677,7 +684,7 @@ bool GLSLPostProcessor::process(const std::string& inputShader, Config const& co
     }
 
     // add texture lod bias
-    if (config.shaderType == ShaderStage::FRAGMENT &&
+    if (config.shaderType == backend::ShaderStage::FRAGMENT &&
         config.domain == MaterialDomain::SURFACE) {
         GLSLTools::textureLodBias(tShader);
     }
@@ -753,8 +760,8 @@ bool GLSLPostProcessor::process(const std::string& inputShader, Config const& co
     return true;
 }
 
-bool GLSLPostProcessor::preprocessOptimization(TShader& tShader,
-        Config const& config, InternalConfig& internalConfig) const {
+bool GLSLPostProcessor::preprocessOptimization(glslang::TShader& tShader,
+        GLSLPostProcessor::Config const& config, InternalConfig& internalConfig) const {
     using TargetApi = MaterialBuilder::TargetApi;
     assert_invariant(bool(internalConfig.spirvOutput) == (config.targetApi != TargetApi::OPENGL));
 
@@ -825,7 +832,7 @@ bool GLSLPostProcessor::preprocessOptimization(TShader& tShader,
 }
 
 bool GLSLPostProcessor::fullOptimization(const TShader& tShader,
-        Config const& config, InternalConfig& internalConfig) const {
+        GLSLPostProcessor::Config const& config, InternalConfig& internalConfig) const {
     SpirvBlob spirv;
 
     bool const optimizeForSize = mOptimization == MaterialBuilderBase::Optimization::SIZE;
@@ -921,7 +928,7 @@ bool GLSLPostProcessor::fullOptimization(const TShader& tShader,
 #else
         try {
             *internalConfig.glslOutput = glslCompiler.compile();
-        } catch (CompilerError e) {
+        } catch (spirv_cross::CompilerError e) {
             slog.e << "ERROR: " << e.what() << io::endl;
             return false;
         }
@@ -941,8 +948,8 @@ bool GLSLPostProcessor::fullOptimization(const TShader& tShader,
     return true;
 }
 
-std::shared_ptr<Optimizer> GLSLPostProcessor::createEmptyOptimizer() {
-    auto optimizer = std::make_shared<Optimizer>(SPV_ENV_UNIVERSAL_1_3);
+std::shared_ptr<spvtools::Optimizer> GLSLPostProcessor::createEmptyOptimizer() {
+    auto optimizer = std::make_shared<spvtools::Optimizer>(SPV_ENV_UNIVERSAL_1_3);
     optimizer->SetMessageConsumer([](spv_message_level_t level,
             const char* source, const spv_position_t& position, const char* message) {
         if (!filterSpvOptimizerMessage(level)) {
@@ -954,7 +961,7 @@ std::shared_ptr<Optimizer> GLSLPostProcessor::createEmptyOptimizer() {
     return optimizer;
 }
 
-std::shared_ptr<Optimizer> GLSLPostProcessor::createOptimizer(
+std::shared_ptr<spvtools::Optimizer> GLSLPostProcessor::createOptimizer(
         MaterialBuilder::Optimization optimization, Config const& config) {
     auto optimizer = createEmptyOptimizer();
 
@@ -993,7 +1000,7 @@ void GLSLPostProcessor::optimizeSpirv(OptimizerPtr optimizer, SpirvBlob& spirv)
 }
 
 void GLSLPostProcessor::fixupClipDistance(
-        SpirvBlob& spirv, Config const& config) const {
+        SpirvBlob& spirv, GLSLPostProcessor::Config const& config) const {
     if (!config.usesClipDistance) {
         return;
     }
@@ -1033,7 +1040,7 @@ void GLSLPostProcessor::fixupClipDistance(
 
 
 void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config const& config) {
-    auto RegisterPass = [&](Optimizer::PassToken&& pass,
+    auto RegisterPass = [&](spvtools::Optimizer::PassToken&& pass,
             MaterialBuilder::TargetApi apiFilter = MaterialBuilder::TargetApi::ALL) {
         if (!(config.targetApi & apiFilter)) {
             return;
@@ -1078,7 +1085,7 @@ void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config c
 }
 
 void GLSLPostProcessor::registerSizePasses(Optimizer& optimizer, Config const& config) {
-    auto RegisterPass = [&](Optimizer::PassToken&& pass,
+    auto RegisterPass = [&](spvtools::Optimizer::PassToken&& pass,
             MaterialBuilder::TargetApi apiFilter = MaterialBuilder::TargetApi::ALL) {
         if (!(config.targetApi & apiFilter)) {
             return;

libs/filamat/src/MaterialVariants.cpp

@@ -16,6 +16,8 @@
 
 #include "MaterialVariants.h"
 
+#include "shaders/ShaderGenerator.h"
+
 #include <private/filament/EngineEnums.h>
 #include <private/filament/Variant.h>
 
@@ -23,8 +25,16 @@
 
 #include <filament/MaterialEnums.h>
 
+#include <utils/compiler.h>
+#include <utils/Panic.h>
+#include <utils/Log.h>
+
+#include <algorithm>
 #include <vector>
 
+#include <stddef.h>
+#include <stdint.h>
+
 namespace filamat {
 
 std::vector<Variant> determineSurfaceVariants(
@@ -52,6 +62,58 @@ std::vector<Variant> determineSurfaceVariants(
         if (fragmentVariant == variant) {
             variants.emplace_back(variant, filament::backend::ShaderStage::FRAGMENT);
         }
+
+        // Here we make sure that the combination of vertex and fragment variants have compatible
+        // PER_VIEW descriptor-set layouts. This could actually be a static/compile-time check
+        // because it is entirely decided in DescriptorSets.cpp. Unfortunately it's not possible
+        // to write this entirely as a constexpr.
+
+        if (UTILS_UNLIKELY(vertexVariant != fragmentVariant)) {
+            // fragment and vertex variants are different, we need to check the layouts are
+            // compatible.
+            using filament::ReflectionMode;
+            using filament::RefractionMode;
+            using filament::backend::ShaderStage;
+
+            // And we need to do that for all configurations of the "PER_VIEW" descriptor set
+            // layouts (there are eight).
+            // See ShaderGenerator::getPerViewDescriptorSetLayoutWithVariant.
+            for (auto reflection: {
+                    ReflectionMode::SCREEN_SPACE,
+                    ReflectionMode::DEFAULT }) {
+                for (auto refraction: {
+                        RefractionMode::SCREEN_SPACE,
+                        RefractionMode::CUBEMAP,
+                        RefractionMode::NONE }) {
+                    auto const vdsl = ShaderGenerator::getPerViewDescriptorSetLayoutWithVariant(
+                            vertexVariant, userVariantFilter, isLit || shadowMultiplier,
+                            reflection, refraction);
+                    auto const fdsl = ShaderGenerator::getPerViewDescriptorSetLayoutWithVariant(
+                            fragmentVariant, userVariantFilter, isLit || shadowMultiplier,
+                            reflection, refraction);
+                    // Check that all bindings present in the vertex shader DescriptorSetLayout
+                    // are also present in the fragment shader DescriptorSetLayout.
+                    for (auto const& r: vdsl.bindings) {
+                        if (!hasShaderType(r.stageFlags, ShaderStage::VERTEX)) {
+                            // ignore descriptors that are of the fragment stage only
+                            continue;
+                        }
+                        auto const pos = std::find_if(fdsl.bindings.begin(), fdsl.bindings.end(),
+                                [r](auto const& l) {
+                                    return l.count == r.count && l.type == r.type &&
+                                           l.binding == r.binding && l.flags == r.flags &&
+                                           l.stageFlags == r.stageFlags;
+                                });
+
+                        // A mismatch is fatal. The material is ill-formed. This typically
+                        // mean a bug / inconsistency in DescriptorsSets.cpp
+                        FILAMENT_CHECK_POSTCONDITION(pos != fdsl.bindings.end())
+                                << "Variant " << +k << " has mismatched descriptorset layouts";
+                    }
+                }
+            }
+        }
+
     }
     return variants;
 }
@@ -71,7 +133,7 @@ std::vector<Variant> determinePostProcessVariants() {
 std::vector<Variant> determineComputeVariants() {
     // TODO: should we have variants for compute shaders?
     std::vector<Variant> variants;
-    filament::Variant const variant(0);
+    filament::Variant variant(0);
     variants.emplace_back(variant, filament::backend::ShaderStage::COMPUTE);
     return variants;
 }

libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp

@@ -39,7 +39,7 @@ namespace filamat {
 
 MaterialUniformInterfaceBlockChunk::MaterialUniformInterfaceBlockChunk(
         BufferInterfaceBlock const& uib) :
-        Chunk(MaterialUib),
+        Chunk(ChunkType::MaterialUib),
         mUib(uib) {
 }
 
@@ -60,7 +60,7 @@ void MaterialUniformInterfaceBlockChunk::flatten(Flattener& f) {
 
 MaterialSamplerInterfaceBlockChunk::MaterialSamplerInterfaceBlockChunk(
         SamplerInterfaceBlock const& sib) :
-        Chunk(MaterialSib),
+        Chunk(ChunkType::MaterialSib),
         mSib(sib) {
 }
 
@@ -81,7 +81,7 @@ void MaterialSamplerInterfaceBlockChunk::flatten(Flattener& f) {
 // ------------------------------------------------------------------------------------------------
 
 MaterialSubpassInterfaceBlockChunk::MaterialSubpassInterfaceBlockChunk(SubpassInfo const& subpass) :
-        Chunk(MaterialSubpass),
+        Chunk(ChunkType::MaterialSubpass),
         mSubpass(subpass) {
 }
 
@@ -101,8 +101,8 @@ void MaterialSubpassInterfaceBlockChunk::flatten(Flattener& f) {
 // ------------------------------------------------------------------------------------------------
 
 MaterialConstantParametersChunk::MaterialConstantParametersChunk(
-        FixedCapacityVector<MaterialConstant> constants)
-    : Chunk(MaterialConstants), mConstants(std::move(constants)) {}
+        utils::FixedCapacityVector<MaterialConstant> constants)
+    : Chunk(ChunkType::MaterialConstants), mConstants(std::move(constants)) {}
 
 void MaterialConstantParametersChunk::flatten(Flattener& f) {
     f.writeUint64(mConstants.size());
@@ -115,8 +115,8 @@ void MaterialConstantParametersChunk::flatten(Flattener& f) {
 // ------------------------------------------------------------------------------------------------
 
 MaterialPushConstantParametersChunk::MaterialPushConstantParametersChunk(
-        CString const& structVarName, FixedCapacityVector<MaterialPushConstant> constants)
-    : Chunk(MaterialPushConstants),
+        CString const& structVarName, utils::FixedCapacityVector<MaterialPushConstant> constants)
+    : Chunk(ChunkType::MaterialPushConstants),
       mStructVarName(structVarName),
       mConstants(std::move(constants)) {}
 
@@ -133,7 +133,7 @@ void MaterialPushConstantParametersChunk::flatten(Flattener& f) {
 // ------------------------------------------------------------------------------------------------
 
 MaterialBindingUniformInfoChunk::MaterialBindingUniformInfoChunk(Container list) noexcept
-        : Chunk(MaterialBindingUniformInfo),
+        : Chunk(ChunkType::MaterialBindingUniformInfo),
           mBindingUniformInfo(std::move(list)) {
 }
 
@@ -155,7 +155,7 @@ void MaterialBindingUniformInfoChunk::flatten(Flattener& f) {
 // ------------------------------------------------------------------------------------------------
 
 MaterialAttributesInfoChunk::MaterialAttributesInfoChunk(Container list) noexcept
-        : Chunk(MaterialAttributeInfo),
+        : Chunk(ChunkType::MaterialAttributeInfo),
           mAttributeInfo(std::move(list))
 {
 }
@@ -170,9 +170,11 @@ void MaterialAttributesInfoChunk::flatten(Flattener& f) {
 
 // ------------------------------------------------------------------------------------------------
 
-MaterialDescriptorBindingsChuck::MaterialDescriptorBindingsChuck(Container const& sib) noexcept
-        : Chunk(MaterialDescriptorBindingsInfo),
-          mSamplerInterfaceBlock(sib) {
+MaterialDescriptorBindingsChuck::MaterialDescriptorBindingsChuck(Container const& sib,
+        backend::DescriptorSetLayout const& perViewLayout) noexcept
+        : Chunk(ChunkType::MaterialDescriptorBindingsInfo),
+          mSamplerInterfaceBlock(sib),
+          mPerViewLayout(perViewLayout) {
 }
 
 void MaterialDescriptorBindingsChuck::flatten(Flattener& f) {
@@ -181,6 +183,13 @@ void MaterialDescriptorBindingsChuck::flatten(Flattener& f) {
 
     using namespace backend;
 
+
+    // number of descriptor-sets
+    f.writeUint8(3);
+
+    // set
+    f.writeUint8(+DescriptorSetBindingPoints::PER_MATERIAL);
+
     // samplers + 1 descriptor for the UBO
     f.writeUint8(mSamplerInterfaceBlock.getSize() + 1);
 
@@ -201,13 +210,37 @@ void MaterialDescriptorBindingsChuck::flatten(Flattener& f) {
         }
         f.writeUint8(entry.binding);
     }
+
+    // set
+    f.writeUint8(+DescriptorSetBindingPoints::PER_RENDERABLE);
+    f.writeUint8(descriptor_sets::getPerRenderableLayout().bindings.size());
+    for (auto const& entry: descriptor_sets::getPerRenderableLayout().bindings) {
+        auto const& name = descriptor_sets::getDescriptorName(
+                DescriptorSetBindingPoints::PER_RENDERABLE, entry.binding);
+        f.writeString({ name.data(), name.size() });
+        f.writeUint8(uint8_t(entry.type));
+        f.writeUint8(entry.binding);
+    }
+
+    // set
+    f.writeUint8(+DescriptorSetBindingPoints::PER_VIEW);
+    f.writeUint8(mPerViewLayout.bindings.size());
+    for (auto const& entry: mPerViewLayout.bindings) {
+        auto const& name = descriptor_sets::getDescriptorName(
+                DescriptorSetBindingPoints::PER_VIEW, entry.binding);
+        f.writeString({ name.data(), name.size() });
+        f.writeUint8(uint8_t(entry.type));
+        f.writeUint8(entry.binding);
+    }
 }
 
 // ------------------------------------------------------------------------------------------------
 
-MaterialDescriptorSetLayoutChunk::MaterialDescriptorSetLayoutChunk(Container const& sib) noexcept
-        : Chunk(MaterialDescriptorSetLayoutInfo),
-          mSamplerInterfaceBlock(sib) {
+MaterialDescriptorSetLayoutChunk::MaterialDescriptorSetLayoutChunk(Container const& sib,
+        backend::DescriptorSetLayout const& perViewLayout) noexcept
+        : Chunk(ChunkType::MaterialDescriptorSetLayoutInfo),
+          mSamplerInterfaceBlock(sib),
+          mPerViewLayout(perViewLayout) {
 }
 
 void MaterialDescriptorSetLayoutChunk::flatten(Flattener& f) {
@@ -238,6 +271,18 @@ void MaterialDescriptorSetLayoutChunk::flatten(Flattener& f) {
         f.writeUint8(uint8_t(DescriptorFlags::NONE));
         f.writeUint16(0);
     }
+
+    // samplers + 1 descriptor for the UBO
+    f.writeUint8(mPerViewLayout.bindings.size());
+
+    // all the material's sampler descriptors
+    for (auto const& entry: mPerViewLayout.bindings) {
+        f.writeUint8(uint8_t(entry.type));
+        f.writeUint8(uint8_t(entry.stageFlags));
+        f.writeUint8(entry.binding);
+        f.writeUint8(uint8_t(entry.flags));
+        f.writeUint16(entry.count);
+    }
 }
 
 } // namespace filamat

libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h

@@ -19,12 +19,16 @@
 
 #include "Chunk.h"
 
+#include <private/filament/EngineEnums.h>
+
+#include <backend/DriverEnums.h>
 #include <backend/Program.h>
 
 #include <utils/CString.h>
 #include <utils/FixedCapacityVector.h>
 
 #include <tuple>
+#include <utility>
 
 #include <stdint.h>
 
@@ -41,10 +45,10 @@ namespace filamat {
 class MaterialUniformInterfaceBlockChunk final : public Chunk {
 public:
     explicit MaterialUniformInterfaceBlockChunk(filament::BufferInterfaceBlock const& uib);
-    ~MaterialUniformInterfaceBlockChunk() override = default;
+    ~MaterialUniformInterfaceBlockChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
     filament::BufferInterfaceBlock const& mUib;
 };
@@ -54,10 +58,10 @@ private:
 class MaterialSamplerInterfaceBlockChunk final : public Chunk {
 public:
     explicit MaterialSamplerInterfaceBlockChunk(filament::SamplerInterfaceBlock const& sib);
-    ~MaterialSamplerInterfaceBlockChunk() override = default;
+    ~MaterialSamplerInterfaceBlockChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
     filament::SamplerInterfaceBlock const& mSib;
 };
@@ -67,10 +71,10 @@ private:
 class MaterialSubpassInterfaceBlockChunk final : public Chunk {
 public:
     explicit MaterialSubpassInterfaceBlockChunk(filament::SubpassInfo const& subpass);
-    ~MaterialSubpassInterfaceBlockChunk() override = default;
+    ~MaterialSubpassInterfaceBlockChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
     filament::SubpassInfo const& mSubpass;
 };
@@ -80,41 +84,41 @@ private:
 class MaterialConstantParametersChunk final : public Chunk {
 public:
     explicit MaterialConstantParametersChunk(
-            FixedCapacityVector<filament::MaterialConstant> constants);
-    ~MaterialConstantParametersChunk() override = default;
+            utils::FixedCapacityVector<filament::MaterialConstant> constants);
+    ~MaterialConstantParametersChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
-    FixedCapacityVector<filament::MaterialConstant> mConstants;
+    utils::FixedCapacityVector<filament::MaterialConstant> mConstants;
 };
 
 // ------------------------------------------------------------------------------------------------
 
 class MaterialPushConstantParametersChunk final : public Chunk {
 public:
-    explicit MaterialPushConstantParametersChunk(CString const& structVarName,
-            FixedCapacityVector<filament::MaterialPushConstant> constants);
-    ~MaterialPushConstantParametersChunk() override = default;
+    explicit MaterialPushConstantParametersChunk(utils::CString const& structVarName,
+            utils::FixedCapacityVector<filament::MaterialPushConstant> constants);
+    ~MaterialPushConstantParametersChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
-    CString mStructVarName;
-    FixedCapacityVector<filament::MaterialPushConstant> mConstants;
+    utils::CString mStructVarName;
+    utils::FixedCapacityVector<filament::MaterialPushConstant> mConstants;
 };
 
 // ------------------------------------------------------------------------------------------------
 
 class MaterialBindingUniformInfoChunk final : public Chunk {
     using Container = FixedCapacityVector<std::tuple<
-            uint8_t, CString, filament::backend::Program::UniformInfo>>;
+            uint8_t, utils::CString, filament::backend::Program::UniformInfo>>;
 public:
     explicit MaterialBindingUniformInfoChunk(Container list) noexcept;
-    ~MaterialBindingUniformInfoChunk() override = default;
+    ~MaterialBindingUniformInfoChunk() final = default;
 
 private:
-    void flatten(Flattener &) override;
+    void flatten(Flattener &) final;
 
     Container mBindingUniformInfo;
 };
@@ -122,13 +126,13 @@ private:
 // ------------------------------------------------------------------------------------------------
 
 class MaterialAttributesInfoChunk final : public Chunk {
-    using Container = FixedCapacityVector<std::pair<CString, uint8_t>>;
+    using Container = FixedCapacityVector<std::pair<utils::CString, uint8_t>>;
 public:
     explicit MaterialAttributesInfoChunk(Container list) noexcept;
-    ~MaterialAttributesInfoChunk() override = default;
+    ~MaterialAttributesInfoChunk() final = default;
 
 private:
-    void flatten(Flattener &) override;
+    void flatten(Flattener &) final;
 
     Container mAttributeInfo;
 };
@@ -138,13 +142,15 @@ private:
 class MaterialDescriptorBindingsChuck final : public Chunk {
     using Container = filament::SamplerInterfaceBlock;
 public:
-    explicit MaterialDescriptorBindingsChuck(Container const& sib) noexcept;
-    ~MaterialDescriptorBindingsChuck() override = default;
+    explicit MaterialDescriptorBindingsChuck(Container const& sib,
+            filament::backend::DescriptorSetLayout const& perViewLayout) noexcept;
+    ~MaterialDescriptorBindingsChuck() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
     Container const& mSamplerInterfaceBlock;
+    filament::backend::DescriptorSetLayout mPerViewLayout;
 };
 
 // ------------------------------------------------------------------------------------------------
@@ -152,13 +158,15 @@ private:
 class MaterialDescriptorSetLayoutChunk final : public Chunk {
     using Container = filament::SamplerInterfaceBlock;
 public:
-    explicit MaterialDescriptorSetLayoutChunk(Container const& sib) noexcept;
-    ~MaterialDescriptorSetLayoutChunk() override = default;
+    explicit MaterialDescriptorSetLayoutChunk(Container const& sib,
+            filament::backend::DescriptorSetLayout const& perViewLayout) noexcept;
+    ~MaterialDescriptorSetLayoutChunk() final = default;
 
 private:
-    void flatten(Flattener&) override;
+    void flatten(Flattener&) final;
 
     Container const& mSamplerInterfaceBlock;
+    filament::backend::DescriptorSetLayout mPerViewLayout;
 };
 
 } // namespace filamat

libs/filamat/src/shaders/ShaderGenerator.cpp

@@ -604,13 +604,27 @@ std::string ShaderGenerator::createSurfaceFragmentProgram(ShaderModel shaderMode
 
     if (featureLevel >= FeatureLevel::FEATURE_LEVEL_1) {
         assert_invariant(mMaterialDomain == MaterialDomain::SURFACE);
+
+        auto const perViewDescriptorSetLayout = getPerViewDescriptorSetLayoutWithVariant(
+                variant, variantFilter,
+                material.isLit || material.hasShadowMultiplier,
+                material.reflectionMode, material.refractionMode);
+
         // this is the list of samplers we need to filter
-        auto const list = SamplerInterfaceBlock::filterSamplerList(
-                SibGenerator::getPerViewSib(variant).getSamplerInfoList(),
-                descriptor_sets::getPerViewDescriptorSetLayoutWithVariant(
-                        variant, mMaterialDomain, variantFilter,
-                        material.isLit || material.hasShadowMultiplier,
-                        material.reflectionMode, material.refractionMode));
+        auto list = SibGenerator::getPerViewSib(variant).getSamplerInfoList();
+
+        // remove all the samplers that are not included in the descriptor-set layout
+        list.erase(
+                std::remove_if(list.begin(), list.end(),
+                        [&perViewDescriptorSetLayout](auto const& entry) {
+                            auto pos = std::find_if(
+                                    perViewDescriptorSetLayout.bindings.begin(),
+                                    perViewDescriptorSetLayout.bindings.end(),
+                                    [&entry](const auto& item) {
+                                        return item.binding == entry.binding;
+                                    });
+                            return pos == perViewDescriptorSetLayout.bindings.end();
+                        }), list.end());
 
         cg.generateCommonSamplers(fs, DescriptorSetBindingPoints::PER_VIEW, list);
     }
@@ -827,4 +841,22 @@ bool ShaderGenerator::hasStereo(
             && featureLevel > MaterialBuilder::FeatureLevel::FEATURE_LEVEL_0;
 }
 
+backend::DescriptorSetLayout ShaderGenerator::getPerViewDescriptorSetLayoutWithVariant(
+        filament::Variant variant,
+        UserVariantFilterMask variantFilter,
+        bool isLit,
+        ReflectionMode reflectionMode,
+        RefractionMode refractionMode) {
+    if (filament::Variant::isValidDepthVariant(variant)) {
+        return descriptor_sets::getDepthVariantLayout();
+    }
+    if (filament::Variant::isSSRVariant(variant)) {
+        return descriptor_sets::getSsrVariantLayout();
+    }
+    // We need to filter out all the descriptors not included in the "resolved" layout below
+    return descriptor_sets::getPerViewDescriptorSetLayout(
+            MaterialDomain::SURFACE, variantFilter,
+            isLit, reflectionMode, refractionMode);
+}
+
 } // namespace filament

libs/filamat/src/shaders/ShaderGenerator.h

@@ -88,6 +88,13 @@ public:
             MaterialBuilder::FeatureLevel featureLevel,
             MaterialInfo const& material) noexcept;
 
+    static filament::backend::DescriptorSetLayout getPerViewDescriptorSetLayoutWithVariant(
+            filament::Variant variant,
+            filament::UserVariantFilterMask variantFilter,
+            bool isLit,
+            filament::ReflectionMode reflectionMode,
+            filament::RefractionMode refractionMode);
+
 private:
     static void generateVertexDomainDefines(utils::io::sstream& out,
             filament::VertexDomain domain) noexcept;

libs/gltfio/src/AssetLoader.cpp

@@ -50,6 +50,8 @@
 #include <utils/Log.h>
 #include <utils/Panic.h>
 #include <utils/NameComponentManager.h>
+
+#define SYSTRACE_TAG SYSTRACE_TAG_GLTFIO
 #include <utils/Systrace.h>
 
 #include <tsl/robin_map.h>

libs/gltfio/src/ResourceLoader.cpp

@@ -37,7 +37,10 @@
 #include <utils/compiler.h>
 #include <utils/JobSystem.h>
 #include <utils/Log.h>
+
+#define SYSTRACE_TAG SYSTRACE_TAG_GLTFIO
 #include <utils/Systrace.h>
+
 #include <utils/Path.h>
 
 #include <cgltf.h>

libs/gltfio/src/Utility.cpp

@@ -21,6 +21,8 @@
 #include "GltfEnums.h"
 
 #include <utils/Log.h>
+
+#define SYSTRACE_TAG SYSTRACE_TAG_GLTFIO
 #include <utils/Systrace.h>
 
 #define CGLTF_IMPLEMENTATION

libs/utils/include/utils/CString.h

@@ -128,7 +128,6 @@ public:
 
     CString& replace(size_type pos, size_type len, const CString& str) noexcept;
     CString& insert(size_type pos, const CString& str) noexcept { return replace(pos, 0, str); }
-    CString& append(const CString& str) noexcept { return insert(length(), str); }
 
     const_reference operator[](size_type pos) const noexcept {
         assert(pos < size());

libs/utils/include/utils/Systrace.h

@@ -17,24 +17,23 @@
 #ifndef TNT_UTILS_SYSTRACE_H
 #define TNT_UTILS_SYSTRACE_H
 
-
-#define SYSTRACE_TAG_NEVER          (0)
-#define SYSTRACE_TAG_ALWAYS         (1<<0)
-#define SYSTRACE_TAG_FILAMENT       (1<<1)  // don't change, used in makefiles
-#define SYSTRACE_TAG_JOBSYSTEM      (1<<2)
+#define SYSTRACE_TAG_DISABLED       (0)
+#define SYSTRACE_TAG_FILAMENT       (2) // don't change used in makefiles
+#define SYSTRACE_TAG_JOBSYSTEM      (3)
+#define SYSTRACE_TAG_GLTFIO         (4)
 
 /*
- * The SYSTRACE_ macros use SYSTRACE_TAG as a the TAG, which should be defined
- * before this file is included. If not, the SYSTRACE_TAG_ALWAYS tag will be used.
+ * The SYSTRACE_ macros use SYSTRACE_TAG as a category, which must be defined
+ * before this file is included.
  */
 
 #ifndef SYSTRACE_TAG
-#define SYSTRACE_TAG (SYSTRACE_TAG_ALWAYS)
+#   error SYSTRACE_TAG must be set to SYSTRACE_TAG_{DISABLED|FILAMENT|JOBSYSTEM}
 #endif
 
 // Systrace on Apple platforms is fragile and adds overhead, should only be enabled in dev builds.
 #ifndef FILAMENT_APPLE_SYSTRACE
-#define FILAMENT_APPLE_SYSTRACE 0
+#   define FILAMENT_APPLE_SYSTRACE 0
 #endif
 
 #if defined(__ANDROID__)
@@ -44,7 +43,6 @@
 #else
 
 #define SYSTRACE_ENABLE()
-#define SYSTRACE_DISABLE()
 #define SYSTRACE_CONTEXT()
 #define SYSTRACE_NAME(name)
 #define SYSTRACE_FRAME_ID(frame)
@@ -56,6 +54,6 @@
 #define SYSTRACE_VALUE32(name, val)
 #define SYSTRACE_VALUE64(name, val)
 
-#endif // ANDROID
+#endif
 
 #endif // TNT_UTILS_SYSTRACE_H

libs/utils/include/utils/android/Systrace.h

@@ -17,226 +17,75 @@
 #ifndef TNT_UTILS_ANDROID_SYSTRACE_H
 #define TNT_UTILS_ANDROID_SYSTRACE_H
 
-#include <atomic>
+#include <perfetto/perfetto.h>
 
 #include <stdint.h>
-#include <stdio.h>
-#include <unistd.h>
 
-#include <utils/compiler.h>
+PERFETTO_DEFINE_CATEGORIES_IN_NAMESPACE(systrace,
+        perfetto::Category("filament"),
+        perfetto::Category("jobsystem"),
+        perfetto::Category("gltfio"));
 
-// enable tracing
-#define SYSTRACE_ENABLE() ::utils::details::Systrace::enable(SYSTRACE_TAG)
+PERFETTO_USE_CATEGORIES_FROM_NAMESPACE(systrace);
 
-// disable tracing
-#define SYSTRACE_DISABLE() ::utils::details::Systrace::disable(SYSTRACE_TAG)
+#if SYSTRACE_TAG == SYSTRACE_TAG_FILAMENT
+#   define UTILS_PERFETTO_CATEGORY "filament"
+#elif SYSTRACE_TAG == SYSTRACE_TAG_JOBSYSTEM
+#   define UTILS_PERFETTO_CATEGORY "jobsystem"
+#elif SYSTRACE_TAG == SYSTRACE_TAG_GLTFIO
+#   define UTILS_PERFETTO_CATEGORY "gltfio"
+#endif
 
+#if SYSTRACE_TAG == SYSTRACE_TAG_DISABLED
 
-/**
- * Creates a Systrace context in the current scope. needed for calling all other systrace
- * commands below.
- */
-#define SYSTRACE_CONTEXT() ::utils::details::Systrace ___trctx(SYSTRACE_TAG)
+#define SYSTRACE_ENABLE()
+#define SYSTRACE_CONTEXT()
+#define SYSTRACE_NAME(name)
+#define SYSTRACE_FRAME_ID(frame)
+#define SYSTRACE_NAME_BEGIN(name)
+#define SYSTRACE_NAME_END()
+#define SYSTRACE_CALL()
+#define SYSTRACE_ASYNC_BEGIN(name, cookie)
+#define SYSTRACE_ASYNC_END(name, cookie)
+#define SYSTRACE_VALUE32(name, val)
+#define SYSTRACE_VALUE64(name, val)
 
+#else
 
-// SYSTRACE_NAME traces the beginning and end of the current scope.  To trace
-// the correct start and end times this macro should be declared first in the
-// scope body.
-// It also automatically creates a Systrace context
-#define SYSTRACE_NAME(name) ::utils::details::ScopedTrace ___tracer(SYSTRACE_TAG, name)
+#define SYSTRACE_ENABLE()
+#define SYSTRACE_CONTEXT()
 
-// Denotes that a new frame has started processing.
-#define SYSTRACE_FRAME_ID(frame) \
-    { /* scope for frame id trace */ \
-        char buf[64]; \
-        snprintf(buf, 64, "frame %u", frame); \
-        SYSTRACE_NAME(buf); \
-    }
+#define SYSTRACE_CALL() \
+    auto constexpr FILAMENT_SYSTRACE_FUNCTION = perfetto::StaticString(__FUNCTION__); \
+    TRACE_EVENT(UTILS_PERFETTO_CATEGORY, FILAMENT_SYSTRACE_FUNCTION)
 
-// SYSTRACE_CALL is an SYSTRACE_NAME that uses the current function name.
-#define SYSTRACE_CALL() SYSTRACE_NAME(__FUNCTION__)
+#define SYSTRACE_NAME(name) TRACE_EVENT(UTILS_PERFETTO_CATEGORY, nullptr,   \
+        [&](perfetto::EventContext ctx) {                                   \
+            ctx.event()->set_name(name);                                    \
+        })
 
-#define SYSTRACE_NAME_BEGIN(name) \
-        ___trctx.traceBegin(SYSTRACE_TAG, name)
+#define SYSTRACE_NAME_BEGIN(name) TRACE_EVENT_BEGIN(UTILS_PERFETTO_CATEGORY, nullptr,   \
+        [&](perfetto::EventContext ctx) {                                               \
+            ctx.event()->set_name(name);                                                \
+        })
 
-#define SYSTRACE_NAME_END() \
-        ___trctx.traceEnd(SYSTRACE_TAG)
+#define SYSTRACE_NAME_END() TRACE_EVENT_END(UTILS_PERFETTO_CATEGORY)
 
-
-/**
- * Trace the beginning of an asynchronous event. Unlike ATRACE_BEGIN/ATRACE_END
- * contexts, asynchronous events do not need to be nested. The name describes
- * the event, and the cookie provides a unique identifier for distinguishing
- * simultaneous events. The name and cookie used to begin an event must be
- * used to end it.
- */
 #define SYSTRACE_ASYNC_BEGIN(name, cookie) \
-        ___trctx.asyncBegin(SYSTRACE_TAG, name, cookie)
+        TRACE_EVENT_BEGIN(UTILS_PERFETTO_CATEGORY, name, perfetto::Track(cookie))
 
-/**
- * Trace the end of an asynchronous event.
- * This should have a corresponding SYSTRACE_ASYNC_BEGIN.
- */
 #define SYSTRACE_ASYNC_END(name, cookie) \
-        ___trctx.asyncEnd(SYSTRACE_TAG, name, cookie)
+        TRACE_EVENT_END(UTILS_PERFETTO_CATEGORY, perfetto::Track(cookie))
+
+#define SYSTRACE_FRAME_ID(frame) \
+    TRACE_EVENT_INSTANT(UTILS_PERFETTO_CATEGORY, "frame", "id", frame)
 
-/**
- * Traces an integer counter value.  name is used to identify the counter.
- * This can be used to track how a value changes over time.
- */
 #define SYSTRACE_VALUE32(name, val) \
-        ___trctx.value(SYSTRACE_TAG, name, int32_t(val))
+    TRACE_COUNTER(UTILS_PERFETTO_CATEGORY, name, val)
 
 #define SYSTRACE_VALUE64(name, val) \
-        ___trctx.value(SYSTRACE_TAG, name, int64_t(val))
+    TRACE_COUNTER(UTILS_PERFETTO_CATEGORY, name, val)
 
-// ------------------------------------------------------------------------------------------------
-// No user serviceable code below...
-// ------------------------------------------------------------------------------------------------
-
-namespace utils {
-namespace details {
-
-class UTILS_PUBLIC Systrace {
-   public:
-
-    enum tags {
-        NEVER       = SYSTRACE_TAG_NEVER,
-        ALWAYS      = SYSTRACE_TAG_ALWAYS,
-        FILAMENT    = SYSTRACE_TAG_FILAMENT,
-        JOBSYSTEM   = SYSTRACE_TAG_JOBSYSTEM
-        // we could define more TAGS here, as we need them.
-    };
-
-    explicit Systrace(uint32_t tag) noexcept {
-        if (tag) init(tag);
-    }
-
-    static void enable(uint32_t tags) noexcept;
-    static void disable(uint32_t tags) noexcept;
-
-
-    inline void traceBegin(uint32_t tag, const char* name) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            beginSection(this, name);
-        }
-    }
-
-    inline void traceEnd(uint32_t tag) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            endSection(this);
-        }
-    }
-
-    inline void asyncBegin(uint32_t tag, const char* name, int32_t cookie) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            beginAsyncSection(this, name, cookie);
-        }
-    }
-
-    inline void asyncEnd(uint32_t tag, const char* name, int32_t cookie) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            endAsyncSection(this, name, cookie);
-        }
-    }
-
-    inline void value(uint32_t tag, const char* name, int32_t value) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            setCounter(this, name, value);
-        }
-    }
-
-    inline void value(uint32_t tag, const char* name, int64_t value) noexcept {
-        if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            setCounter(this, name, value);
-        }
-    }
-
-   private:
-    friend class ScopedTrace;
-
-    // whether tracing is supported at all by the platform
-
-    using ATrace_isEnabled_t          = bool (*)();
-    using ATrace_beginSection_t       = void (*)(const char* sectionName);
-    using ATrace_endSection_t         = void (*)();
-    using ATrace_beginAsyncSection_t  = void (*)(const char* sectionName, int32_t cookie);
-    using ATrace_endAsyncSection_t    = void (*)(const char* sectionName, int32_t cookie);
-    using ATrace_setCounter_t         = void (*)(const char* counterName, int64_t counterValue);
-
-    struct GlobalState {
-        bool isTracingAvailable;
-        std::atomic<uint32_t> isTracingEnabled;
-        int markerFd;
-
-        ATrace_isEnabled_t ATrace_isEnabled;
-        ATrace_beginSection_t ATrace_beginSection;
-        ATrace_endSection_t ATrace_endSection;
-        ATrace_beginAsyncSection_t ATrace_beginAsyncSection;
-        ATrace_endAsyncSection_t ATrace_endAsyncSection;
-        ATrace_setCounter_t ATrace_setCounter;
-
-        void (*beginSection)(Systrace* that, const char* name);
-        void (*endSection)(Systrace* that);
-        void (*beginAsyncSection)(Systrace* that, const char* name, int32_t cookie);
-        void (*endAsyncSection)(Systrace* that, const char* name, int32_t cookie);
-        void (*setCounter)(Systrace* that, const char* name, int64_t value);
-    };
-
-    static GlobalState sGlobalState;
-
-
-    // per-instance versions for better performance
-    ATrace_isEnabled_t ATrace_isEnabled;
-    ATrace_beginSection_t ATrace_beginSection;
-    ATrace_endSection_t ATrace_endSection;
-    ATrace_beginAsyncSection_t ATrace_beginAsyncSection;
-    ATrace_endAsyncSection_t ATrace_endAsyncSection;
-    ATrace_setCounter_t ATrace_setCounter;
-
-    void (*beginSection)(Systrace* that, const char* name);
-    void (*endSection)(Systrace* that);
-    void (*beginAsyncSection)(Systrace* that, const char* name, int32_t cookie);
-    void (*endAsyncSection)(Systrace* that, const char* name, int32_t cookie);
-    void (*setCounter)(Systrace* that, const char* name, int64_t value);
-
-    void init(uint32_t tag) noexcept;
-
-    // cached values for faster access, no need to be initialized
-    bool mIsTracingEnabled;
-    int mMarkerFd = -1;
-    pid_t mPid;
-
-    static void setup() noexcept;
-    static void init_once() noexcept;
-    static bool isTracingEnabled(uint32_t tag) noexcept;
-
-    static void begin_body(int fd, int pid, const char* name) noexcept;
-    static void end_body(int fd, int pid) noexcept;
-    static void async_begin_body(int fd, int pid, const char* name, int32_t cookie) noexcept;
-    static void async_end_body(int fd, int pid, const char* name, int32_t cookie) noexcept;
-    static void int64_body(int fd, int pid, const char* name, int64_t value) noexcept;
-};
-
-// ------------------------------------------------------------------------------------------------
-
-class UTILS_PUBLIC ScopedTrace {
-public:
-    // we don't inline this because it's relatively heavy due to a global check
-    ScopedTrace(uint32_t tag, const char* name) noexcept: mTrace(tag), mTag(tag) {
-        mTrace.traceBegin(tag, name);
-    }
-
-    inline ~ScopedTrace() noexcept {
-        mTrace.traceEnd(mTag);
-    }
-
-private:
-    Systrace mTrace;
-    const uint32_t mTag;
-};
-
-} // namespace details
-} // namespace utils
+#endif // SYSTRACE_TAG == SYSTRACE_TAG_DISABLED
 
 #endif // TNT_UTILS_ANDROID_SYSTRACE_H

libs/utils/include/utils/darwin/Systrace.h

@@ -29,13 +29,25 @@
 #include <utils/compiler.h>
 #include <stack>
 
+#if SYSTRACE_TAG == SYSTRACE_TAG_DISABLED
+
+#define SYSTRACE_ENABLE()
+#define SYSTRACE_CONTEXT()
+#define SYSTRACE_NAME(name)
+#define SYSTRACE_FRAME_ID(frame)
+#define SYSTRACE_NAME_BEGIN(name)
+#define SYSTRACE_NAME_END()
+#define SYSTRACE_CALL()
+#define SYSTRACE_ASYNC_BEGIN(name, cookie)
+#define SYSTRACE_ASYNC_END(name, cookie)
+#define SYSTRACE_VALUE32(name, val)
+#define SYSTRACE_VALUE64(name, val)
+
+#else
+
 // enable tracing
 #define SYSTRACE_ENABLE() ::utils::details::Systrace::enable(SYSTRACE_TAG)
 
-// disable tracing
-#define SYSTRACE_DISABLE() ::utils::details::Systrace::disable(SYSTRACE_TAG)
-
-
 /**
  * Creates a Systrace context in the current scope. needed for calling all other systrace
  * commands below.
@@ -93,6 +105,8 @@ extern thread_local std::stack<const char*> ___tracerSections;
 #define SYSTRACE_VALUE64(name, val) \
         ___tracer.value(SYSTRACE_TAG, name, int64_t(val))
 
+#endif // SYSTRACE_TAG == SYSTRACE_TAG_DISABLED
+
 // ------------------------------------------------------------------------------------------------
 // No user serviceable code below...
 // ------------------------------------------------------------------------------------------------
@@ -118,50 +132,40 @@ namespace utils {
 namespace details {
 
 class Systrace {
-   public:
-
-    enum tags {
-        NEVER       = SYSTRACE_TAG_NEVER,
-        ALWAYS      = SYSTRACE_TAG_ALWAYS,
-        FILAMENT    = SYSTRACE_TAG_FILAMENT,
-        JOBSYSTEM   = SYSTRACE_TAG_JOBSYSTEM
-        // we could define more TAGS here, as we need them.
-    };
-
+public:
     explicit Systrace(uint32_t tag) noexcept {
         if (tag) init(tag);
     }
 
-    static void enable(uint32_t tags) noexcept;
-    static void disable(uint32_t tags) noexcept;
+    static void enable(uint32_t tag) noexcept;
 
-    inline void traceBegin(uint32_t tag, const char* name) noexcept {
+    void traceBegin(uint32_t tag, const char* name) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             APPLE_SIGNPOST_EMIT(sGlobalState.systraceLog, OS_SIGNPOST_INTERVAL_BEGIN,
                     OS_SIGNPOST_ID_EXCLUSIVE, name, name)
         }
     }
 
-    inline void traceEnd(uint32_t tag, const char* name) noexcept {
+    void traceEnd(uint32_t tag, const char* name) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             APPLE_SIGNPOST_EMIT(sGlobalState.systraceLog, OS_SIGNPOST_INTERVAL_END,
                     OS_SIGNPOST_ID_EXCLUSIVE, name, "")
         }
     }
 
-    inline void asyncBegin(uint32_t tag, const char* name, int32_t cookie) noexcept {
+    void asyncBegin(uint32_t tag, const char* name, int32_t cookie) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             // TODO
         }
     }
 
-    inline void asyncEnd(uint32_t tag, const char* name, int32_t cookie) noexcept {
+    void asyncEnd(uint32_t tag, const char* name, int32_t cookie) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             // TODO
         }
     }
 
-    inline void value(uint32_t tag, const char* name, int32_t value) noexcept {
+    void value(uint32_t tag, const char* name, int32_t value) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             char buf[64];
             snprintf(buf, 64, "%s - %d", name, value);
@@ -170,7 +174,7 @@ class Systrace {
         }
     }
 
-    inline void value(uint32_t tag, const char* name, int64_t value) noexcept {
+    void value(uint32_t tag, const char* name, int64_t value) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
             char buf[64];
             snprintf(buf, 64, "%s - %lld", name, value);
@@ -179,16 +183,16 @@ class Systrace {
         }
     }
 
-    inline void frameId(uint32_t tag, uint32_t frame) noexcept {
+    void frameId(uint32_t tag, uint32_t frame) noexcept {
         if (tag && UTILS_UNLIKELY(mIsTracingEnabled)) {
-            char buf[64]; \
-            snprintf(buf, 64, "frame %u", frame); \
+            char buf[64];
+            snprintf(buf, 64, "frame %u", frame);
             APPLE_SIGNPOST_EMIT(sGlobalState.frameIdLog, OS_SIGNPOST_EVENT,
                     OS_SIGNPOST_ID_EXCLUSIVE, "frame", buf)
         }
     }
 
-   private:
+private:
     friend class ScopedTrace;
 
     struct GlobalState {
@@ -213,25 +217,25 @@ class Systrace {
 // ------------------------------------------------------------------------------------------------
 
 class ScopedTrace {
-   public:
+public:
     // we don't inline this because it's relatively heavy due to a global check
     ScopedTrace(uint32_t tag, const char* name) noexcept : mTrace(tag), mName(name), mTag(tag) {
         mTrace.traceBegin(tag, name);
     }
 
-    inline ~ScopedTrace() noexcept {
+    ~ScopedTrace() noexcept {
         mTrace.traceEnd(mTag, mName);
     }
 
-    inline void value(uint32_t tag, const char* name, int32_t v) noexcept {
+    void value(uint32_t tag, const char* name, int32_t v) noexcept {
         mTrace.value(tag, name, v);
     }
 
-    inline void value(uint32_t tag, const char* name, int64_t v) noexcept {
+    void value(uint32_t tag, const char* name, int64_t v) noexcept {
         mTrace.value(tag, name, v);
     }
 
-   private:
+private:
     Systrace mTrace;
     const char* mName;
     const uint32_t mTag;

libs/utils/src/JobSystem.cpp

@@ -15,10 +15,8 @@
  */
 
 // Note: The overhead of SYSTRACE_TAG_JOBSYSTEM is not negligible especially with parallel_for().
-#ifndef SYSTRACE_TAG
+#define SYSTRACE_TAG SYSTRACE_TAG_DISABLED
 //#define SYSTRACE_TAG SYSTRACE_TAG_JOBSYSTEM
-#define SYSTRACE_TAG SYSTRACE_TAG_NEVER
-#endif
 
 // when SYSTRACE_TAG_JOBSYSTEM is used, enables even heavier systraces
 #define HEAVY_SYSTRACE  0

libs/utils/src/android/Systrace.cpp

@@ -14,206 +14,26 @@
  * limitations under the License.
  */
 
-#include <utils/Systrace.h>
-#include <utils/Log.h>
+#include <utils/compiler.h>
+#include <utils/android/Systrace.h>
 
-#include <cinttypes>
+#include <perfetto/perfetto.h>
 
-#include <string.h>
+PERFETTO_TRACK_EVENT_STATIC_STORAGE_IN_NAMESPACE(systrace);
 
-#include <errno.h>
-#include <fcntl.h>
-#include <pthread.h>
-#include <dlfcn.h>
+namespace {
 
-namespace utils {
-namespace details {
-
-static pthread_once_t atrace_once_control = PTHREAD_ONCE_INIT;
-
-template <typename T>
-static void loadSymbol(T*& pfn, const char *symbol) noexcept {
-    pfn = (T*)dlsym(RTLD_DEFAULT, symbol);
-}
-
-Systrace::GlobalState Systrace::sGlobalState = {};
-
-void Systrace::init_once() noexcept {
-    GlobalState& s = sGlobalState;
-
-    s.markerFd = -1;
-
-    // API 23
-    loadSymbol(s.ATrace_isEnabled, "ATrace_isEnabled");
-    loadSymbol(s.ATrace_beginSection, "ATrace_beginSection");
-    loadSymbol(s.ATrace_endSection, "ATrace_endSection");
-    // API 29
-    loadSymbol(s.ATrace_beginAsyncSection, "ATrace_beginAsyncSection");
-    loadSymbol(s.ATrace_endAsyncSection, "ATrace_endAsyncSection");
-    loadSymbol(s.ATrace_setCounter, "ATrace_setCounter");
-
-
-    const bool hasBasicAtrace = s.ATrace_isEnabled &&
-                                s.ATrace_beginSection &&
-                                s.ATrace_endSection;
-
-    const bool hasFullATrace = hasBasicAtrace &&
-                               s.ATrace_beginAsyncSection &&
-                               s.ATrace_endAsyncSection &&
-                               s.ATrace_setCounter;
-
-    if (!hasFullATrace) {
-        s.markerFd = open("/sys/kernel/debug/tracing/trace_marker", O_WRONLY | O_CLOEXEC);
+class SystraceStaticInitialization {
+public:
+    SystraceStaticInitialization() {
+        perfetto::TracingInitArgs args;
+        args.backends |= perfetto::kSystemBackend;
+        perfetto::Tracing::Initialize(args);
+        systrace::TrackEvent::Register();
     }
+};
 
-    if (hasBasicAtrace && !hasFullATrace) {
-        // no-op if we don't have all these
-        s.ATrace_beginAsyncSection = [](const char* sectionName, int32_t cookie){};
-        s.ATrace_endAsyncSection = [](const char* sectionName, int32_t cookie){};
-        s.ATrace_setCounter = [](const char* sectionName, int64_t counterValue){};
-    }
+UTILS_UNUSED SystraceStaticInitialization sSystraceStaticInitialization{};
 
-    const bool hasLegacySystrace = s.markerFd != -1;
-
-    if (hasLegacySystrace && !hasFullATrace) {
-        // use legacy
-        s.beginSection = [](Systrace* that, const char* name) {
-            begin_body(that->mMarkerFd, that->mPid, name);
-        };
-        s.endSection = [](Systrace* that) {
-            end_body(that->mMarkerFd, that->mPid);
-        };
-        s.beginAsyncSection = [](Systrace* that, const char* name, int32_t cookie) {
-            async_begin_body(that->mMarkerFd, that->mPid, name, cookie);
-        };
-        s.endAsyncSection = [](Systrace* that, const char* name, int32_t cookie) {
-            async_end_body(that->mMarkerFd, that->mPid, name, cookie);
-        };
-        s.setCounter = [](Systrace* that, const char* name, int64_t value) {
-            int64_body(that->mMarkerFd, that->mPid, name, value);
-        };
-    } else if (hasBasicAtrace) {
-        // we have at least basic ATrace
-        s.beginSection = [](Systrace* that, const char* name) {
-            that->ATrace_beginSection(name);
-        };
-        s.endSection = [](Systrace* that) {
-            that->ATrace_endSection();
-        };
-        s.beginAsyncSection = [](Systrace* that, const char* name, int32_t cookie) {
-            that->ATrace_beginAsyncSection(name, cookie);
-        };
-        s.endAsyncSection = [](Systrace* that, const char* name, int32_t cookie) {
-            that->ATrace_endAsyncSection(name, cookie);
-        };
-        s.setCounter = [](Systrace* that, const char* name, int64_t value) {
-            that->ATrace_setCounter(name, value);
-        };
-    }
-
-    s.isTracingAvailable = hasLegacySystrace || hasFullATrace || hasBasicAtrace;
 }
 
-void Systrace::setup() noexcept {
-    pthread_once(&atrace_once_control, init_once);
-}
-
-void Systrace::enable(uint32_t tags) noexcept {
-    setup();
-    if (UTILS_LIKELY(sGlobalState.isTracingAvailable)) {
-        sGlobalState.isTracingEnabled.fetch_or(tags, std::memory_order_relaxed);
-    }
-}
-
-void Systrace::disable(uint32_t tags) noexcept {
-    sGlobalState.isTracingEnabled.fetch_and(~tags, std::memory_order_relaxed);
-}
-
-// unfortunately, this generates quite a bit of code because reading a global is not
-// trivial. For this reason, we do not inline this method.
-bool Systrace::isTracingEnabled(uint32_t tag) noexcept {
-    if (tag) {
-        setup();
-        return bool((sGlobalState.isTracingEnabled.load(std::memory_order_relaxed) | SYSTRACE_TAG_ALWAYS) & tag);
-    }
-    return false;
-}
-
-// ------------------------------------------------------------------------------------------------
-
-void Systrace::init(uint32_t tag) noexcept {
-    // must be called first
-    mIsTracingEnabled = isTracingEnabled(tag);
-
-    // cache static variables for better efficiency
-    GlobalState& s = sGlobalState;
-    ATrace_isEnabled         = s.ATrace_isEnabled;
-    ATrace_beginSection      = s.ATrace_beginSection;
-    ATrace_endSection        = s.ATrace_endSection;
-    ATrace_beginAsyncSection = s.ATrace_beginAsyncSection;
-    ATrace_endAsyncSection   = s.ATrace_endAsyncSection;
-    ATrace_setCounter        = s.ATrace_setCounter;
-
-    beginSection = s.beginSection;
-    endSection = s.endSection;
-    beginAsyncSection = s.beginAsyncSection;
-    endAsyncSection = s.endAsyncSection;
-    setCounter = s.setCounter;
-
-    mMarkerFd = s.markerFd;
-
-    mPid = getpid();
-}
-
-// ------------------------------------------------------------------------------------------------
-
-/**
- * Maximum size of a message that can be logged to the trace buffer.
- * Note this message includes a tag, the pid, and the string given as the name.
- * Names should be kept short to get the most use of the trace buffer.
- */
-#define ATRACE_MESSAGE_LENGTH 512
-
-#define WRITE_MSG(format_begin, format_end, pid, name, value) {                 \
-    char buf[ATRACE_MESSAGE_LENGTH];                                            \
-    int len = snprintf(buf, sizeof(buf), format_begin "%s" format_end, pid,     \
-        name, value);                                                           \
-    if (len >= (int) sizeof(buf)) {                                             \
-        /* Given the sizeof(buf), and all of the current format buffers,        \
-         * it is impossible for name_len to be < 0 if len >= sizeof(buf). */    \
-        int name_len = strlen(name) - (len - sizeof(buf)) - 1;                  \
-        /* Truncate the name to make the message fit. */                        \
-        len = snprintf(buf, sizeof(buf), format_begin "%.*s" format_end, pid,   \
-            name_len, name, value);                                             \
-    }                                                                           \
-    write(fd, buf, len);                                                        \
-}
-
-void Systrace::begin_body(int fd, int pid, const char* name) noexcept {
-    char buf[ATRACE_MESSAGE_LENGTH];
-    ssize_t len = snprintf(buf, sizeof(buf), "B|%d|%s", pid, name);
-    if (len >= sizeof(buf)) {
-        len = sizeof(buf) - 1;
-    }
-    write(fd, buf, size_t(len));
-}
-
-void Systrace::end_body(int fd, int pid) noexcept {
-    const char END_TAG = 'E';
-    write(fd, &END_TAG, 1);
-}
-
-void Systrace::async_begin_body(int fd, int pid, const char* name, int32_t cookie) noexcept {
-    WRITE_MSG("S|%d|", "|%" PRId32, pid, name, cookie);
-}
-
-void Systrace::async_end_body(int fd, int pid, const char* name, int32_t cookie) noexcept {
-    WRITE_MSG("F|%d|", "|%" PRId32, pid, name, cookie);
-}
-
-void Systrace::int64_body(int fd, int pid, const char* name, int64_t value) noexcept {
-    WRITE_MSG("C|%d|", "|%" PRId64, pid, name, value);
-}
-
-} // namespace details
-} // namespace utils

libs/utils/src/darwin/Systrace.cpp

@@ -14,11 +14,17 @@
  * limitations under the License.
  */
 
-#include <utils/Systrace.h>
-#include <utils/Log.h>
+#include <utils/darwin/Systrace.h>
+
+#ifndef FILAMENT_APPLE_SYSTRACE
+#   define FILAMENT_APPLE_SYSTRACE 0
+#endif
 
 #if FILAMENT_APPLE_SYSTRACE
 
+#include <atomic>
+#include <stack>
+#include <stdint.h>
 #include <pthread.h>
 
 static pthread_once_t atrace_once_control = PTHREAD_ONCE_INIT;
@@ -41,21 +47,24 @@ void Systrace::setup() noexcept {
     pthread_once(&atrace_once_control, init_once);
 }
 
-void Systrace::enable(uint32_t tags) noexcept {
+void Systrace::enable(uint32_t tag) noexcept {
     setup();
-    sGlobalState.isTracingEnabled.fetch_or(tags, std::memory_order_relaxed);
+    uint32_t const mask = 1 << tag;
+    sGlobalState.isTracingEnabled.fetch_or(mask, std::memory_order_relaxed);
 }
 
-void Systrace::disable(uint32_t tags) noexcept {
-    sGlobalState.isTracingEnabled.fetch_and(~tags, std::memory_order_relaxed);
+void Systrace::disable(uint32_t tag) noexcept {
+    uint32_t const mask = 1 << tag;
+    sGlobalState.isTracingEnabled.fetch_and(~mask, std::memory_order_relaxed);
 }
 
-// unfortunately, this generates quite a bit of code because reading a global is not
+// Unfortunately, this generates quite a bit of code because reading a global is not
 // trivial. For this reason, we do not inline this method.
 bool Systrace::isTracingEnabled(uint32_t tag) noexcept {
     if (tag) {
         setup();
-        return bool((sGlobalState.isTracingEnabled.load(std::memory_order_relaxed) | SYSTRACE_TAG_ALWAYS) & tag);
+        uint32_t const mask = 1 << tag;
+        return bool(sGlobalState.isTracingEnabled.load(std::memory_order_relaxed) & mask);
     }
     return false;
 }

libs/viewer/src/Settings.cpp

@@ -700,8 +700,7 @@ constexpr ToneMapper* createToneMapper(const ColorGradingSettings& settings) noe
 }
 
 ColorGrading* createColorGrading(const ColorGradingSettings& settings, Engine* engine) {
-    ToneMapper* toneMapperRaw = createToneMapper(settings);
-    std::shared_ptr<ToneMapper> toneMapper(toneMapperRaw);
+    ToneMapper* toneMapper = createToneMapper(settings);
     ColorGrading *colorGrading = ColorGrading::Builder()
             .quality(settings.quality)
             .exposure(settings.exposure)
@@ -724,6 +723,7 @@ ColorGrading* createColorGrading(const ColorGradingSettings& settings, Engine* e
             .gamutMapping(settings.gamutMapping)
             .outputColorSpace(settings.colorspace)
             .build(*engine);
+    delete toneMapper;
     return colorGrading;
 }
 

test/renderdiff/src/golden_manager.py

@@ -1,143 +0,0 @@
-# Copyright (C) 2025 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-import os
-import shutil
-import re
-
-from utils import execute, ArgParseImpl, mkdir_p
-
-GOLDENS_DIR = 'renderdiff'
-
-ACCESS_TYPE_TOKEN = 'token'
-ACCESS_TYPE_SSH = 'ssh'
-ACCESS_TYPE_READ_ONLY = 'read-only'
-
-def _read_git_config(curdir):
-  with open(os.path.join(curdir, './.git/config'), 'r') as f:
-    return f.read()
-
-def _write_git_config(curdir, config_str):
-  with open(os.path.join(curdir, './.git/config'), 'w') as f:
-    return f.write(config_str)
-
-class GoldenManager:
-  def __init__(self, working_dir, access_type=ACCESS_TYPE_READ_ONLY, access_token=None):
-    self.working_dir_ = working_dir
-    self.access_token_ = access_token
-    self.access_type_ = access_type
-    assert os.path.isdir(self.working_dir_),\
-        f"working directory {self.working_dir_} does not exist"
-    self._prepare()
-
-  def _assets_dir(self):
-    return os.path.join(self.working_dir_, "filament-assets")
-
-  # Returns the directory containing the goldens
-  def directory(self):
-    return os.path.join(self._assets_dir(), GOLDENS_DIR)
-
-  def _get_repo_url(self):
-    protocol = ''
-    protocol_separator = ''
-    if self.access_type_ == ACCESS_TYPE_SSH:
-      protocol = 'git@'
-      protocol_separator = ':'
-    else:
-      protocol = 'https://' + \
-        (f'x-access-token:{self.access_token_}@' if self.access_token_ else '')
-      protocol_separator = '/'
-    return f'{protocol}github.com{protocol_separator}google/filament-assets.git'
-
-  def _prepare(self):
-    assets_dir = self._assets_dir()
-    if not os.path.exists(assets_dir):
-      execute(
-          f'git clone --depth=1 {self._get_repo_url()}',
-          cwd=self.working_dir_,
-          capture_output=False
-      )
-    else:
-      if self.access_type_ == ACCESS_TYPE_SSH:
-        config = _read_git_config(self._assets_dir())
-        https_url = r'https://github\.com\/google\/filament\.git'
-        config = re.sub(https_url, self._get_repo_url(), config)
-        _write_git_config(self._assets_dir(), config)
-      self.update()
-
-  def update(self):
-    self._git_exec('fetch')
-    self._git_exec('checkout main')
-    self._git_exec('rebase')
-
-  def _git_exec(self, cmd):
-    execute(f'git {cmd}', cwd=self._assets_dir(), capture_output=False)
-
-  def merge_to_main(self, branch, push_to_remote=False):
-    self.update()
-    assets_dir = self._assets_dir()
-    self._git_exec(f'checkout main')
-    self._git_exec(f'merge --no-ff {branch}')
-    if push_to_remote and \
-       (self.access_token_ or self.access_type_ == ACCESS_TYPE_SSH):
-      self._git_exec(f'push origin main')
-      self.update()
-
-  def source_from(self, src_dir, commit_msg, branch,
-                  updates=[], deletes=[], push_to_remote=False):
-    assets_dir = self._assets_dir()
-    self._git_exec(f'checkout main')
-    # Force create the branch (note will overwrite the old branch)
-    self._git_exec(f'switch -C {branch}')
-    rdiff_dir = os.path.join(assets_dir, GOLDENS_DIR)
-    if len(updates) == 0 and len(deletes) == 0:
-      shutil.rmtree(rdiff_dir, ignore_errors=True)
-      mkdir_p(rdiff_dir)
-      shutil.copytree(src_dir, rdiff_dir, dirs_exist_ok=True)
-      self._git_exec(f'add {GOLDENS_DIR}')
-    else:
-      for f in deletes:
-        self._git_exec(f'remove {os.path.join(GOLDENS_DIR, f)}')
-      for f in updates:
-        shutil.copy2(
-          os.path.join(src_dir, f),
-          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:
-      f.write(commit_msg)
-    self._git_exec(f'commit -a -F {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}')
-      self.update()
-
-  def download_to(self, dest_dir, branch='main'):
-    self._git_exec(f'checkout {branch}')
-    assets_dir = self._assets_dir()
-    mkdir_p(dest_dir)
-    rdiff_dir = os.path.join(assets_dir, GOLDENS_DIR)
-    shutil.copytree(rdiff_dir, dest_dir, dirs_exist_ok=True)
-
-# For testing only
-if __name__ == "__main__":
-  golden_manager = GoldenManager(os.getcwd())
-  # golden_manager.source_from_and_commit(
-  #     os.path.join(os.getcwd(), 'out/renderdiff_tests'),
-  #     'First commit (local)',
-  #     branch='branch-test')
-  # golden_manager.merge_to_main('branch-test', push_to_remote=True)
-  # golden_manager.download_to(os.path.join(os.getcwd(), 'tmp/goldens'))

test/renderdiff/src/image_diff.py

@@ -1,43 +0,0 @@
-# Copyright (C) 2025 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-import tifffile
-import numpy
-
-def same_image(tiff_file_a, tiff_file_b):
-  try:
-    img1_data = tifffile.imread(tiff_file_a)
-    img2_data = tifffile.imread(tiff_file_b)
-
-    # If the dimensions (height, width, number of channels, number of pages/frames)
-    # are different, the images are not the same.
-    if img1_data.shape != img2_data.shape:
-      print(f"Images have different shapes: {img1_data.shape} vs {img2_data.shape}")
-      return False
-
-    # numpy.array_equal() checks if two arrays have the same shape and elements.
-    if numpy.array_equal(img1_data, img2_data):
-      return True
-    else:
-      return False
-
-  except FileNotFoundError:
-    print(f"Error: One or both files not found ('{file_path1}', '{file_path2}').")
-    return False
-  except tifffile.TiffFileError as e:
-    print(f"Error: One or both files are not valid TIFF files or could not be read. Details: {e}")
-    return False
-  except Exception as e:
-    print(f"An unexpected error occurred: {e}")
-    return False

test/renderdiff/src/run.py

@@ -14,14 +14,9 @@
 
 import sys
 import os
-import json
-import glob
-import shutil
 
-from utils import execute, ArgParseImpl, mkdir_p, mv_f
+from utils import execute, ArgParseImpl
 from parse_test_json import parse_test_config_from_path
-from golden_manager import GoldenManager
-from image_diff import same_image
 
 def important_print(msg):
   lines = msg.split('\n')
@@ -33,21 +28,13 @@ def important_print(msg):
     print(information)
   print('-' * (max_len + 8))
 
-RESULT_OK = 'ok'
-RESULT_FAILED_TO_RENDER = 'failed-to-render'
-RESULT_FAILED_IMAGE_DIFF = 'failed-image-diff'
-RESULT_FAILED_NO_GOLDEN = 'failed-no-golden'
-
-def run_test(gltf_viewer,
-             test_config,
-             output_dir,
-             opengl_lib=None,
-             vk_icd=None):
+def render_test(gltf_viewer, test_config, output_dir,
+                opengl_lib=None, vk_icd=None):
   assert os.path.isdir(output_dir), f"output directory {output_dir} does not exist"
   assert os.access(gltf_viewer, os.X_OK)
 
   named_output_dir = os.path.join(output_dir, test_config.name)
-  mkdir_p(named_output_dir)
+  execute(f'mkdir -p {named_output_dir}')
 
   results = []
   for test in test_config.tests:
@@ -73,45 +60,43 @@ def run_test(gltf_viewer,
 
         important_print(f'Rendering {test_desc}')
 
-        out_code, _ = execute(
-          f'{gltf_viewer} -a {backend} --batch={test_json_path} -e {model_path} --headless',
-          env=env, capture_output=False
-        )
+        res, _ = execute(f'{gltf_viewer} -a {backend} --batch={test_json_path} -e {model_path} --headless',
+                env=env, capture_output=False)
 
-        result = ''
-        if out_code == 0:
-          result = RESULT_OK
-          out_tif_basename = f'{out_name}.tif'
-          out_tif_name = f'{named_output_dir}/{out_tif_basename}'
-          mv_f(f'{test.name}0.tif', out_tif_name)
-          mv_f(f'{test.name}0.json', f'{named_output_dir}/{test.name}.json')
+        if res == 0:
+          execute(f'mv -f {test.name}0.tif {named_output_dir}/{out_name}.tif', capture_output=False)
+          execute(f'mv -f {test.name}0.json {named_output_dir}/{test.name}.json', capture_output=False)
         else:
-          result = RESULT_FAILED_TO_RENDER
-          important_print(f'{test_desc} rendering failed with error={out_code}')
+          important_print(f'{test_desc} failed with error={res}')
+        print('')
 
-        results.append({
-          'name': out_name,
-          'result': result,
-          'result_code': out_code,
-        })
-  return named_output_dir, results
+        results.append((out_name, res))
+  return results
 
-def compare_goldens(render_results, output_dir, goldens):
-  for result in render_results:
-    if result['result'] != RESULT_OK:
-      continue
+GOLDENS_DIR = 'renderdiff_goldens'
 
-    out_tif_basename = f"{result['name']}.tif"
-    out_tif_name = f'{output_dir}/{out_tif_basename}'
-    golden_path = goldens.get(out_tif_basename)
-    if not golden_path:
-      result['result'] = RESULT_FAILED_NO_GOLDEN
-      result['result_code'] = 1
-    elif not same_image(golden_path, out_tif_name):
-      result['result'] = RESULT_FAILED_IMAGE_DIFF
-      result['result_code'] = 1
+# We pull the goldens from the filament-assets repo
+def pull_goldens(output_dir):
+  assert os.path.isdir(output_dir), f"output directory {output_dir} does not exist"
+  golden_dir = os.path.join(output_dir, "golden")
+  assets_dir = os.path.join(output_dir, "filament-assets")
 
-  return render_results
+  if not os.path.exists(assets_dir):
+    execute('git clone --depth 1 git@github.com:google/filament-assets.git', cwd=output_dir)
+  else:
+    execute('git fetch', cwd=assets_dir)
+    execute('git checkout main ', cwd=assets_dir)
+    execute('git rebase', cwd=assets_dir)
+
+  if os.path.exists(golden_dir):
+    execute('rm -f goldens/*', cwd=output_dir)
+  execute(f'cp filament-assets/{GOLDENS_DIR}/* goldens', cwd=output_dir)
+
+def push_goldens(output_dir, test_name, filter_func=lambda a:True):
+  for test in test_config.tests:
+    for backend in test_config.backends:
+      for model in test.models:
+        pass
 
 if __name__ == "__main__":
   parser = ArgParseImpl()
@@ -120,43 +105,12 @@ if __name__ == "__main__":
   parser.add_argument('--output_dir', help='Output Directory', required=True)
   parser.add_argument('--opengl_lib', help='Path to the folder containing OpenGL driver lib (for LD_LIBRARY_PATH)')
   parser.add_argument('--vk_icd', help='Path to VK ICD file')
-  parser.add_argument('--golden_branch', help='Branch of the golden repo to compare against')
 
   args, _ = parser.parse_known_args(sys.argv[1:])
   test = parse_test_config_from_path(args.test)
+  render_result = render_test(args.gltf_viewer, test, args.output_dir, opengl_lib=args.opengl_lib, vk_icd=args.vk_icd)
 
-  output_dir, results = \
-    run_test(args.gltf_viewer,
-             test,
-             args.output_dir,
-             opengl_lib=args.opengl_lib,
-             vk_icd=args.vk_icd)
-
-  do_compare = False
-  # The presence of this argument indicates comparison against a set of goldens.
-  if args.golden_branch:
-    # prepare goldens working directory
-    tmp_golden_dir = '/tmp/renderdiff-goldens'
-    mkdir_p(tmp_golden_dir)
-
-    # Download the golden repo into the current working directory
-    golden_manager = GoldenManager(os.getcwd())
-    golden_manager.download_to(tmp_golden_dir, branch=args.golden_branch)
-
-    goldens = {
-      os.path.basename(fpath) : fpath for fpath in \
-      glob.glob(f'{os.path.join(tmp_golden_dir, test.name)}/**/*.tif', recursive=True)
-    }
-    results = compare_goldens(results, output_dir, goldens)
-    do_compare = True
-
-  with open(f'{output_dir}/results.json', 'w') as f:
-    f.write(json.dumps(results))
-
-  shutil.copy2(args.test, f'{output_dir}/test.json')
-
-  failed = [f"   {k['name']}" for k in results if k['result'] != RESULT_OK]
-  success_count = len(results) - len(failed)
-  op = 'tested' if do_compare else 'rendered'
-  important_print(f'Successfully {op} {success_count} / {len(results)}' +
+  failed = [f'   {tname}' for tname, res in render_result if res != 0]
+  success_count = len(render_result) - len(failed  )
+  important_print(f'Successfully rendered {success_count} / {len(render_result)}' +
                   ('\nFailed:\n' + ('\n'.join(failed)) if len(failed) > 0 else ''))

test/renderdiff/src/update_golden.py

@@ -1,171 +0,0 @@
-# Copyright (C) 2025 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-import sys
-import os
-import glob
-import time
-
-from golden_manager import GoldenManager, ACCESS_TYPE_SSH, ACCESS_TYPE_TOKEN
-from image_diff import same_image
-
-from utils import execute, ArgParseImpl
-from utils import prompt_helper, PROMPT_YES, PROMPT_NO
-
-def line_prompt(prompt, validator=lambda a:True):
-  while True:
-    res = input(f'{prompt} => ').strip()
-    if validator(res):
-      return res
-  return None
-
-CONFIG_NEW_SRC_DIR = 'goldens_dir'
-CONFIG_GOLDENS_BRANCH = 'goldens_branch'
-CONFIG_GOLDENS_UPDATES = 'goldens_updates'
-CONFIG_GOLDENS_DELETES = 'goldens_deletes'
-CONFIG_AUTO_COMMIT = 'auto-commit'
-CONFIG_COMMIT_MSG = 'commit_msg'
-
-def _get_current_branch():
-  code, res = execute('git branch --show-current')
-  return res.strip()
-
-def _file_as_str(fpath):
-  with open(fpath, 'r') as f:
-    return f.read()
-
-def _do_update(golden_manager, config):
-  deletes = config[CONFIG_GOLDENS_DELETES]
-  updates = config[CONFIG_GOLDENS_UPDATES]
-  if len(deletes) == 0 and len(updates) == 0:
-    print('Nothing to update. Exiting...')
-    exit(0)
-
-  branch = config[CONFIG_GOLDENS_BRANCH]
-  src_dir = config[CONFIG_NEW_SRC_DIR]
-  auto_commit = config[CONFIG_AUTO_COMMIT]
-  commit_msg = config[CONFIG_COMMIT_MSG]
-  golden_manager.source_from(src_dir, commit_msg, branch,
-                             updates=updates,
-                             deletes=deletes,
-                             push_to_remote=auto_commit)
-
-def _get_deletes_updates(update_dir, golden_dir):
-  ret_delete = []
-  ret_update = []
-  for ext in ['tif', 'json']:
-    base = set(glob.glob(f'./**/*.{ext}', root_dir=golden_dir, recursive=True))
-    new = set(glob.glob(f'./**/*.{ext}', root_dir=update_dir, recursive=True))
-
-    delete = list(base - new)
-    update = list(new - base)
-
-    for fpath in base.intersection(new):
-      base_fpath = os.path.join(golden_dir, fpath)
-      new_fpath = os.path.join(update_dir, fpath)
-      if (ext == 'tif' and not same_image(new_fpath, base_fpath)) or \
-         (ext == 'json' and _file_as_str(new_fpath) != _file_as_str(base_fpath)):
-        update.append(fpath)
-
-    ret_update += update
-    ret_delete += delete
-
-  return ret_delete, ret_update
-
-# Ask a bunch of questions to gather the configuration for the update
-def _interactive_mode(base_golden_dir):
-  config = {}
-  cur_branch = _get_current_branch()
-  if prompt_helper(
-          f'Generate the new goldens from your local ' \
-          f'Filament branch? (branch={cur_branch})') == PROMPT_YES:
-    code, res = execute('bash ./test/renderdiff/test.sh generate',
-            capture_output=False)
-    if code != 0:
-      print('Failed to generate new goldens')
-      exit(1)
-    config[CONFIG_NEW_SRC_DIR] = os.path.join(os.getcwd(), './out/renderdiff_tests/')
-  else:
-    def validator(src_dir):
-      if not os.path.exists(src_dir):
-        print(f'Cannot find directory {src_dir}. Please try again.')
-        return False
-      return True
-
-    config[CONFIG_NEW_SRC_DIR] = line_prompt(
-        'Please provide path of directory containing new goldens',
-        validator)
-
-  if prompt_helper(f'Update new goldens to branch={cur_branch}? '
-                   '(Note that this refers to a branch in the goldens repo, not the Filament repo.)'
-                   ) == PROMPT_YES:
-    config[CONFIG_GOLDENS_BRANCH] = cur_branch
-  else:
-    config[CONFIG_GOLDENS_BRANCH] = line_prompt('Please provide new branch name for update')
-
-  if prompt_helper(f'Provide a commit message?') == PROMPT_YES:
-    config[CONFIG_COMMIT_MSG] = line_prompt('Message:')
-  else:
-    config[CONFIG_COMMIT_MSG] = f'Update {time.time()} from filament ({cur_branch})'
-
-  new_golden_dir = config[CONFIG_NEW_SRC_DIR]
-  deletes, updates = _get_deletes_updates(new_golden_dir, base_golden_dir)
-  if len(deletes) + len(updates) != 0:
-    prompt = 'The following files will be changed:\n' + \
-        '\n'.join([f'  {fname} [delete]' for fname in deletes]) + \
-        '\n'.join([f'  {fname} [update]' for fname in updates]) + \
-        '\nIs that ok?'
-    if prompt_helper(prompt) == PROMPT_YES:
-      config[CONFIG_GOLDENS_DELETES] = deletes
-      config[CONFIG_GOLDENS_UPDATES] = updates
-    else:
-      # We cannot proceed if user answered no.
-      exit(1)
-  else:
-    config[CONFIG_GOLDENS_DELETES] = []
-    config[CONFIG_GOLDENS_UPDATES] = []
-
-  config[CONFIG_AUTO_COMMIT] = \
-      prompt_helper(f'Commit golden repo changes to remote?') == PROMPT_YES
-  return config
-
-if __name__ == "__main__":
-  parser = ArgParseImpl()
-  parser.add_argument('--branch', help='Branch of the golden repo to write to')
-  parser.add_argument('--source', help='Directory containing the new goldens')
-  parser.add_argument('--commit-msg', help='Message for the commit to the golden repo')
-  parser.add_argument('--golden-repo-token', help='Access token for the golden repo')
-
-  args, _ = parser.parse_known_args(sys.argv[1:])
-  config = {}
-  golden_manager = GoldenManager(
-      os.getcwd(),
-      access_type=ACCESS_TYPE_SSH if not args.golden_repo_token else ACCESS_TYPE_TOKEN,
-      access_token=args.golden_repo_token
-  )
-  base_golden_dir = golden_manager.directory()
-  if args.branch and args.source and args.commit_msg:
-    assert os.path.exists(args.source), f'{args.source} (--source) directory not found'
-    deletes, updates = _get_deletes_updates(args.source, base_golden_dir)
-    config = {
-        CONFIG_AUTO_COMMIT: True,
-        CONFIG_GOLDENS_BRANCH: args.branch,
-        CONFIG_NEW_SRC_DIR: args.source,
-        CONFIG_GOLDENS_UPDATES: updates,
-        CONFIG_GOLDENS_DELETES: deletes,
-        CONFIG_COMMIT_MSG: args.commit_msg,
-    }
-  else:
-    config = _interactive_mode(base_golden_dir)
-  _do_update(golden_manager, config)

test/renderdiff/src/utils.py

@@ -16,7 +16,6 @@ import subprocess
 import os
 import argparse
 import sys
-import pathlib
 
 def execute(cmd,
             cwd=None,
@@ -67,42 +66,3 @@ class ArgParseImpl(argparse.ArgumentParser):
     sys.stderr.write('error: %s\n' % message)
     self.print_help()
     sys.exit(1)
-
-
-PROMPT_YES = 'y'
-PROMPT_NO = 'n'
-PROMPT_YES_NO = f'{PROMPT_YES}{PROMPT_NO}'
-
-class GetCh:
-  def __init__(self):
-    pass
-
-  def __call__(self):
-    import sys, tty, termios
-    fd = sys.stdin.fileno()
-    old_settings = termios.tcgetattr(fd)
-    try:
-      tty.setraw(sys.stdin.fileno())
-      ch = sys.stdin.read(1)
-    finally:
-      termios.tcsetattr(fd, termios.TCSADRAIN, old_settings)
-    return ch
-
-getch = GetCh()
-
-def prompt_helper(prompt_str, keys=PROMPT_YES_NO):
-  while True:
-    print(f'{prompt_str}: [' + ', '.join(keys) + '] => ', end='', flush=True)
-    val = getch()
-    print(val)
-    if val in keys or ord(val) == 3: # If user pressed Ctrl+c
-      if ord(val) == 3:
-        exit(1)
-    return val
-
-def mkdir_p(path_str):
-  pathlib.Path(path_str).mkdir(parents=True, exist_ok=True)
-
-def mv_f(src_str, dst_str):
-  src = pathlib.Path(src_str)
-  src.replace(dst_str)

test/renderdiff/src/workflow_presubmit_msg.py

@@ -17,29 +17,18 @@ from utils import execute
 def get_last_commit():
   res, o = execute('git log -1')
   commit, author, date, _, title, *desc = o.split('\n')
-  commit = commit.split(' ')[1]
-  title = title.strip()
 
   desc = [l.strip() for l in desc[1:]]
-  while len(desc) > 0 and len(desc[0]) == 0:
+  if len(desc) > 0 and len(desc[0]) == 0:
     desc = desc[1:]
 
   return (
-    commit,
-    title,
-    desc
-  )
+    commit.split(' ')[1],
+    title.strip(),
+    desc)
 
 def sanitized_split(line, split_atom='\n'):
-  return list(
-    filter(
-      lambda x: len(x) > 0,
-      map(
-        lambda x: x.strip(),
-        line.split(split_atom)
-      )
-    )
-  )
+  return list(filter(lambda x: len(x) > 0, map(lambda x: x.strip(), line.split(split_atom))))
 
 RDIFF_UPDATE_GOLDEN_STR = 'RDIFF_UPDATE_GOLDEN'
 

test/renderdiff/test.sh

@@ -18,7 +18,6 @@ OUTPUT_DIR="$(pwd)/out/renderdiff_tests"
 RENDERDIFF_TEST_DIR="$(pwd)/test/renderdiff"
 TEST_UTILS_DIR="$(pwd)/test/utils"
 MESA_DIR="$(pwd)/mesa/out/"
-VENV_DIR="$(pwd)/venv"
 
 os_name=$(uname -s)
 if [[ "$os_name" == "Linux" ]]; then
@@ -30,33 +29,9 @@ else
     exit 1
 fi
 
-function start_() {
-    if [[ "$GITHUB_WORKFLOW" ]]; then
-        set -ex
-    else
-        if [ ! -d ${MESA_LIB_DIR} ]; then
-            bash ${TEST_UTILS_DIR}/get_mesa.sh
-        fi
-
-        # Install python deps
-        python3 -m venv ${VENV_DIR}
-        source ${VENV_DIR}/bin/activate
-
-        NEEDED_PYTHON_DEPS=("numpy" "tifffile")
-        for cmd in "${NEEDED_PYTHON_DEPS[@]}"; do
-            if ! python3 -m pip show -q "${cmd}"; then
-                python3 -m pip install ${cmd}
-            fi
-        done
-    fi
-
-}
-
-function end_() {
-    if [[ "$GITHUB_WORKFLOW" ]]; then
-        set +ex
-    else
-        deactivate # End python virtual env
+function prepare_mesa() {
+    if [ ! -d ${MESA_LIB_DIR} ]; then
+        bash ${TEST_UTILS_DIR}/get_mesa.sh
     fi
 }
 
@@ -66,19 +41,11 @@ function end_() {
 #  - Run the python script that runs the test
 #  - Zip up the result
 
-GOLDEN_BRANCH_PARAM='--golden_branch=main'
-
-if [ "$1" == "generate" ]; then
-    GOLDEN_BRANCH_PARAM=''
-fi
-
-start_ && \
+set -ex && prepare_mesa && \
     mkdir -p ${OUTPUT_DIR} && \
-    CXX=`which clang++` CC=`which clang` ./build.sh -f -X ${MESA_DIR} -p desktop debug gltf_viewer && \
-    python3 ${RENDERDIFF_TEST_DIR}/src/run.py \
+        CXX=`which clang++` CC=`which clang` ./build.sh -X ${MESA_DIR} -p desktop debug gltf_viewer && \
+        python3 ${RENDERDIFF_TEST_DIR}/src/run.py \
             --gltf_viewer="$(pwd)/out/cmake-debug/samples/gltf_viewer" \
             --test=${RENDERDIFF_TEST_DIR}/tests/presubmit.json \
             --output_dir=${OUTPUT_DIR} \
-            --opengl_lib=${MESA_LIB_DIR} \
-            ${GOLDEN_BRANCH_PARAM} && \
-    end_
+            --opengl_lib=${MESA_LIB_DIR}

test/utils/get_mesa.sh

@@ -14,9 +14,9 @@
 
 #!/usr/bin/bash
 
-set -e
+set -x
 if [[ "$GITHUB_WORKFLOW" ]]; then
-    set -x
+    set -e
 fi
 
 OS_NAME=$(uname -s)
@@ -35,7 +35,7 @@ source ${ORIG_DIR}/venv/bin/activate
 
 NEEDED_PYTHON_DEPS=("mako" "setuptools" "pyyaml")
 for cmd in "${NEEDED_PYTHON_DEPS[@]}"; do
-    if ! python3 -m pip show -q "${cmd}" >/dev/null 2>&1; then
+    if ! python3 -m pip show "${cmd}" >/dev/null 2>&1; then
         python3 -m pip install ${cmd}
     fi
 done
@@ -145,6 +145,6 @@ deactivate
 popd
 
 if [[ "$GITHUB_WORKFLOW" ]]; then
-    set +x
+    set +e
 fi
-set +e
+set +x

web/filament-js/package.json

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