From ac29bb509496f2a16a51aaabe2ab85f45cdd91d7 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 5 Aug 2025 13:48:13 -0700 Subject: [PATCH] utils: add precond/postcond macro guarded by feature flags We link the behavior of certain precondition/postcondition checks to feature flag states. - If provided *flag* is true, then this macro will assert when the *condition* is false. - If provided *flag* is fase, then this macro will output a warning when the condition is false. - If the condition is true, then neither of the above will happen These two macros will enable us to provide less restrictive behavior for certain correctness assertions, allowing clients to modify their before enabling these assertions by default. --- filament/src/details/Engine.cpp | 21 ++++++---------- filament/src/details/Material.cpp | 15 ++++-------- filament/src/details/VertexBuffer.cpp | 24 +++++++----------- libs/utils/include/utils/Panic.h | 35 +++++++++++++++++++++++++++ libs/utils/src/Panic.cpp | 12 +++++++++ 5 files changed, 68 insertions(+), 39 deletions(-) diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 780521aa11..58fc61a6d0 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -1235,20 +1235,13 @@ bool FEngine::destroy(const FMaterialInstance* p) { size_t const primitiveCount = rcm.getPrimitiveCount(ri, 0); for (size_t j = 0; j < primitiveCount; j++) { auto const* const mi = rcm.getMaterialInstanceAt(ri, 0, j); - if (features.engine.debug.assert_material_instance_in_use) { - FILAMENT_CHECK_PRECONDITION(mi != p) - << "destroying MaterialInstance \"" - << mi->getName() << "\" which is still in use by Renderable (entity=" - << entity.getId() << ", instance=" - << ri.asValue() << ", index=" << j << ")"; - } else { - if (UTILS_UNLIKELY(mi == p)) { - LOG(ERROR) << "destroying MaterialInstance \"" << mi->getName() - << "\" which is still in use by Renderable (entity=" - << entity.getId() << ", instance=" << ri.asValue() - << ", index=" << j << ")"; - } - } + auto const& featureFlags = features.engine.debug; + FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION( + mi != p, + featureFlags.assert_material_instance_in_use) + << "destroying MaterialInstance \"" << mi->getName() + << "\" which is still in use by Renderable (entity=" << entity.getId() + << ", instance=" << ri.asValue() << ", index=" << j << ")"; } } } diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index dc05340988..ebffc1a19f 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -391,16 +391,11 @@ void FMaterial::terminate(FEngine& engine) { auto const& materialInstanceResourceList = engine.getMaterialInstanceResourceList(); auto pos = materialInstanceResourceList.find(this); if (UTILS_LIKELY(pos != materialInstanceResourceList.cend())) { - if (engine.features.engine.debug.assert_destroy_material_before_material_instance) { - FILAMENT_CHECK_PRECONDITION(pos->second.empty()) - << "destroying material \"" << this->getName().c_str_safe() << "\" but " - << pos->second.size() << " instances still alive."; - } else { - if (UTILS_UNLIKELY(!pos->second.empty())) { - LOG(ERROR) << "destroying material \"" << this->getName().c_str_safe() << "\" but " - << pos->second.size() << " instances still alive."; - } - } + auto const& featureFlags = engine.features.engine.debug; + FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION(pos->second.empty(), + featureFlags.assert_destroy_material_before_material_instance) + << "destroying material \"" << this->getName().c_str_safe() << "\" but " + << pos->second.size() << " instances still alive."; } #if FILAMENT_ENABLE_MATDBG diff --git a/filament/src/details/VertexBuffer.cpp b/filament/src/details/VertexBuffer.cpp index 5e38abef6e..631829789a 100644 --- a/filament/src/details/VertexBuffer.cpp +++ b/filament/src/details/VertexBuffer.cpp @@ -158,12 +158,11 @@ VertexBuffer* VertexBuffer::Builder::build(Engine& engine) { static_assert(DOCUMENTED_MAX_VERTEX_BUFFER_COUNT <= MAX_VERTEX_BUFFER_COUNT); - if (downcast(engine).features.engine.debug.assert_vertex_buffer_count_exceeds_8) { - FILAMENT_CHECK_PRECONDITION(mImpl->mBufferCount <= DOCUMENTED_MAX_VERTEX_BUFFER_COUNT) - << "bufferCount cannot be more than " << DOCUMENTED_MAX_VERTEX_BUFFER_COUNT; - } else if (mImpl->mBufferCount > DOCUMENTED_MAX_VERTEX_BUFFER_COUNT) { - LOG(WARNING) << "bufferCount cannot be more than " << DOCUMENTED_MAX_VERTEX_BUFFER_COUNT; - } + auto const& featureFlags = static_cast(&engine)->features.engine.debug; + FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION( + mImpl->mBufferCount <= DOCUMENTED_MAX_VERTEX_BUFFER_COUNT, + featureFlags.assert_vertex_buffer_count_exceeds_8) + << "bufferCount cannot be more than " << DOCUMENTED_MAX_VERTEX_BUFFER_COUNT; // Next we check if any unused buffer slots have been allocated. This helps prevent errors // because uploading to an unused slot can trigger undefined behavior in the backend. @@ -177,15 +176,10 @@ VertexBuffer* VertexBuffer::Builder::build(Engine& engine) { << "attribute " << j << " offset=" << attributes[j].offset << " is not multiple of 4"; - FEngine* fengine = static_cast(&engine); - if (fengine->features.engine.debug.assert_vertex_buffer_attribute_stride_mult_of_4) { - FILAMENT_CHECK_PRECONDITION((attributes[j].stride & 0x3u) == 0) - << "attribute " << j << " stride=" << attributes[j].stride - << " is not multiple of 4"; - } else if ((attributes[j].stride & 0x3u) != 0) { - LOG(WARNING) << "attribute " << j << " stride=" << attributes[j].stride - << " is not multiple of 4"; - } + FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION((attributes[j].stride & 0x3u) == 0, + featureFlags.assert_vertex_buffer_attribute_stride_mult_of_4) + << "attribute " << j << " stride=" << +attributes[j].stride + << " is not multiple of 4"; if (engine.getActiveFeatureLevel() == FeatureLevel::FEATURE_LEVEL_0) { FILAMENT_CHECK_PRECONDITION(!(attributes[j].flags & Attribute::FLAG_INTEGER_TARGET)) diff --git a/libs/utils/include/utils/Panic.h b/libs/utils/include/utils/Panic.h index e3be728c38..9c613d48d0 100644 --- a/libs/utils/include/utils/Panic.h +++ b/libs/utils/include/utils/Panic.h @@ -544,6 +544,27 @@ public: } }; +template +class FlagGuardedStream : public PanicStream { +public: + FlagGuardedStream(bool enable, char const* function, char const* file, int line, + char const* condition) + : PanicStream(function, file, line, condition), + mEnablePanic(enable) {} + ~FlagGuardedStream() noexcept(false) { + if (mEnablePanic) { + PanicType::panic(mFunction, mFile, mLine, mLiteral, mStream.c_str()); + } else { + logWarning(); + } + } + +private: + void logWarning() noexcept; + + bool mEnablePanic; +}; + } // namespace details // ----------------------------------------------------------------------------------------------- @@ -582,6 +603,20 @@ public: FILAMENT_CHECK_CONDITION_IMPL(condition) FILAMENT_PANIC_IMPL(#condition, ArithmeticPanic) #endif +#define FILAMENT_FLAG_GUARDED_PANIC_IMPL(flag, condition, TYPE) \ + ::utils::details::FlagGuardedStream<::utils::TYPE>(flag, PANIC_FUNCTION, PANIC_FILE(__FILE__), \ + __LINE__, #condition) + +#ifndef FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION +#define FILAMENT_FLAG_GUARDED_CHECK_PRECONDITION(condition, flag) \ + FILAMENT_CHECK_CONDITION_IMPL(condition) FILAMENT_FLAG_GUARDED_PANIC_IMPL(flag, condition, PreconditionPanic) +#endif + +#ifndef FILAMENT_FLAG_GUARDED_CHECK_POSTCONDITION +#define FILAMENT_FLAG_GUARDED_CHECK_POSTCONDITION(condition, flag) \ + FILAMENT_CHECK_CONDITION_IMPL(condition) FILAMENT_FLAG_GUARDED_PANIC_IMPL(flag, condition, PostconditionPanic) +#endif + #define PANIC_PRECONDITION_IMPL(cond, format, ...) \ ::utils::PreconditionPanic::panic(PANIC_FUNCTION, \ PANIC_FILE(__FILE__), __LINE__, #cond, format, ##__VA_ARGS__) diff --git a/libs/utils/src/Panic.cpp b/libs/utils/src/Panic.cpp index 7c1beca324..dd7ee25493 100644 --- a/libs/utils/src/Panic.cpp +++ b/libs/utils/src/Panic.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -352,6 +353,17 @@ PanicStream& PanicStream::operator<<(std::string_view const& value) noexcept { return *this; } +template +void FlagGuardedStream::logWarning() noexcept { + LOG(WARNING) << "Failed [" << mLiteral << "]" << "\n file=" << mFile + << "\n function=" << mFunction << "\n line=" << mLine << "\n info=\"" + << mStream.c_str() << "\""; + +} + +template void FlagGuardedStream::logWarning() noexcept; +template void FlagGuardedStream::logWarning() noexcept; + } // namespace details // -----------------------------------------------------------------------------------------------