From 0689e79441108c3a4e1693b154524ebf03e05b9f Mon Sep 17 00:00:00 2001
From: Powei Feng <powei@google.com>
Date: Wed, 2 Oct 2024 06:24:41 -0700
Subject: [PATCH] Work around client descriptor set issues (#8171)

- We change GLDescriptorSet::Buffer default constructor to
  workaround a client's compiler set up issue.
- We removed the assert_invariant that checks that ubo/samplers
  are not changed after committed in DescriptorSet. This caused
  an existing client's build to crash.
---
 filament/backend/src/opengl/GLDescriptorSet.h |  8 ++++++--
 filament/src/ds/DescriptorSet.cpp             | 20 +++++++++++++++++--
 filament/src/ds/DescriptorSet.h               |  1 +
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/filament/backend/src/opengl/GLDescriptorSet.h b/filament/backend/src/opengl/GLDescriptorSet.h
index 26a783b740c..51ac2afd884 100644
--- a/filament/backend/src/opengl/GLDescriptorSet.h
+++ b/filament/backend/src/opengl/GLDescriptorSet.h
@@ -78,8 +78,12 @@ struct GLDescriptorSet : public HwDescriptorSet {
 private:
     // a Buffer Descriptor such as SSBO or UBO with static offset
     struct Buffer {
-        Buffer() = default;
-        explicit Buffer(GLenum target) noexcept : target(target) { }
+        // Workaround: we cannot define the following as Buffer() = default because one of our
+        // clients has their compiler set up where such declaration (possibly coupled with explicit)
+        // will be considered a deleted constructor.
+        Buffer() {}
+
+        explicit Buffer(GLenum target) noexcept : target(target) {}
         GLenum target;                          // 4
         GLuint id = 0;                          // 4
         uint32_t offset = 0;                    // 4
diff --git a/filament/src/ds/DescriptorSet.cpp b/filament/src/ds/DescriptorSet.cpp
index 50667978408..9d41b20a33f 100644
--- a/filament/src/ds/DescriptorSet.cpp
+++ b/filament/src/ds/DescriptorSet.cpp
@@ -27,6 +27,7 @@
 
 #include <utils/compiler.h>
 #include <utils/debug.h>
+#include <utils/Log.h>
 
 #include <utility>
 #include <limits>
@@ -44,7 +45,8 @@ DescriptorSet::~DescriptorSet() noexcept {
 
 DescriptorSet::DescriptorSet(DescriptorSetLayout const& descriptorSetLayout) noexcept
         : mDescriptors(descriptorSetLayout.getMaxDescriptorBinding() + 1),
-          mDirty(std::numeric_limits<uint64_t>::max()) {
+          mDirty(std::numeric_limits<uint64_t>::max()),
+          mSetAfterCommitWarning(false) {
 }
 
 DescriptorSet::DescriptorSet(DescriptorSet&& rhs) noexcept = default;
@@ -57,6 +59,7 @@ DescriptorSet& DescriptorSet::operator=(DescriptorSet&& rhs) noexcept {
         mDescriptorSetHandle = std::move(rhs.mDescriptorSetHandle);
         mDirty = rhs.mDirty;
         mValid = rhs.mValid;
+        mSetAfterCommitWarning = rhs.mSetAfterCommitWarning;
     }
     return *this;
 }
@@ -103,8 +106,21 @@ void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints
 void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints set,
         backend::DescriptorSetOffsetArray dynamicOffsets) const noexcept {
     // TODO: on debug check that dynamicOffsets is large enough
-    assert_invariant(mDirty.none());
+
     assert_invariant(mDescriptorSetHandle);
+
+    // TODO: Make sure clients do the right thing and not change material instance parameters
+    // within the renderpass. We have to comment the assert out since it crashed a client's debug
+    // build.
+    // assert_invariant(mDirty.none());
+    if (mDirty.any() && !mSetAfterCommitWarning) {
+        mDirty.forEachSetBit([&](uint8_t binding) {
+            utils::slog.w << "Descriptor set (handle=" << mDescriptorSetHandle.getId()
+                          << ") binding=" << (int) binding
+                          << " was set between begin/endRenderPass" << utils::io::endl;
+        });
+        mSetAfterCommitWarning = true;
+    }
     driver.bindDescriptorSet(mDescriptorSetHandle, +set, std::move(dynamicOffsets));
 }
 
diff --git a/filament/src/ds/DescriptorSet.h b/filament/src/ds/DescriptorSet.h
index 4b8d6d02dfa..2bfa8be6680 100644
--- a/filament/src/ds/DescriptorSet.h
+++ b/filament/src/ds/DescriptorSet.h
@@ -103,6 +103,7 @@ class DescriptorSet {
     mutable utils::bitset64 mDirty;                         //  8
     mutable utils::bitset64 mValid;                         //  8
     backend::DescriptorSetHandle mDescriptorSetHandle;      //  4
+    mutable bool mSetAfterCommitWarning;                    //  1
 };
 
 } // namespace filament