Skip to content

Commit c6e2090

Browse files
committed
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.
1 parent d6b1efd commit c6e2090

File tree

3 files changed

+25
-4
lines changed

3 files changed

+25
-4
lines changed

filament/backend/src/opengl/GLDescriptorSet.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,12 @@ struct GLDescriptorSet : public HwDescriptorSet {
7878
private:
7979
// a Buffer Descriptor such as SSBO or UBO with static offset
8080
struct Buffer {
81-
Buffer() = default;
82-
explicit Buffer(GLenum target) noexcept : target(target) { }
81+
// Workaround: we cannot define the following as Buffer() = default because one of our
82+
// clients has their compiler set up where such declaration (possibly coupled with explicit)
83+
// will be considered a deleted constructor.
84+
Buffer() {}
85+
86+
explicit Buffer(GLenum target) noexcept : target(target) {}
8387
GLenum target; // 4
8488
GLuint id = 0; // 4
8589
uint32_t offset = 0; // 4

filament/src/ds/DescriptorSet.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <utils/compiler.h>
2929
#include <utils/debug.h>
30+
#include <utils/Log.h>
3031

3132
#include <utility>
3233
#include <limits>
@@ -44,7 +45,8 @@ DescriptorSet::~DescriptorSet() noexcept {
4445

4546
DescriptorSet::DescriptorSet(DescriptorSetLayout const& descriptorSetLayout) noexcept
4647
: mDescriptors(descriptorSetLayout.getMaxDescriptorBinding() + 1),
47-
mDirty(std::numeric_limits<uint64_t>::max()) {
48+
mDirty(std::numeric_limits<uint64_t>::max()),
49+
mSetAfterCommitWarning(false) {
4850
}
4951

5052
DescriptorSet::DescriptorSet(DescriptorSet&& rhs) noexcept = default;
@@ -57,6 +59,7 @@ DescriptorSet& DescriptorSet::operator=(DescriptorSet&& rhs) noexcept {
5759
mDescriptorSetHandle = std::move(rhs.mDescriptorSetHandle);
5860
mDirty = rhs.mDirty;
5961
mValid = rhs.mValid;
62+
mSetAfterCommitWarning = rhs.mSetAfterCommitWarning;
6063
}
6164
return *this;
6265
}
@@ -103,8 +106,21 @@ void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints
103106
void DescriptorSet::bind(FEngine::DriverApi& driver, DescriptorSetBindingPoints set,
104107
backend::DescriptorSetOffsetArray dynamicOffsets) const noexcept {
105108
// TODO: on debug check that dynamicOffsets is large enough
106-
assert_invariant(mDirty.none());
109+
107110
assert_invariant(mDescriptorSetHandle);
111+
112+
// TODO: Make sure clients do the right thing and not change material instance parameters
113+
// within the renderpass. We have to comment the assert out since it crashed a client's debug
114+
// build.
115+
// assert_invariant(mDirty.none());
116+
if (mDirty.any() && !mSetAfterCommitWarning) {
117+
mDirty.forEachSetBit([&](uint8_t binding) {
118+
utils::slog.w << "Descriptor set (handle=" << mDescriptorSetHandle.getId()
119+
<< ") binding=" << (int) binding
120+
<< " was set between begin/endRenderPass" << utils::io::endl;
121+
});
122+
mSetAfterCommitWarning = true;
123+
}
108124
driver.bindDescriptorSet(mDescriptorSetHandle, +set, std::move(dynamicOffsets));
109125
}
110126

filament/src/ds/DescriptorSet.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class DescriptorSet {
103103
mutable utils::bitset64 mDirty; // 8
104104
mutable utils::bitset64 mValid; // 8
105105
backend::DescriptorSetHandle mDescriptorSetHandle; // 4
106+
mutable bool mSetAfterCommitWarning; // 1
106107
};
107108

108109
} // namespace filament

0 commit comments

Comments
 (0)