From 17664d294e39e63d3b60d2dcf3f7907b958438a3 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 5 May 2023 16:53:20 -0700 Subject: [PATCH] A new Builder-based API to create a filament Engine This doesn't add or remove functionality, but merely changes the API to create an Engine, to be more consistant with how we construct other objects in filament. You can now use Engine::Builder to construct an Engine. --- filament/include/filament/Engine.h | 198 +++++++++++++++++------------ filament/src/Engine.cpp | 9 -- filament/src/details/Engine.cpp | 193 +++++++++++++++++----------- filament/src/details/Engine.h | 14 +- filament/test/filament_test.cpp | 2 +- 5 files changed, 241 insertions(+), 175 deletions(-) diff --git a/filament/include/filament/Engine.h b/filament/include/filament/Engine.h index 18a3602116..cc1fa96f99 100644 --- a/filament/include/filament/Engine.h +++ b/filament/include/filament/Engine.h @@ -17,6 +17,8 @@ #ifndef TNT_FILAMENT_ENGINE_H #define TNT_FILAMENT_ENGINE_H +#include + #include #include @@ -165,6 +167,7 @@ class TransformManager; * @see Renderer */ class UTILS_PUBLIC Engine { + struct BuilderDetails; public: using Platform = backend::Platform; using Backend = backend::Backend; @@ -266,96 +269,124 @@ public: uint32_t perFrameCommandsSizeMB = FILAMENT_PER_FRAME_COMMANDS_SIZE_IN_MB; }; + +#if UTILS_HAS_THREADING + using CreateCallback = void(void* user, void* token); +#endif + /** - * Creates an instance of Engine - * - * @param backend Which driver backend to use. - * - * @param platform A pointer to an object that implements Platform. If this is - * provided, then this object is used to create the hardware context - * and expose platform features to it. - * - * If not provided (or nullptr is used), an appropriate Platform - * is created automatically. - * - * All methods of this interface are called from filament's - * render thread, which is different from the main thread. - * - * The lifetime of \p platform must exceed the lifetime of - * the Engine object. - * - * @param sharedGLContext A platform-dependant OpenGL context used as a shared context - * when creating filament's internal context. - * Setting this parameter will force filament to use the OpenGL - * implementation (instead of Vulkan for instance). - * - * @param config A pointer to optional parameters to specify memory size - * configuration options. If nullptr, then defaults used. - * - * @return A pointer to the newly created Engine, or nullptr if the Engine couldn't be created. - * - * nullptr if the GPU driver couldn't be initialized, for instance if it doesn't - * support the right version of OpenGL or OpenGL ES. - * - * @exception utils::PostConditionPanic can be thrown if there isn't enough memory to - * allocate the command buffer. If exceptions are disabled, this condition if fatal and - * this function will abort. - * - * \remark - * This method is thread-safe. + * Engine::Builder is used to create a new filament Engine. */ - static Engine* create(Backend backend = Backend::DEFAULT, - Platform* platform = nullptr, void* sharedGLContext = nullptr, - const Config* config = nullptr); + class Builder : public BuilderBase { + friend struct BuilderDetails; + friend class FEngine; + public: + Builder() noexcept; + Builder(Builder const& rhs) noexcept; + Builder(Builder&& rhs) noexcept; + ~Builder() noexcept; + Builder& operator=(Builder const& rhs) noexcept; + Builder& operator=(Builder&& rhs) noexcept; + + /** + * @param backend Which driver backend to use + * @return A reference to this Builder for chaining calls. + */ + Builder& backend(Backend backend) noexcept; + + /** + * @param platform A pointer to an object that implements Platform. If this is + * provided, then this object is used to create the hardware context + * and expose platform features to it. + * + * If not provided (or nullptr is used), an appropriate Platform + * is created automatically. + * + * All methods of this interface are called from filament's + * render thread, which is different from the main thread. + * + * The lifetime of \p platform must exceed the lifetime of + * the Engine object. + * + * @return A reference to this Builder for chaining calls. + */ + Builder& platform(Platform* platform) noexcept; + + /** + * @param config A pointer to optional parameters to specify memory size + * configuration options. If nullptr, then defaults used. + * + * @return A reference to this Builder for chaining calls. + */ + Builder& config(const Config* config) noexcept; + + /** + * @param sharedContext A platform-dependant context used as a shared context + * when creating filament's internal context. + * + * @return A reference to this Builder for chaining calls. + */ + Builder& sharedContext(void* sharedContext) noexcept; + +#if UTILS_HAS_THREADING + /** + * Creates the filament Engine asynchronously. + * + * @param callback Callback called once the engine is initialized and it is safe to + * call Engine::getEngine(). + */ + void build(utils::Invocable&& callback) const; +#endif + + /** + * Creates an instance of Engine. + * + * @return A pointer to the newly created Engine, or nullptr if the Engine couldn't be + * created. + * nullptr if the GPU driver couldn't be initialized, for instance if it doesn't + * support the right version of OpenGL or OpenGL ES. + * + * @exception utils::PostConditionPanic can be thrown if there isn't enough memory to + * allocate the command buffer. If exceptions are disabled, this condition if + * fatal and this function will abort. + */ + Engine* build() const; + }; + + /** + * Backward compatibility helper to create an Engine. + * @see Builder + */ + static inline Engine* create(Backend backend = Backend::DEFAULT, + Platform* platform = nullptr, void* sharedContext = nullptr, + const Config* config = nullptr) { + return Engine::Builder() + .backend(backend) + .platform(platform) + .sharedContext(sharedContext) + .config(config) + .build(); + } + #if UTILS_HAS_THREADING /** - * A callback used with Engine::createAsync() called once the engine is initialized and it is - * safe to call Engine::getEngine(token). This callback is invoked from an arbitrary worker - * thread. Engine::getEngine() CANNOT be called from that thread, instead it must be called - * from the same thread than Engine::createAsync() was called from. - * - * @param user User provided parameter given in createAsync(). - * - * @param token An opaque token used to call Engine::getEngine(). + * Backward compatibility helper to create an Engine asynchronously. + * @see Builder */ - using CreateCallback = void(void* user, void* token); - - /** - * Creates an instance of Engine asynchronously - * - * @param callback Callback called once the engine is initialized and it is safe to - * call Engine::getEngine. - * - * @param user A user provided pointer that is given back to callback unmodified. - * - * @param backend Which driver backend to use. - * - * @param platform A pointer to an object that implements Platform. If this is - * provided, then this object is used to create the hardware context - * and expose platform features to it. - * - * If not provided (or nullptr is used), an appropriate Platform - * is created automatically. - * - * All methods of this interface are called from filament's - * render thread, which is different from the main thread. - * - * The lifetime of \p platform must exceed the lifetime of - * the Engine object. - * - * @param sharedGLContext A platform-dependant OpenGL context used as a shared context - * when creating filament's internal context. - * Setting this parameter will force filament to use the OpenGL - * implementation (instead of Vulkan for instance). - * - * @param config A pointer to optional parameters to specify memory size - * configuration options - */ - static void createAsync(CreateCallback callback, void* user, + static inline void createAsync(CreateCallback callback, void* user, Backend backend = Backend::DEFAULT, - Platform* platform = nullptr, void* sharedGLContext = nullptr, - const Config* config = nullptr); + Platform* platform = nullptr, void* sharedContext = nullptr, + const Config* config = nullptr) { + Engine::Builder() + .backend(backend) + .platform(platform) + .sharedContext(sharedContext) + .config(config) + .build([callback, user](void* token) { + callback(user, token); + }); + } /** * Retrieve an Engine* from createAsync(). This must be called from the same thread than @@ -372,6 +403,7 @@ public: static Engine* getEngine(void* token); #endif + /** * Destroy the Engine instance and all associated resources. * diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index ddc8184a54..cdceb3c9e9 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -46,20 +46,11 @@ namespace filament { using namespace math; using namespace backend; -Engine* Engine::create(Backend backend, Platform* platform, void* sharedGLContext, const Config* config) { - return FEngine::create(backend, platform, sharedGLContext, config); -} - void Engine::destroy(Engine* engine) { FEngine::destroy(downcast(engine)); } #if UTILS_HAS_THREADING -void Engine::createAsync(Engine::CreateCallback callback, void* user, Backend backend, - Platform* platform, void* sharedGLContext, const Config* config) { - FEngine::createAsync(callback, user, backend, platform, sharedGLContext, config); -} - Engine* Engine::getEngine(void* token) { return FEngine::getEngine(token); } diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 4f2fa0eb28..3206d1ec70 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -36,8 +36,6 @@ #include "details/VertexBuffer.h" #include "details/View.h" -#include - #include #include @@ -48,6 +46,7 @@ #include #include #include +#include #include #include @@ -64,13 +63,19 @@ namespace filament { using namespace backend; using namespace filaflat; -FEngine* FEngine::create(Backend backend, Platform* platform, - void* sharedGLContext, const Config *pConfig) { +struct Engine::BuilderDetails { + Backend mBackend = Backend::DEFAULT; + Platform* mPlatform = nullptr; + Engine::Config mConfig; + void* mSharedContext = nullptr; + static Config validateConfig(const Config* pConfig) noexcept; +}; + +Engine* FEngine::create(Engine::Builder const& builder) { SYSTRACE_ENABLE(); SYSTRACE_CALL(); - const Config config{ validateConfig(pConfig) }; - FEngine* instance = new FEngine(backend, platform, config, sharedGLContext); + FEngine* instance = new FEngine(builder); // initialize all fields that need an instance of FEngine // (this cannot be done safely in the ctor) @@ -78,6 +83,9 @@ FEngine* FEngine::create(Backend backend, Platform* platform, // Normally we launch a thread and create the context and Driver from there (see FEngine::loop). // In the single-threaded case, we do so in the here and now. if (!UTILS_HAS_THREADING) { + Platform* platform = builder->mPlatform; + void* const sharedContext = builder->mSharedContext; + if (platform == nullptr) { platform = PlatformFactory::create(&instance->mBackend); instance->mPlatform = platform; @@ -88,8 +96,9 @@ FEngine* FEngine::create(Backend backend, Platform* platform, delete instance; return nullptr; } - DriverConfig driverConfig{ .handleArenaSize = instance->getRequestedDriverHandleArenaSize() }; - instance->mDriver = platform->createDriver(sharedGLContext, driverConfig); + DriverConfig const driverConfig{ + .handleArenaSize = instance->getRequestedDriverHandleArenaSize() }; + instance->mDriver = platform->createDriver(sharedContext, driverConfig); } else { // start the driver thread @@ -118,20 +127,19 @@ FEngine* FEngine::create(Backend backend, Platform* platform, #if UTILS_HAS_THREADING -void FEngine::createAsync(CreateCallback callback, void* user, - Backend backend, Platform* platform, void* sharedGLContext, const Config* config) { +void FEngine::create(Engine::Builder const& builder, Invocable&& callback) { SYSTRACE_ENABLE(); SYSTRACE_CALL(); - Config validConfig = validateConfig(config); - FEngine* instance = new FEngine(backend, platform, validConfig, sharedGLContext); + + FEngine* instance = new FEngine(builder); // start the driver thread instance->mDriverThread = std::thread(&FEngine::loop, instance); // launch a thread to call the callback -- so it can't do any damage. - std::thread callbackThread = std::thread([instance, callback, user]() { + std::thread callbackThread = std::thread([instance, callback = std::move(callback)]() { instance->mDriverBarrier.await(); - callback(user, instance); + callback(instance); }); // let the callback thread die on its own @@ -163,48 +171,6 @@ FEngine* FEngine::getEngine(void* token) { #endif -Engine::Config FEngine::validateConfig(const Config* const pConfig) noexcept -{ - // Rule of thumb: perRenderPassArenaMB must be roughly 1 MB larger than perFrameCommandsMB - constexpr uint32_t COMMAND_ARENA_OVERHEAD = 1; - constexpr uint32_t CONCURRENT_FRAME_COUNT = 3; - - Config config; - if (!pConfig) { - return config; - } - - // make sure to copy all the fields - config = *pConfig; - - // Use at least the defaults set by the build system - config.minCommandBufferSizeMB = std::max( - config.minCommandBufferSizeMB, - (uint32_t)FILAMENT_MIN_COMMAND_BUFFERS_SIZE_IN_MB); - - config.perFrameCommandsSizeMB = std::max( - config.perFrameCommandsSizeMB, - (uint32_t)FILAMENT_PER_FRAME_COMMANDS_SIZE_IN_MB); - - config.perRenderPassArenaSizeMB = std::max( - config.perRenderPassArenaSizeMB, - (uint32_t)FILAMENT_PER_RENDER_PASS_ARENA_SIZE_IN_MB); - - config.commandBufferSizeMB = std::max( - config.commandBufferSizeMB, - config.minCommandBufferSizeMB * CONCURRENT_FRAME_COUNT); - - // Enforce pre-render-pass arena rule-of-thumb - config.perRenderPassArenaSizeMB = std::max( - config.perRenderPassArenaSizeMB, - config.perFrameCommandsSizeMB + COMMAND_ARENA_OVERHEAD); - - // This value gets validated during driver creation, so pass it through - config.driverHandleArenaSizeMB = config.driverHandleArenaSizeMB; - - return config; -} - // these must be static because only a pointer is copied to the render stream // Note that these coordinates are specified in OpenGL clip space. Other backends can transform // these in the vertex shader as needed. @@ -217,24 +183,28 @@ static constexpr float4 sFullScreenTriangleVertices[3] = { // these must be static because only a pointer is copied to the render stream static const uint16_t sFullScreenTriangleIndices[3] = { 0, 1, 2 }; -FEngine::FEngine(Backend backend, Platform* platform, const Config& config, void* sharedGLContext) : - mBackend(backend), - mPlatform(platform), - mSharedGLContext(sharedGLContext), +FEngine::FEngine(Engine::Builder const& builder) : + mBackend(builder->mBackend), + mPlatform(builder->mPlatform), + mSharedGLContext(builder->mSharedContext), mPostProcessManager(*this), mEntityManager(EntityManager::get()), mRenderableManager(*this), mTransformManager(), mLightManager(*this), mCameraManager(*this), - mCommandBufferQueue(config.minCommandBufferSizeMB * MiB, config.commandBufferSizeMB * MiB), - mPerRenderPassAllocator("FEngine::mPerRenderPassAllocator", config.perRenderPassArenaSizeMB * MiB), + mCommandBufferQueue( + builder->mConfig.minCommandBufferSizeMB * MiB, + builder->mConfig.commandBufferSizeMB * MiB), + mPerRenderPassAllocator( + "FEngine::mPerRenderPassAllocator", + builder->mConfig.perRenderPassArenaSizeMB * MiB), mHeapAllocator("FEngine::mHeapAllocator", AreaPolicy::NullArea{}), mJobSystem(getJobSystemThreadPoolSize()), mEngineEpoch(std::chrono::steady_clock::now()), mDriverBarrier(1), mMainThreadId(ThreadUtils::getThreadId()), - mConfig(config) + mConfig(builder->mConfig) { // we're assuming we're on the main thread here. // (it may not be the case) @@ -411,8 +381,8 @@ void FEngine::shutdown() { #ifndef NDEBUG // print out some statistics about this run - size_t wm = mCommandBufferQueue.getHighWatermark(); - size_t wmpct = wm / (getCommandBufferSize() / 100); + size_t const wm = mCommandBufferQueue.getHighWatermark(); + size_t const wmpct = wm / (getCommandBufferSize() / 100); slog.d << "CircularBuffer: High watermark " << wm / 1024 << " KiB (" << wmpct << "%)" << io::endl; #endif @@ -637,7 +607,7 @@ int FEngine::loop() { JobSystem::setThreadName("FEngine::loop"); JobSystem::setThreadPriority(JobSystem::Priority::DISPLAY); - DriverConfig driverConfig { .handleArenaSize = getRequestedDriverHandleArenaSize() }; + DriverConfig const driverConfig { .handleArenaSize = getRequestedDriverHandleArenaSize() }; mDriver = mPlatform->createDriver(mSharedGLContext, driverConfig); mDriverBarrier.latch(); @@ -651,7 +621,7 @@ int FEngine::loop() { // configuration. This is also a core not used by the JobSystem. // Either way the main reason to do this is to avoid this thread jumping from core to core // and lose its caches in the process. - uint32_t id = std::thread::hardware_concurrency() - 1; + uint32_t const id = std::thread::hardware_concurrency() - 1; while (true) { // looks like thread affinity needs to be reset regularly (on Android) @@ -792,7 +762,7 @@ FView* FEngine::createView() noexcept { FFence* FEngine::createFence(FFence::Type type) noexcept { FFence* p = mHeapAllocator.make(*this, type); if (p) { - std::lock_guard guard(mFenceListLock); + std::lock_guard const guard(mFenceListLock); mFences.insert(p); } return p; @@ -886,7 +856,7 @@ template UTILS_ALWAYS_INLINE inline bool FEngine::terminateAndDestroy(const T* ptr, ResourceList& list) { if (ptr == nullptr) return true; - bool success = list.remove(ptr); + bool const success = list.remove(ptr); #if UTILS_HAS_RTTI auto typeName = CallStack::typeName(); @@ -908,7 +878,7 @@ UTILS_ALWAYS_INLINE inline bool FEngine::terminateAndDestroyLocked(Lock& lock, const T* ptr, ResourceList& list) { if (ptr == nullptr) return true; lock.lock(); - bool success = list.remove(ptr); + bool const success = list.remove(ptr); lock.unlock(); #if UTILS_HAS_RTTI @@ -1058,7 +1028,6 @@ void* FEngine::streamAlloc(size_t size, size_t alignment) noexcept { } bool FEngine::execute() { - // wait until we get command buffers to be executed (or thread exit requested) auto buffers = mCommandBufferQueue.waitForCommands(); if (UTILS_UNLIKELY(buffers.empty())) { @@ -1100,4 +1069,86 @@ void FEngine::resetBackendState() noexcept { } #endif +// ------------------------------------------------------------------------------------------------ + +Engine::Builder::Builder() noexcept = default; +Engine::Builder::~Builder() noexcept = default; +Engine::Builder::Builder(Engine::Builder const& rhs) noexcept = default; +Engine::Builder::Builder(Engine::Builder&& rhs) noexcept = default; +Engine::Builder& Engine::Builder::operator=(Engine::Builder const& rhs) noexcept = default; +Engine::Builder& Engine::Builder::operator=(Engine::Builder&& rhs) noexcept = default; + +Engine::Builder& Engine::Builder::backend(Backend backend) noexcept { + mImpl->mBackend = backend; + return *this; +} + +Engine::Builder& Engine::Builder::platform(Platform* platform) noexcept { + mImpl->mPlatform = platform; + return *this; +} + +Engine::Builder& Engine::Builder::config(Engine::Config const* config) noexcept { + mImpl->mConfig = BuilderDetails::validateConfig(config); + return *this; +} + +Engine::Builder& Engine::Builder::sharedContext(void* sharedContext) noexcept { + mImpl->mSharedContext = sharedContext; + return *this; +} + +#if UTILS_HAS_THREADING + +void Engine::Builder::build(Invocable&& callback) const { + FEngine::create(*this, std::move(callback)); +} + +#endif + +Engine* Engine::Builder::build() const { + return FEngine::create(*this); +} + +Engine::Config Engine::BuilderDetails::validateConfig(const Config* const pConfig) noexcept { + // Rule of thumb: perRenderPassArenaMB must be roughly 1 MB larger than perFrameCommandsMB + constexpr uint32_t COMMAND_ARENA_OVERHEAD = 1; + constexpr uint32_t CONCURRENT_FRAME_COUNT = 3; + + Config config; + if (!pConfig) { + return config; + } + + // make sure to copy all the fields + config = *pConfig; + + // Use at least the defaults set by the build system + config.minCommandBufferSizeMB = std::max( + config.minCommandBufferSizeMB, + (uint32_t)FILAMENT_MIN_COMMAND_BUFFERS_SIZE_IN_MB); + + config.perFrameCommandsSizeMB = std::max( + config.perFrameCommandsSizeMB, + (uint32_t)FILAMENT_PER_FRAME_COMMANDS_SIZE_IN_MB); + + config.perRenderPassArenaSizeMB = std::max( + config.perRenderPassArenaSizeMB, + (uint32_t)FILAMENT_PER_RENDER_PASS_ARENA_SIZE_IN_MB); + + config.commandBufferSizeMB = std::max( + config.commandBufferSizeMB, + config.minCommandBufferSizeMB * CONCURRENT_FRAME_COUNT); + + // Enforce pre-render-pass arena rule-of-thumb + config.perRenderPassArenaSizeMB = std::max( + config.perRenderPassArenaSizeMB, + config.perFrameCommandsSizeMB + COMMAND_ARENA_OVERHEAD); + + // This value gets validated during driver creation, so pass it through + config.driverHandleArenaSizeMB = config.driverHandleArenaSizeMB; + + return config; +} + } // namespace filament diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index 9353b92e67..8724950aa0 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -120,16 +120,10 @@ public: using duration = clock::duration; public: - static FEngine* create(Backend backend = Backend::DEFAULT, - Platform* platform = nullptr, void* sharedGLContext = nullptr, - const Config* pConfig = nullptr); + static Engine* create(Builder const& builder); #if UTILS_HAS_THREADING - static void createAsync(CreateCallback callback, void* user, - Backend backend = Backend::DEFAULT, - Platform* platform = nullptr, void* sharedGLContext = nullptr, - const Config* config = nullptr); - + static void create(Builder const& builder, utils::Invocable&& callback); static FEngine* getEngine(void* token); #endif @@ -385,9 +379,7 @@ public: #endif private: - static Config validateConfig(const Config* pConfig) noexcept; - - FEngine(Backend backend, Platform* platform, const Config& config, void* sharedGLContext); + explicit FEngine(Engine::Builder const& builder); void init(); void shutdown(); diff --git a/filament/test/filament_test.cpp b/filament/test/filament_test.cpp index 790be957a6..0b942832cc 100644 --- a/filament/test/filament_test.cpp +++ b/filament/test/filament_test.cpp @@ -640,7 +640,7 @@ TEST(FilamentTest, ColorConversion) { TEST(FilamentTest, FroxelData) { using namespace filament; - FEngine* engine = FEngine::create(); + FEngine* engine = downcast(Engine::create()); LinearAllocatorArena arena("FRenderer: per-frame allocator", 3 * 1024 * 1024); utils::ArenaScope scope(arena);