Merge branch 'rc/1.56.3' into release

README.md

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

RELEASE_NOTES.md

@@ -7,6 +7,9 @@ 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.56.3
+
+
 ## v1.56.2
 
 - vk: fix stage pool gc logic

android/gradle.properties

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

filament/src/details/Engine.cpp

@@ -529,19 +529,31 @@ void FEngine::shutdown() {
     mPerViewDescriptorSetLayoutSsrVariant.terminate(mHwDescriptorSetLayoutFactory, driver);
     mPerRenderableDescriptorSetLayout.terminate(mHwDescriptorSetLayoutFactory, driver);
 
-    driver.destroyRenderPrimitive(mFullScreenTriangleRph);
+    driver.destroyRenderPrimitive(std::move(mFullScreenTriangleRph));
 
     destroy(mFullScreenTriangleIb);
+    mFullScreenTriangleIb = nullptr;
+
     destroy(mFullScreenTriangleVb);
+    mFullScreenTriangleVb = nullptr;
+
     destroy(mDummyMorphTargetBuffer);
+    mDummyMorphTargetBuffer = nullptr;
+
     destroy(mDefaultIblTexture);
+    mDefaultIblTexture = nullptr;
+
     destroy(mDefaultIbl);
+    mDefaultIbl = nullptr;
 
     destroy(mDefaultColorGrading);
+    mDefaultColorGrading = nullptr;
 
     destroy(mDefaultMaterial);
+    mDefaultMaterial = nullptr;
 
     destroy(mUnprotectedDummySwapchain);
+    mUnprotectedDummySwapchain = nullptr;
 
     /*
      * clean-up after the user -- we call terminate on each "leaked" object and clear each list.
@@ -558,6 +570,7 @@ void FEngine::shutdown() {
 
     // this must be done after Skyboxes and before materials
     destroy(mSkyboxMaterial);
+    mSkyboxMaterial = nullptr;
 
     cleanupResourceList(std::move(mBufferObjects));
     cleanupResourceList(std::move(mIndexBuffers));
@@ -574,12 +587,12 @@ void FEngine::shutdown() {
 
     cleanupResourceListLocked(mFenceListLock, std::move(mFences));
 
-    driver.destroyTexture(mDummyOneTexture);
-    driver.destroyTexture(mDummyOneTextureArray);
-    driver.destroyTexture(mDummyZeroTexture);
-    driver.destroyTexture(mDummyZeroTextureArray);
+    driver.destroyTexture(std::move(mDummyOneTexture));
+    driver.destroyTexture(std::move(mDummyOneTextureArray));
+    driver.destroyTexture(std::move(mDummyZeroTexture));
+    driver.destroyTexture(std::move(mDummyZeroTextureArray));
 
-    driver.destroyRenderTarget(mDefaultRenderTarget);
+    driver.destroyRenderTarget(std::move(mDefaultRenderTarget));
 
     /*
      * Shutdown the backend...

filament/src/details/Material.cpp

@@ -321,18 +321,20 @@ FMaterial::FMaterial(FEngine& engine, const Material::Builder& builder,
         parser->getSpecularAntiAliasingThreshold(&mSpecularAntiAliasingThreshold);
     }
 
-    processBlendingMode(parser);
-    processSpecializationConstants(engine, builder, parser);
-    processPushConstants(engine, parser);
-    processDepthVariants(engine, parser);
-    processDescriptorSets(engine, parser);
+    parser->hasCustomDepthShader(&mHasCustomDepthShader);
 
     mPerViewLayoutIndex = ColorPassDescriptorSet::getIndex(
             mIsVariantLit,
             mReflectionMode == ReflectionMode::SCREEN_SPACE ||
             mRefractionMode == RefractionMode::SCREEN_SPACE,
             !(mVariantFilterMask & +UserVariantFilterBit::FOG));
 
+    processBlendingMode(parser);
+    processSpecializationConstants(engine, builder, parser);
+    processPushConstants(engine, parser);
+    processDescriptorSets(engine, parser);
+    precacheDepthVariants(engine);
+
 #if FILAMENT_ENABLE_MATDBG
     // Register the material with matdbg.
     matdbg::DebugServer* server = downcast(engine).debug.server;
@@ -346,47 +348,21 @@ FMaterial::FMaterial(FEngine& engine, const Material::Builder& builder,
 FMaterial::~FMaterial() noexcept = default;
 
 void FMaterial::invalidate(Variant::type_t variantMask, Variant::type_t variantValue) noexcept {
-    DriverApi& driverApi = mEngine.getDriverApi();
-    if (mMaterialDomain == MaterialDomain::SURFACE) {
-        auto& cachedPrograms = mCachedPrograms;
-        for (size_t k = 0, n = VARIANT_COUNT; k < n; ++k) {
-            Variant const variant(k);
-            if ((k & variantMask) == variantValue) {
-                if (UTILS_LIKELY(!mIsDefaultMaterial)) {
-                    // The depth variants may be shared with the default material, in which case
-                    // we should not free it now.
-                    bool const isSharedVariant =
-                            Variant::isValidDepthVariant(variant) && !mHasCustomDepthShader;
-                    if (isSharedVariant) {
-                        // we don't own this variant, skip.
-                        continue;
-                    }
-                }
-                driverApi.destroyProgram(cachedPrograms[k]);
-                cachedPrograms[k].clear();
-            }
-        }
-
-         if (UTILS_UNLIKELY(!mIsDefaultMaterial && !mHasCustomDepthShader)) {
-            FMaterial const* const pDefaultMaterial = mEngine.getDefaultMaterial();
-            for (Variant const variant: pDefaultMaterial->mDepthVariants) {
-                pDefaultMaterial->prepareProgram(variant);
-                if (!cachedPrograms[variant.key]) {
-                    cachedPrograms[variant.key] = pDefaultMaterial->getProgram(variant);
-                }
-            }
+    // Note: This API is not public at the moment, so it's okay to have some debugging logs
+    // and extra checks.
+    if (mMaterialDomain == MaterialDomain::SURFACE &&
+        !mIsDefaultMaterial &&
+        !mHasCustomDepthShader) {
+        // it would be unsafe to invalidate any of the cached depth variant
+        if (UTILS_UNLIKELY(!((variantMask & Variant::DEP) && !(variantValue & Variant::DEP)))) {
+            slog.w << io::hex << "FMaterial::invalidate("
+                   << +variantMask << ", " << +variantValue
+                   << ") would corrupt the depth variant cache" << io::endl;
         }
-    } else if (mMaterialDomain == MaterialDomain::POST_PROCESS) {
-        auto& cachedPrograms = mCachedPrograms;
-        for (size_t k = 0, n = POST_PROCESS_VARIANT_COUNT; k < n; ++k) {
-            if ((k & variantMask) == variantValue) {
-                driverApi.destroyProgram(cachedPrograms[k]);
-                cachedPrograms[k].clear();
-            }
-        }
-    } else if (mMaterialDomain == MaterialDomain::COMPUTE) {
-        // TODO: handle compute variants if any
+        variantMask |= Variant::DEP;
+        variantValue &= ~Variant::DEP;
     }
+    destroyPrograms(mEngine, variantMask, variantValue);
 }
 
 void FMaterial::terminate(FEngine& engine) {
@@ -626,10 +602,42 @@ Program FMaterial::getProgramWithVariants(
 }
 
 void FMaterial::createAndCacheProgram(Program&& p, Variant variant) const noexcept {
-    auto program = mEngine.getDriverApi().createProgram(std::move(p));
-    mEngine.getDriverApi().setDebugTag(program.getId(), mName);
+    FEngine const& engine = mEngine;
+    DriverApi& driverApi = mEngine.getDriverApi();
+
+    bool const isSharedVariant =
+            (mMaterialDomain == MaterialDomain::SURFACE) &&
+            !mIsDefaultMaterial && !mHasCustomDepthShader &&
+            Variant::isValidDepthVariant(variant);
+
+    // Check if the default material has this program cached
+    if (isSharedVariant) {
+        FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial();
+        if (pDefaultMaterial) {
+            auto program = pDefaultMaterial->mCachedPrograms[variant.key];
+            if (program) {
+                mCachedPrograms[variant.key] = program;
+                return;
+            }
+        }
+    }
+
+    auto program = driverApi.createProgram(std::move(p));
+    driverApi.setDebugTag(program.getId(), mName);
     assert_invariant(program);
     mCachedPrograms[variant.key] = program;
+
+    // If the default material doesn't already have this program cached, and all caching conditions
+    // are met (Surface Domain and no custom depth shader), cache it now.
+    // New Materials will inherit these program automatically.
+    if (isSharedVariant) {
+        FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial();
+        if (pDefaultMaterial && !pDefaultMaterial->mCachedPrograms[variant.key]) {
+            // set the tag to the default material name
+            driverApi.setDebugTag(program.getId(), mName);
+            pDefaultMaterial->mCachedPrograms[variant.key] = program;
+        }
+    }
 }
 
 size_t FMaterial::getParameters(ParameterInfo* parameters, size_t count) const noexcept {
@@ -734,22 +742,76 @@ void FMaterial::onQueryCallback(void* userdata, VariantList* pVariants) {
 #endif // FILAMENT_ENABLE_MATDBG
 
 
-void FMaterial::destroyPrograms(FEngine& engine) {
+void FMaterial::destroyPrograms(FEngine& engine,
+        Variant::type_t const variantMask, Variant::type_t const variantValue) {
+
     DriverApi& driverApi = engine.getDriverApi();
     auto& cachedPrograms = mCachedPrograms;
-    for (size_t k = 0, n = VARIANT_COUNT; k < n; ++k) {
-        const Variant variant(k);
-        if (!mIsDefaultMaterial) {
-            // The depth variants may be shared with the default material, in which case
-            // we should not free it now.
-            bool const isSharedVariant = Variant::isValidDepthVariant(variant) && !mHasCustomDepthShader;
-            if (isSharedVariant) {
-                // we don't own this variant, skip.
-                continue;
+
+    switch (mMaterialDomain) {
+        case MaterialDomain::SURFACE: {
+            if (mIsDefaultMaterial || mHasCustomDepthShader) {
+                // default material or we have custom depth shaders, we destroy all variants
+                for (size_t k = 0, n = VARIANT_COUNT; k < n; ++k) {
+                    if ((k & variantMask) == variantValue) {
+                        // Only destroy if the handle is valid. Not strictly needed, but we have a lot
+                        // of variants, and this generates traffic in the command queue.
+                        if (cachedPrograms[k]) {
+                            driverApi.destroyProgram(std::move(cachedPrograms[k]));
+                        }
+                    }
+                }
+            } else {
+                // The depth variants may be shared with the default material, in which case
+                // we should not free them now.
+
+                // During Engine::shutdown(), auto-cleanup destroys the default material first,
+                // so this can be null, but this is only used for debugging.
+                UTILS_UNUSED_IN_RELEASE
+                auto UTILS_NULLABLE pDefaultMaterial = engine.getDefaultMaterial();
+
+                for (size_t k = 0, n = VARIANT_COUNT; k < n; ++k) {
+                    if ((k & variantMask) == variantValue) {
+                        // Only destroy if the handle is valid. Not strictly needed, but we have a lot
+                        // of variant, and this generates traffic in the command queue.
+                        if (cachedPrograms[k]) {
+                            if (Variant::isValidDepthVariant(Variant(k))) {
+                                // By construction this should always be true, because this
+                                // field is populated only when a material creates the program
+                                // for this variant.
+                                // During Engine::shutdown, auto-cleanup destroys the
+                                // default material first
+                                assert_invariant(!pDefaultMaterial ||
+                                        pDefaultMaterial->mCachedPrograms[k]);
+                                // we don't own this variant, skip, but clear the entry.
+                                cachedPrograms[k].clear();
+                                continue;
+                            }
+
+                            driverApi.destroyProgram(std::move(cachedPrograms[k]));
+                        }
+                    }
+                }
             }
+            break;
+        }
+        case MaterialDomain::POST_PROCESS: {
+            for (size_t k = 0, n = POST_PROCESS_VARIANT_COUNT; k < n; ++k) {
+                if ((k & variantMask) == variantValue) {
+                    // Only destroy if the handle is valid. Not strictly needed, but we have a lot
+                    // of variant, and this generates traffic in the command queue.
+                    if (cachedPrograms[k]) {
+                        driverApi.destroyProgram(std::move(cachedPrograms[k]));
+                    }
+                }
+            }
+            break;
+        }
+        case MaterialDomain::COMPUTE: {
+            // Compute programs don't have variants
+            driverApi.destroyProgram(std::move(cachedPrograms[0]));
+            break;
         }
-        driverApi.destroyProgram(cachedPrograms[k]);
-        cachedPrograms[k].clear();
     }
 }
 
@@ -1014,34 +1076,32 @@ void FMaterial::processPushConstants(FEngine& engine, MaterialParser const* pars
             });
 }
 
-void FMaterial::processDepthVariants(FEngine& engine, MaterialParser const* const parser) {
-    parser->hasCustomDepthShader(&mHasCustomDepthShader);
-
+void FMaterial::precacheDepthVariants(FEngine& engine) {
+    // pre-cache all depth variants inside the default material. Note that this should be
+    // entirely optional; if we remove this pre-caching, these variants will be populated
+    // later, when/if needed by createAndCacheProgram(). Doing it now potentially uses more
+    // memory and increases init time, but reduces hiccups during the first frame.
     if (UTILS_UNLIKELY(mIsDefaultMaterial)) {
-        assert_invariant(mMaterialDomain == MaterialDomain::SURFACE);
-        filaflat::MaterialChunk const& materialChunk{ parser->getMaterialChunk() };
-        auto variants = FixedCapacityVector<Variant>::with_capacity(materialChunk.getShaderCount());
-        materialChunk.visitShaders([&variants](
-                ShaderModel, Variant variant, ShaderStage) {
-            if (Variant::isValidDepthVariant(variant)) {
-                variants.push_back(variant);
+        auto const allDepthVariants = VariantUtils::getDepthVariants();
+        for (auto const variant: allDepthVariants) {
+            assert_invariant(Variant::isValidDepthVariant(variant));
+            if (hasVariant(variant)) {
+                prepareProgram(variant);
             }
-        });
-        std::sort(variants.begin(), variants.end(),
-                [](Variant lhs, Variant rhs) { return lhs.key < rhs.key; });
-        auto pos = std::unique(variants.begin(), variants.end());
-        variants.resize(std::distance(variants.begin(), pos));
-        std::swap(mDepthVariants, variants);
+        }
+        return;
     }
 
-    if (mMaterialDomain == MaterialDomain::SURFACE) {
-        if (UTILS_UNLIKELY(!mIsDefaultMaterial && !mHasCustomDepthShader)) {
-            FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial();
-            auto& cachedPrograms = mCachedPrograms;
-            for (Variant const variant: pDefaultMaterial->mDepthVariants) {
-                pDefaultMaterial->prepareProgram(variant);
-                cachedPrograms[variant.key] = pDefaultMaterial->getProgram(variant);
-            }
+    // if possible pre-cache all depth variants from the default material
+    if (mMaterialDomain == MaterialDomain::SURFACE &&
+            !mIsDefaultMaterial &&
+            !mHasCustomDepthShader) {
+        FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial();
+        assert_invariant(pDefaultMaterial);
+        auto const allDepthVariants = VariantUtils::getDepthVariants();
+        for (auto const variant: allDepthVariants) {
+            assert_invariant(Variant::isValidDepthVariant(variant));
+            mCachedPrograms[variant.key] = pDefaultMaterial->mCachedPrograms[variant.key];
         }
     }
 }

filament/src/details/Material.h

@@ -218,7 +218,9 @@ class FMaterial : public Material {
 
     uint32_t generateMaterialInstanceId() const noexcept { return mMaterialInstanceId++; }
 
-    void destroyPrograms(FEngine& engine);
+    void destroyPrograms(FEngine& engine,
+            Variant::type_t variantMask = 0,
+            Variant::type_t variantValue = 0);
 
     // return the id of a specialization constant specified by name for this material
     std::optional<uint32_t> getSpecializationConstantId(std::string_view name) const noexcept ;
@@ -282,7 +284,7 @@ class FMaterial : public Material {
 
     void processPushConstants(FEngine& engine, MaterialParser const* parser);
 
-    void processDepthVariants(FEngine& engine, MaterialParser const* parser);
+    void precacheDepthVariants(FEngine& engine);
 
     void processDescriptorSets(FEngine& engine, MaterialParser const* parser);
 
@@ -331,7 +333,6 @@ class FMaterial : public Material {
     SamplerInterfaceBlock mSamplerInterfaceBlock;
     BufferInterfaceBlock mUniformInterfaceBlock;
     SubpassInfo mSubpassInfo;
-    utils::FixedCapacityVector<Variant> mDepthVariants; // only populated with default material
 
     using BindingUniformInfoContainer = utils::FixedCapacityVector<std::tuple<
             uint8_t, utils::CString, backend::Program::UniformInfo>>;