From cb5fcbc784e37bfb20e3c64f8abe6fbed111dfaa Mon Sep 17 00:00:00 2001 From: Pixelflinger Date: Tue, 14 Apr 2020 13:05:00 -0700 Subject: [PATCH] get rid of the static list of Engines Originally it was used for validating the Engine* used, but this makes no sense (no reason to validate this pointer more than any other one). The list also made sure that Engine instances were automatically destroyed when the filament library was unloaded, but again, there is no reason to do this -- it's C++ after all! --- filament/src/Engine.cpp | 51 +------------------ filament/src/IndexBuffer.cpp | 1 - filament/src/IndirectLight.cpp | 1 - filament/src/Material.cpp | 1 - filament/src/RenderTarget.cpp | 1 - filament/src/Skybox.cpp | 1 - filament/src/Stream.cpp | 1 - filament/src/Texture.cpp | 1 - filament/src/VertexBuffer.cpp | 1 - filament/src/components/LightManager.cpp | 1 - filament/src/components/RenderableManager.cpp | 1 - filament/src/details/Engine.h | 2 - 12 files changed, 2 insertions(+), 61 deletions(-) diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index 80f3cab20f..a7aa8941a8 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -60,43 +60,6 @@ using namespace filaflat; namespace details { -// Global list of engines used for diagnostic purposes only. -// This console output in the constructor helps catch situations where the Filament library is -// loaded twice, or not at all. Note that we avoid using slog due to static initialization. -class EngineList { -public: - EngineList() { - io::LogStream cinfo(io::LogStream::Priority::INFO); - cinfo << "Filament library loaded." << io::endl; - } - void add(FEngine* instance) { - std::unique_ptr engine(instance); - std::lock_guard guard(mLock); - Engine* handle = engine.get(); - mEngines[handle] = std::move(engine); - } - std::unique_ptr remove(FEngine* instance) { - std::unique_ptr filamentEngine; - std::lock_guard guard(mLock); - auto const& pos = mEngines.find(instance); - if (pos != mEngines.end()) { - std::swap(filamentEngine, pos->second); - mEngines.erase(pos); - } - return filamentEngine; - } - bool isValid(Engine const& engine, const char* function) { - std::lock_guard guard(mLock); - auto const& pos = mEngines.find(&engine); - return pos != mEngines.end(); - } -private: - std::unordered_map> mEngines; - std::mutex mLock; -}; - -static EngineList sEngines; - FEngine* FEngine::create(Backend backend, Platform* platform, void* sharedGLContext) { FEngine* instance = new FEngine(backend, platform, sharedGLContext); @@ -133,8 +96,6 @@ FEngine* FEngine::create(Backend backend, Platform* platform, void* sharedGLCont } } - sEngines.add(instance); - // now we can initialize the largest part of the engine instance->init(); @@ -145,12 +106,6 @@ FEngine* FEngine::create(Backend backend, Platform* platform, void* sharedGLCont return instance; } -void FEngine::assertValid(Engine const& engine, const char* function) { - bool valid = sEngines.isValid(engine, function); - ASSERT_POSTCONDITION(valid, - "Using an invalid Engine instance (@ %p) from %s.", &engine, function); -} - // 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. @@ -801,10 +756,8 @@ bool FEngine::execute() { void FEngine::destroy(FEngine* engine) { if (engine) { - std::unique_ptr filamentEngine = sEngines.remove(engine); - if (filamentEngine) { - filamentEngine->shutdown(); - } + engine->shutdown(); + delete engine; } } diff --git a/filament/src/IndexBuffer.cpp b/filament/src/IndexBuffer.cpp index e8e3940e0f..1e0ed96450 100644 --- a/filament/src/IndexBuffer.cpp +++ b/filament/src/IndexBuffer.cpp @@ -48,7 +48,6 @@ IndexBuffer::Builder& IndexBuffer::Builder::bufferType(IndexType indexType) noex } IndexBuffer* IndexBuffer::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); return upcast(engine).createIndexBuffer(*this); } diff --git a/filament/src/IndirectLight.cpp b/filament/src/IndirectLight.cpp index 519198d9a5..5b403c4b21 100644 --- a/filament/src/IndirectLight.cpp +++ b/filament/src/IndirectLight.cpp @@ -149,7 +149,6 @@ IndirectLight::Builder& IndirectLight::Builder::rotation(mat3f const& rotation) } IndirectLight* IndirectLight::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); if (mImpl->mReflectionsMap) { if (!ASSERT_POSTCONDITION_NON_FATAL( mImpl->mReflectionsMap->getTarget() == Texture::Sampler::SAMPLER_CUBEMAP, diff --git a/filament/src/Material.cpp b/filament/src/Material.cpp index 09d05b11b8..3969d51b1c 100644 --- a/filament/src/Material.cpp +++ b/filament/src/Material.cpp @@ -74,7 +74,6 @@ Material::Builder& Material::Builder::package(const void* payload, size_t size) } Material* Material::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); MaterialParser* materialParser = FMaterial::createParser( upcast(engine).getBackend(), mImpl->mPayload, mImpl->mSize); diff --git a/filament/src/RenderTarget.cpp b/filament/src/RenderTarget.cpp index 32a4781eb2..597a7f2164 100644 --- a/filament/src/RenderTarget.cpp +++ b/filament/src/RenderTarget.cpp @@ -61,7 +61,6 @@ RenderTarget::Builder& RenderTarget::Builder::layer(AttachmentPoint pt, uint32_t } RenderTarget* RenderTarget::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); using backend::TextureUsage; const FRenderTarget::Attachment& color = mImpl->mAttachments[COLOR]; const FRenderTarget::Attachment& depth = mImpl->mAttachments[DEPTH]; diff --git a/filament/src/Skybox.cpp b/filament/src/Skybox.cpp index 31fc578d98..21a2832a7c 100644 --- a/filament/src/Skybox.cpp +++ b/filament/src/Skybox.cpp @@ -78,7 +78,6 @@ Skybox::Builder& Skybox::Builder::showSun(bool show) noexcept { } Skybox* Skybox::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); FTexture* cubemap = upcast(mImpl->mEnvironmentMap); if (!ASSERT_PRECONDITION_NON_FATAL(!cubemap || cubemap->isCubemap(), diff --git a/filament/src/Stream.cpp b/filament/src/Stream.cpp index 923ee72d78..08c543fd8a 100644 --- a/filament/src/Stream.cpp +++ b/filament/src/Stream.cpp @@ -69,7 +69,6 @@ Stream::Builder& Stream::Builder::height(uint32_t height) noexcept { } Stream* Stream::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); if (!ASSERT_PRECONDITION_NON_FATAL(!mImpl->mStream || !mImpl->mExternalTextureId, "One and only one of the stream or external texture can be specified")) { return nullptr; diff --git a/filament/src/Texture.cpp b/filament/src/Texture.cpp index 4c06b6539b..fb98940007 100644 --- a/filament/src/Texture.cpp +++ b/filament/src/Texture.cpp @@ -110,7 +110,6 @@ Texture::Builder& Texture::Builder::swizzle(Swizzle r, Swizzle g, Swizzle b, Swi } Texture* Texture::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); if (!ASSERT_POSTCONDITION_NON_FATAL(Texture::isTextureFormatSupported(engine, mImpl->mFormat), "Texture format %u not supported on this platform", mImpl->mFormat)) { return nullptr; diff --git a/filament/src/VertexBuffer.cpp b/filament/src/VertexBuffer.cpp index b947e6740e..d3c3873255 100644 --- a/filament/src/VertexBuffer.cpp +++ b/filament/src/VertexBuffer.cpp @@ -111,7 +111,6 @@ VertexBuffer::Builder& VertexBuffer::Builder::normalized(VertexAttribute attribu } VertexBuffer* VertexBuffer::Builder::build(Engine& engine) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); if (!ASSERT_PRECONDITION_NON_FATAL(mImpl->mVertexCount > 0, "vertexCount cannot be 0")) { return nullptr; } diff --git a/filament/src/components/LightManager.cpp b/filament/src/components/LightManager.cpp index a5a0fc6ee0..d5b355de2b 100644 --- a/filament/src/components/LightManager.cpp +++ b/filament/src/components/LightManager.cpp @@ -128,7 +128,6 @@ LightManager::Builder& LightManager::Builder::sunHaloFalloff(float haloFalloff) } LightManager::Builder::Result LightManager::Builder::build(Engine& engine, Entity entity) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); upcast(engine).createLight(*this, entity); return Success; } diff --git a/filament/src/components/RenderableManager.cpp b/filament/src/components/RenderableManager.cpp index b900c7ebe0..c0ba5c2188 100644 --- a/filament/src/components/RenderableManager.cpp +++ b/filament/src/components/RenderableManager.cpp @@ -173,7 +173,6 @@ RenderableManager::Builder& RenderableManager::Builder::blendOrder(size_t index, } RenderableManager::Builder::Result RenderableManager::Builder::build(Engine& engine, Entity entity) { - FEngine::assertValid(engine, __PRETTY_FUNCTION__); bool isEmpty = true; if (!ASSERT_PRECONDITION_NON_FATAL(mImpl->mSkinningBoneCount <= CONFIG_MAX_BONE_COUNT, diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index 4b92d1bcb3..4613cb0a92 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -135,8 +135,6 @@ public: static void destroy(FEngine* engine); - static void assertValid(Engine const& engine, const char* function); - ~FEngine() noexcept; backend::Driver& getDriver() const noexcept { return *mDriver; }