Skip to content

Commit

Permalink
Work around client descriptor set issues
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
poweifeng committed Oct 2, 2024
1 parent 4f5369c commit 28f01f4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 4 deletions.
8 changes: 6 additions & 2 deletions filament/backend/src/opengl/GLDescriptorSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions filament/src/ds/DescriptorSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include <utils/compiler.h>
#include <utils/debug.h>
#include <utils/Log.h>

#include <utility>
#include <limits>
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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));
}

Expand Down
1 change: 1 addition & 0 deletions filament/src/ds/DescriptorSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 28f01f4

Please sign in to comment.