From 574b6b9014978aa3c73cf216e081bbaf4963dba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Wed, 22 Jul 2026 22:07:00 -0300 Subject: [PATCH] move nine-patches into scoped catalogs Return shared handles from NinePatch creation and remove the process-wide NinePatchManager singleton. Extend ResourceCatalog with typed drawable-source ownership and expose drawable publication, lookup, and removal through ResourceScope. Preserve separate texture and drawable namespaces for direct typed lookup. Give each UITheme its own resource catalog and have scene-owned theme managers explicitly import and remove theme catalogs. Handle registered themes and default-only themes without leaking stale resource visibility between scenes. Update theme loading, Engine teardown, platform source manifests, and resource ownership tests for the catalog-based model. Document that remaining singleton resource managers must migrate to naturally owned catalogs and explicit scope imports instead of becoming new process-global semantic namespaces. --- .../resource_shared_ownership_architecture.md | 19 ++++- include/eepp/graphics.hpp | 1 - include/eepp/graphics/ninepatch.hpp | 16 ++-- include/eepp/graphics/ninepatchmanager.hpp | 21 ----- include/eepp/graphics/resourcecatalog.hpp | 8 ++ include/eepp/graphics/resourcescope.hpp | 6 ++ include/eepp/ui/uitheme.hpp | 4 + include/eepp/ui/uithememanager.hpp | 12 +++ projects/linux/ee.files | 2 - projects/macos/ee.files | 2 - projects/windows/ee.files | 2 - src/eepp/graphics/ninepatch.cpp | 21 ++--- src/eepp/graphics/ninepatchmanager.cpp | 9 --- src/eepp/graphics/resourcecatalog.cpp | 64 ++++++++++++++- src/eepp/graphics/resourcescope.cpp | 43 ++++++++-- src/eepp/ui/uiscenenode.cpp | 2 + src/eepp/ui/uitheme.cpp | 17 ++-- src/eepp/ui/uithememanager.cpp | 60 +++++++++++++- src/eepp/window/engine.cpp | 3 - .../resource_prerequisite_tests.cpp | 78 ++++++++++++++++++- 20 files changed, 314 insertions(+), 76 deletions(-) delete mode 100644 include/eepp/graphics/ninepatchmanager.hpp delete mode 100644 src/eepp/graphics/ninepatchmanager.cpp diff --git a/.agent/plans/resource_shared_ownership_architecture.md b/.agent/plans/resource_shared_ownership_architecture.md index f263f34a2..789d2fe16 100644 --- a/.agent/plans/resource_shared_ownership_architecture.md +++ b/.agent/plans/resource_shared_ownership_architecture.md @@ -790,6 +790,17 @@ Exit criteria: ### Stage 7: remaining resource families +Status: in progress. Nine-patches are the first migrated family: `NinePatch::New()` returns a +`NinePatchPtr`, theme-owned `ResourceCatalog` instances retain their named sources, and scene +`ResourceScope` imports make those sources visible intentionally. `NinePatchManager` was removed. +Removing a catalog entry releases only catalog ownership and leaves retained consumers valid. + +This is the required pattern for the remaining process-wide singleton resource managers. A +singleton must not be modernized into another process-global semantic namespace. Each family moves +to ordinary catalogs owned by its application, scene, theme, document, or other natural lifetime +boundary. `globalResourceCatalog()` is reserved for resources deliberately published process-wide; +scene scopes see non-global resources only through their local catalog or explicit imports. + Migrate fonts, font faces/fallback caches, themes, shader programs/shaders, nine-patch catalogs, atlas managers, and every remaining raw-owning ResourceManager subclass one family at a time. Their self-contained GPU objects retain the established graphics-thread destruction contract unless a @@ -860,6 +871,8 @@ Remove raw-owning `ResourceManager` only when no subclass or consumer depends ## 12. Next implementation deliverable -Stage 7 migrates the remaining ResourceManager families one at a time: fonts and font caches, -themes/icons, shaders/programs, nine-patches, atlases, and any remaining raw-owning manager. The -raw-owning ResourceManager template is removed only after its final consumer is migrated. +Stage 7 continues with the remaining ResourceManager families one at a time: fonts and font caches, +themes/icons, shaders/programs, atlases, and any remaining raw-owning manager. Each singleton +semantic namespace is replaced by naturally owned catalogs plus explicit scope imports, following +the completed nine-patch migration. The raw-owning ResourceManager template is removed only after +its final consumer is migrated. diff --git a/include/eepp/graphics.hpp b/include/eepp/graphics.hpp index 624b55d3a..78a08125f 100644 --- a/include/eepp/graphics.hpp +++ b/include/eepp/graphics.hpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include diff --git a/include/eepp/graphics/ninepatch.hpp b/include/eepp/graphics/ninepatch.hpp index 71233271f..6c7102b1d 100644 --- a/include/eepp/graphics/ninepatch.hpp +++ b/include/eepp/graphics/ninepatch.hpp @@ -7,6 +7,10 @@ namespace EE { namespace Graphics { +class NinePatch; +using NinePatchPtr = ResourcePtr; +using NinePatchWeakPtr = ResourceWeakPtr; + class EE_API NinePatch : public DrawableResource { public: enum NinePatchSides { @@ -22,14 +26,14 @@ class EE_API NinePatch : public DrawableResource { SideCount }; - static NinePatch* New( ResourceId textureId, int left, int top, int right, int bottom, - const Float& pixelDensity = 1, const std::string& name = "" ); + static NinePatchPtr New( ResourceId textureId, int left, int top, int right, int bottom, + const Float& pixelDensity = 1, const std::string& name = "" ); - static NinePatch* New( TexturePtr tex, int left, int top, int right, int bottom, - const Float& pixelDensity = 1, const std::string& name = "" ); + static NinePatchPtr New( TexturePtr tex, int left, int top, int right, int bottom, + const Float& pixelDensity = 1, const std::string& name = "" ); - static NinePatch* New( TextureRegion* textureRegion, int left, int top, int right, int bottom, - const std::string& name = "" ); + static NinePatchPtr New( TextureRegion* textureRegion, int left, int top, int right, int bottom, + const std::string& name = "" ); NinePatch( TexturePtr tex, int left, int top, int right, int bottom, const Float& pixelDensity = 1, const std::string& name = "" ); diff --git a/include/eepp/graphics/ninepatchmanager.hpp b/include/eepp/graphics/ninepatchmanager.hpp deleted file mode 100644 index 1dd1e18d1..000000000 --- a/include/eepp/graphics/ninepatchmanager.hpp +++ /dev/null @@ -1,21 +0,0 @@ -#ifndef EE_GRAPHICS_NINEPATCHMANAGER_HPP -#define EE_GRAPHICS_NINEPATCHMANAGER_HPP - -#include -#include - -#include -#include -using namespace EE::System; - -namespace EE { namespace Graphics { - -class EE_API NinePatchManager : public ResourceManager { - SINGLETON_DECLARE_HEADERS( NinePatchManager ) - - ~NinePatchManager(); -}; - -}} // namespace EE::Graphics - -#endif diff --git a/include/eepp/graphics/resourcecatalog.hpp b/include/eepp/graphics/resourcecatalog.hpp index 82bf92ed9..a1161d7ec 100644 --- a/include/eepp/graphics/resourcecatalog.hpp +++ b/include/eepp/graphics/resourcecatalog.hpp @@ -2,6 +2,7 @@ #define EE_GRAPHICS_RESOURCECATALOG_HPP #include +#include #include #include #include @@ -18,18 +19,25 @@ class EE_API ResourceCatalog { void publish( ResourceKey key, TexturePtr texture ); void publish( std::string key, TexturePtr texture ); + void publishDrawable( ResourceKey key, DrawablePtr drawable ); + void publishDrawable( std::string key, DrawablePtr drawable ); TexturePtr findTexture( const ResourceKey& key ) const; TexturePtr findTexture( const std::string& key ) const; + DrawablePtr findDrawable( const ResourceKey& key ) const; + DrawablePtr findDrawable( const std::string& key ) const; bool erase( const ResourceKey& key ); bool erase( const std::string& key ); + bool eraseDrawable( const ResourceKey& key ); + bool eraseDrawable( const std::string& key ); void clear(); std::size_t size() const; private: mutable System::Mutex mMutex; UnorderedMap mTextures; + UnorderedMap mDrawables; }; }} // namespace EE::Graphics diff --git a/include/eepp/graphics/resourcescope.hpp b/include/eepp/graphics/resourcescope.hpp index 60752c466..65ce3220b 100644 --- a/include/eepp/graphics/resourcescope.hpp +++ b/include/eepp/graphics/resourcescope.hpp @@ -18,13 +18,19 @@ class EE_API ResourceScope { TexturePtr findTexture( const ResourceKey& key ) const; TexturePtr findTexture( const std::string& key ) const; + DrawablePtr findDrawableSource( const ResourceKey& key ) const; + DrawablePtr findDrawableSource( const std::string& key ) const; DrawablePtr findDrawable( const std::string& name, bool firstSearchSprite = false ) const; DrawablePtr findDrawable( const Uint32& id ) const; void publishLocal( ResourceKey key, TexturePtr texture ); void publishLocal( std::string key, TexturePtr texture ); + void publishLocalDrawable( ResourceKey key, DrawablePtr drawable ); + void publishLocalDrawable( std::string key, DrawablePtr drawable ); bool eraseLocal( const ResourceKey& key ); bool eraseLocal( const std::string& key ); + bool eraseLocalDrawable( const ResourceKey& key ); + bool eraseLocalDrawable( const std::string& key ); void clearLocal(); void importCatalog( ResourceCatalogPtr catalog ); diff --git a/include/eepp/ui/uitheme.hpp b/include/eepp/ui/uitheme.hpp index ecd7f9574..87df234c6 100644 --- a/include/eepp/ui/uitheme.hpp +++ b/include/eepp/ui/uitheme.hpp @@ -1,6 +1,7 @@ #ifndef EE_UICUITHEME_HPP #define EE_UICUITHEME_HPP +#include #include #include #include @@ -87,6 +88,8 @@ class EE_API UITheme : protected ResourceManagerMulti { UIIconTheme* getIconTheme() const; + const Graphics::ResourceCatalogPtr& getResourceCatalog() const; + const std::string& getStyleSheetPath() const; void setStyleSheetPath( const std::string& styleSheetPath ); @@ -103,6 +106,7 @@ class EE_API UITheme : protected ResourceManagerMulti { CSS::StyleSheet mStyleSheet; std::string mStyleSheetPath; UIIconTheme* mIconTheme; + Graphics::ResourceCatalogPtr mResourceCatalog; void setTextureAtlas( Graphics::TextureAtlas* SG ); diff --git a/include/eepp/ui/uithememanager.hpp b/include/eepp/ui/uithememanager.hpp index 1bf4ef441..f37da5033 100644 --- a/include/eepp/ui/uithememanager.hpp +++ b/include/eepp/ui/uithememanager.hpp @@ -1,6 +1,7 @@ #ifndef EE_UICTHEMEMANAGER #define EE_UICTHEMEMANAGER +#include #include #include @@ -14,6 +15,16 @@ class EE_API UIThemeManager : public ResourceManager { virtual ~UIThemeManager(); + virtual UITheme* add( UITheme* theme ); + + bool remove( UITheme* theme, bool destroy = true ); + + bool removeById( const String::HashType& id, bool destroy = true ); + + bool removeByName( const std::string& name, bool destroy = true ); + + UIThemeManager* setResourceScope( Graphics::ResourceScopePtr resourceScope ); + UIThemeManager* setDefaultFont( Font* Font ); Font* getDefaultFont() const; @@ -72,6 +83,7 @@ class EE_API UIThemeManager : public ResourceManager { bool mTooltipFollowMouse; Sizei mCursorSize; + Graphics::ResourceScopePtr mResourceScope; UIThemeManager(); }; diff --git a/projects/linux/ee.files b/projects/linux/ee.files index 3f333207e..5cf2d6393 100644 --- a/projects/linux/ee.files +++ b/projects/linux/ee.files @@ -74,7 +74,6 @@ ../../include/eepp/graphics/glyphdrawable.hpp ../../include/eepp/graphics/image.hpp ../../include/eepp/graphics/ninepatch.hpp -../../include/eepp/graphics/ninepatchmanager.hpp ../../include/eepp/graphics/packerhelper.hpp ../../include/eepp/graphics/particle.hpp ../../include/eepp/graphics/particlesystem.hpp @@ -574,7 +573,6 @@ ../../src/eepp/graphics/glyphdrawable.cpp ../../src/eepp/graphics/image.cpp ../../src/eepp/graphics/ninepatch.cpp -../../src/eepp/graphics/ninepatchmanager.cpp ../../src/eepp/graphics/particle.cpp ../../src/eepp/graphics/particlesystem.cpp ../../src/eepp/graphics/pixeldensity.cpp diff --git a/projects/macos/ee.files b/projects/macos/ee.files index a2525b6d2..040f1f3f1 100644 --- a/projects/macos/ee.files +++ b/projects/macos/ee.files @@ -74,7 +74,6 @@ ../../include/eepp/graphics/glyphdrawable.hpp ../../include/eepp/graphics/image.hpp ../../include/eepp/graphics/ninepatch.hpp -../../include/eepp/graphics/ninepatchmanager.hpp ../../include/eepp/graphics/packerhelper.hpp ../../include/eepp/graphics/particle.hpp ../../include/eepp/graphics/particlesystem.hpp @@ -565,7 +564,6 @@ ../../src/eepp/graphics/glyphdrawable.cpp ../../src/eepp/graphics/image.cpp ../../src/eepp/graphics/ninepatch.cpp -../../src/eepp/graphics/ninepatchmanager.cpp ../../src/eepp/graphics/particle.cpp ../../src/eepp/graphics/particlesystem.cpp ../../src/eepp/graphics/pixeldensity.cpp diff --git a/projects/windows/ee.files b/projects/windows/ee.files index f3cdd029c..a829bd017 100644 --- a/projects/windows/ee.files +++ b/projects/windows/ee.files @@ -72,7 +72,6 @@ ../../include/eepp/graphics/glyphdrawable.hpp ../../include/eepp/graphics/image.hpp ../../include/eepp/graphics/ninepatch.hpp -../../include/eepp/graphics/ninepatchmanager.hpp ../../include/eepp/graphics/packerhelper.hpp ../../include/eepp/graphics/particle.hpp ../../include/eepp/graphics/particlesystem.hpp @@ -556,7 +555,6 @@ ../../src/eepp/graphics/glyphdrawable.cpp ../../src/eepp/graphics/image.cpp ../../src/eepp/graphics/ninepatch.cpp -../../src/eepp/graphics/ninepatchmanager.cpp ../../src/eepp/graphics/particle.cpp ../../src/eepp/graphics/particlesystem.cpp ../../src/eepp/graphics/pixeldensity.cpp diff --git a/src/eepp/graphics/ninepatch.cpp b/src/eepp/graphics/ninepatch.cpp index d2a12a3b7..54fd323ca 100644 --- a/src/eepp/graphics/ninepatch.cpp +++ b/src/eepp/graphics/ninepatch.cpp @@ -4,20 +4,21 @@ namespace EE { namespace Graphics { -NinePatch* NinePatch::New( ResourceId textureId, int left, int top, int right, int bottom, - const Float& pixelDensity, const std::string& name ) { - return eeNew( NinePatch, ( TextureFactory::instance()->getTexture( textureId ), left, top, - right, bottom, pixelDensity, name ) ); +NinePatchPtr NinePatch::New( ResourceId textureId, int left, int top, int right, int bottom, + const Float& pixelDensity, const std::string& name ) { + return makeResource( TextureFactory::instance()->getTexture( textureId ), left, top, + right, bottom, pixelDensity, name ); } -NinePatch* NinePatch::New( TexturePtr tex, int left, int top, int right, int bottom, - const Float& pixelDensity, const std::string& name ) { - return eeNew( NinePatch, ( std::move( tex ), left, top, right, bottom, pixelDensity, name ) ); +NinePatchPtr NinePatch::New( TexturePtr tex, int left, int top, int right, int bottom, + const Float& pixelDensity, const std::string& name ) { + return makeResource( std::move( tex ), left, top, right, bottom, pixelDensity, + name ); } -NinePatch* NinePatch::New( TextureRegion* textureRegion, int left, int top, int right, int bottom, - const std::string& name ) { - return eeNew( NinePatch, ( textureRegion, left, top, right, bottom, name ) ); +NinePatchPtr NinePatch::New( TextureRegion* textureRegion, int left, int top, int right, int bottom, + const std::string& name ) { + return makeResource( textureRegion, left, top, right, bottom, name ); } NinePatch::NinePatch( TexturePtr tex, int left, int top, int right, int bottom, diff --git a/src/eepp/graphics/ninepatchmanager.cpp b/src/eepp/graphics/ninepatchmanager.cpp deleted file mode 100644 index 73aea7ecc..000000000 --- a/src/eepp/graphics/ninepatchmanager.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include - -namespace EE { namespace Graphics { - -SINGLETON_DECLARE_IMPLEMENTATION( NinePatchManager ) - -NinePatchManager::~NinePatchManager() {} - -}} // namespace EE::Graphics diff --git a/src/eepp/graphics/resourcecatalog.cpp b/src/eepp/graphics/resourcecatalog.cpp index 729f76ad5..c36d1ab55 100644 --- a/src/eepp/graphics/resourcecatalog.cpp +++ b/src/eepp/graphics/resourcecatalog.cpp @@ -39,6 +39,35 @@ void ResourceCatalog::publish( std::string key, TexturePtr texture ) { previous.reset(); } +void ResourceCatalog::publishDrawable( ResourceKey key, DrawablePtr drawable ) { + publishDrawable( key.value(), std::move( drawable ) ); +} + +void ResourceCatalog::publishDrawable( std::string key, DrawablePtr drawable ) { + if ( key.empty() ) + return; + + if ( !drawable ) { + eraseDrawable( key ); + return; + } + + DrawablePtr previous; + { + Lock lock( mMutex ); + auto it = mDrawables.find( key ); + if ( it == mDrawables.end() ) { + mDrawables.emplace( std::move( key ), std::move( drawable ) ); + return; + } + + previous = std::move( it->second ); + it->second = std::move( drawable ); + } + + previous.reset(); +} + TexturePtr ResourceCatalog::findTexture( const ResourceKey& key ) const { return findTexture( key.value() ); } @@ -49,6 +78,16 @@ TexturePtr ResourceCatalog::findTexture( const std::string& key ) const { return it != mTextures.end() ? it->second : TexturePtr{}; } +DrawablePtr ResourceCatalog::findDrawable( const ResourceKey& key ) const { + return findDrawable( key.value() ); +} + +DrawablePtr ResourceCatalog::findDrawable( const std::string& key ) const { + Lock lock( mMutex ); + auto it = mDrawables.find( key ); + return it != mDrawables.end() ? it->second : DrawablePtr{}; +} + bool ResourceCatalog::erase( const ResourceKey& key ) { return erase( key.value() ); } @@ -69,19 +108,42 @@ bool ResourceCatalog::erase( const std::string& key ) { return true; } +bool ResourceCatalog::eraseDrawable( const ResourceKey& key ) { + return eraseDrawable( key.value() ); +} + +bool ResourceCatalog::eraseDrawable( const std::string& key ) { + DrawablePtr drawable; + { + Lock lock( mMutex ); + auto it = mDrawables.find( key ); + if ( it == mDrawables.end() ) + return false; + + drawable = std::move( it->second ); + mDrawables.erase( it ); + } + + drawable.reset(); + return true; +} + void ResourceCatalog::clear() { UnorderedMap textures; + UnorderedMap drawables; { Lock lock( mMutex ); textures = std::move( mTextures ); + drawables = std::move( mDrawables ); } textures.clear(); + drawables.clear(); } std::size_t ResourceCatalog::size() const { Lock lock( mMutex ); - return mTextures.size(); + return mTextures.size() + mDrawables.size(); } }} // namespace EE::Graphics diff --git a/src/eepp/graphics/resourcescope.cpp b/src/eepp/graphics/resourcescope.cpp index 8f5d64597..8ef9b05e9 100644 --- a/src/eepp/graphics/resourcescope.cpp +++ b/src/eepp/graphics/resourcescope.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include #include @@ -34,6 +33,22 @@ TexturePtr ResourceScope::findTexture( const std::string& key ) const { return {}; } +DrawablePtr ResourceScope::findDrawableSource( const ResourceKey& key ) const { + return findDrawableSource( key.value() ); +} + +DrawablePtr ResourceScope::findDrawableSource( const std::string& key ) const { + if ( DrawablePtr drawable = mLocalCatalog->findDrawable( key ) ) + return drawable; + + Lock lock( mMutex ); + for ( const ResourceCatalogPtr& catalog : mImports ) { + if ( DrawablePtr drawable = catalog->findDrawable( key ) ) + return drawable; + } + return {}; +} + DrawablePtr ResourceScope::findDrawable( const std::string& name, bool firstSearchSprite ) const { if ( name.empty() ) return {}; @@ -77,15 +92,17 @@ DrawablePtr ResourceScope::findDrawable( const std::string& name, bool firstSear if ( String::startsWith( name, "@drawable/" ) ) return findDrawable( name.substr( 10 ) ); if ( String::startsWith( name, "@9p/" ) ) { - Drawable* source = NinePatchManager::instance()->getByName( name.substr( 4 ) ); - return source ? source->clone() : DrawablePtr{}; + DrawablePtr source = findDrawableSource( name.substr( 4 ) ); + return source && source->getDrawableType() == Drawable::NINEPATCH ? source->clone() + : DrawablePtr{}; } } + if ( DrawablePtr source = findDrawableSource( name ) ) + return source->clone(); + String::HashType id = String::hash( name ); Drawable* source = TextureAtlasManager::instance()->getTextureRegionById( id ); - if ( source == nullptr ) - source = NinePatchManager::instance()->getById( id ); if ( source ) return source->clone(); @@ -106,6 +123,14 @@ void ResourceScope::publishLocal( std::string key, TexturePtr texture ) { mLocalCatalog->publish( std::move( key ), std::move( texture ) ); } +void ResourceScope::publishLocalDrawable( ResourceKey key, DrawablePtr drawable ) { + publishLocalDrawable( key.value(), std::move( drawable ) ); +} + +void ResourceScope::publishLocalDrawable( std::string key, DrawablePtr drawable ) { + mLocalCatalog->publishDrawable( std::move( key ), std::move( drawable ) ); +} + bool ResourceScope::eraseLocal( const ResourceKey& key ) { return mLocalCatalog->erase( key ); } @@ -114,6 +139,14 @@ bool ResourceScope::eraseLocal( const std::string& key ) { return mLocalCatalog->erase( key ); } +bool ResourceScope::eraseLocalDrawable( const ResourceKey& key ) { + return mLocalCatalog->eraseDrawable( key ); +} + +bool ResourceScope::eraseLocalDrawable( const std::string& key ) { + return mLocalCatalog->eraseDrawable( key ); +} + void ResourceScope::clearLocal() { mLocalCatalog->clear(); } diff --git a/src/eepp/ui/uiscenenode.cpp b/src/eepp/ui/uiscenenode.cpp index 834a59631..061e8e903 100644 --- a/src/eepp/ui/uiscenenode.cpp +++ b/src/eepp/ui/uiscenenode.cpp @@ -162,6 +162,7 @@ UISceneNode::UISceneNode( EE::Window::Window* window ) : mRoot->enableReportSizeChangeToChildren(); mAsyncResourceLoadState->owner.store( this, std::memory_order_release ); mDocumentSessionId = mWebResourceCache->createSession(); + mUIThemeManager->setResourceScope( mResourceScope ); resizeNode( mWindow ); } @@ -763,6 +764,7 @@ const ResourceScopePtr& UISceneNode::getResourceScope() const { UISceneNode* UISceneNode::setResourceScope( ResourceScopePtr resourceScope ) { mResourceScope = resourceScope ? std::move( resourceScope ) : ResourceScope::New(); + mUIThemeManager->setResourceScope( mResourceScope ); return this; } diff --git a/src/eepp/ui/uitheme.cpp b/src/eepp/ui/uitheme.cpp index f3c0524cc..718de5ee5 100644 --- a/src/eepp/ui/uitheme.cpp +++ b/src/eepp/ui/uitheme.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -132,8 +131,8 @@ UITheme* UITheme::loadFromTextureAtlas( UITheme* tTheme, Graphics::TextureAtlas* std::string skinName( elemNameFromSkin( nameParts ) ); - Drawable* drawable = NinePatchManager::instance()->add( - NinePatch::New( textureRegion, l, t, r, b, realName ) ); + NinePatchPtr drawable = NinePatch::New( textureRegion, l, t, r, b, realName ); + tTheme->getResourceCatalog()->publishDrawable( realName, drawable ); if ( skins.find( skinName ) == skins.end() ) skins[skinName] = tTheme->add( UISkin::New( skinName ) ); @@ -232,9 +231,10 @@ UITheme* UITheme::loadFromDirectory( UITheme* tTheme, const std::string& Path, std::string skinName( elemNameFromSkin( nameParts ) ); - Drawable* drawable = NinePatchManager::instance()->add( + NinePatchPtr drawable = NinePatch::New( TextureFactory::instance()->loadFromFile( fpath ), l, t, r, - b, pixelDensity, realName ) ); + b, pixelDensity, realName ); + tTheme->getResourceCatalog()->publishDrawable( realName, drawable ); if ( skins.find( skinName ) == skins.end() ) skins[skinName] = tTheme->add( UISkin::New( skinName ) ); @@ -298,7 +298,8 @@ UITheme::UITheme( const std::string& name, const std::string& Abbr, Graphics::Fo mTextureAtlas( NULL ), mDefaultFont( defaultFont ), mDefaultFontSize( PixelDensity::dpToPx( PixelDensity::getPixelDensity() > 1.4 ? 11 : 12 ) ), - mIconTheme( UIIconTheme::New( name ) ) {} + mIconTheme( UIIconTheme::New( name ) ), + mResourceCatalog( ResourceCatalog::New() ) {} UITheme::~UITheme() { eeSAFE_DELETE( mIconTheme ); @@ -369,6 +370,10 @@ UIIconTheme* UITheme::getIconTheme() const { return mIconTheme; } +const ResourceCatalogPtr& UITheme::getResourceCatalog() const { + return mResourceCatalog; +} + const std::string& UITheme::getStyleSheetPath() const { return mStyleSheetPath; } diff --git a/src/eepp/ui/uithememanager.cpp b/src/eepp/ui/uithememanager.cpp index 48ad18bbf..9fbeb184e 100644 --- a/src/eepp/ui/uithememanager.cpp +++ b/src/eepp/ui/uithememanager.cpp @@ -20,7 +20,57 @@ UIThemeManager::UIThemeManager() : mTooltipFollowMouse( false ), mCursorSize( 16, 16 ) {} -UIThemeManager::~UIThemeManager() {} +UIThemeManager::~UIThemeManager() { + if ( mResourceScope ) { + each( [this]( const auto& resource ) { + if ( resource.second ) + mResourceScope->removeCatalog( resource.second->getResourceCatalog() ); + } ); + } +} + +UITheme* UIThemeManager::add( UITheme* theme ) { + UITheme* added = ResourceManager::add( theme ); + if ( added && mResourceScope ) + mResourceScope->importCatalog( added->getResourceCatalog() ); + return added; +} + +bool UIThemeManager::remove( UITheme* theme, bool destroy ) { + if ( theme && mResourceScope ) + mResourceScope->removeCatalog( theme->getResourceCatalog() ); + if ( theme == mThemeDefault ) + mThemeDefault = nullptr; + return ResourceManager::remove( theme, destroy ); +} + +bool UIThemeManager::removeById( const String::HashType& id, bool destroy ) { + return remove( getById( id ), destroy ); +} + +bool UIThemeManager::removeByName( const std::string& name, bool destroy ) { + return remove( getByName( name ), destroy ); +} + +UIThemeManager* UIThemeManager::setResourceScope( ResourceScopePtr resourceScope ) { + if ( mResourceScope == resourceScope ) + return this; + + if ( mResourceScope ) { + each( [this]( const auto& resource ) { + if ( resource.second ) + mResourceScope->removeCatalog( resource.second->getResourceCatalog() ); + } ); + } + mResourceScope = std::move( resourceScope ); + if ( mResourceScope ) { + each( [this]( const auto& resource ) { + if ( resource.second ) + mResourceScope->importCatalog( resource.second->getResourceCatalog() ); + } ); + } + return this; +} UIThemeManager* UIThemeManager::setDefaultFont( Font* Font ) { mFont = Font; @@ -46,7 +96,15 @@ const Float& UIThemeManager::getDefaultFontSize() const { } UIThemeManager* UIThemeManager::setDefaultTheme( UITheme* Theme ) { + UITheme* previousTheme = mThemeDefault; + if ( previousTheme && previousTheme != Theme && mResourceScope && + !findIf( [previousTheme]( const auto& resource ) { + return resource.second == previousTheme; + } ) ) + mResourceScope->removeCatalog( previousTheme->getResourceCatalog() ); mThemeDefault = Theme; + if ( mThemeDefault && mResourceScope ) + mResourceScope->importCatalog( mThemeDefault->getResourceCatalog() ); if ( NULL != mThemeDefault && NULL == mThemeDefault->getDefaultFont() ) { setDefaultFont( mFont ); diff --git a/src/eepp/window/engine.cpp b/src/eepp/window/engine.cpp index d764069d8..28e1c3e21 100644 --- a/src/eepp/window/engine.cpp +++ b/src/eepp/window/engine.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -108,8 +107,6 @@ Engine::~Engine() { Doc::SyntaxDefinitionManager::destroySingleton(); - NinePatchManager::destroySingleton(); - FontManager::destroySingleton(); TextureAtlasManager::destroySingleton(); diff --git a/src/tests/unit_tests/resource_prerequisite_tests.cpp b/src/tests/unit_tests/resource_prerequisite_tests.cpp index 7f1573c21..5ef6bad17 100644 --- a/src/tests/unit_tests/resource_prerequisite_tests.cpp +++ b/src/tests/unit_tests/resource_prerequisite_tests.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -33,6 +33,8 @@ #include #include #include +#include +#include #include #include #include @@ -343,6 +345,35 @@ UTEST( ResourcePrerequisites, resourceCatalogOwnsPublishedTextures ) { Engine::destroySingleton(); } +UTEST( ResourcePrerequisites, resourceCatalogRemovalPreservesRetainedDrawableConsumers ) { + EE::Window::Window* window = createLifecycleTestWindow( "Nine-patch catalog ownership test" ); + TextureFactory* factory = TextureFactory::instance(); + ResourceCatalogPtr catalog = ResourceCatalog::New(); + TexturePtr texture = factory->createEmptyTexture( 4, 4 ); + ASSERT_TRUE( texture != nullptr ); + + NinePatchPtr ninePatch = NinePatch::New( texture, 1, 1, 1, 1, 1, "retained-nine-patch" ); + ASSERT_TRUE( ninePatch != nullptr ); + catalog->publishDrawable( "retained-nine-patch", ninePatch ); + NinePatchWeakPtr weakNinePatch = ninePatch; + TextureWeakPtr weakTexture = texture; + texture.reset(); + + EXPECT_TRUE( catalog->findDrawable( "retained-nine-patch" ) == ninePatch ); + EXPECT_TRUE( catalog->eraseDrawable( "retained-nine-patch" ) ); + EXPECT_TRUE( catalog->findDrawable( "retained-nine-patch" ) == nullptr ); + EXPECT_FALSE( weakNinePatch.expired() ); + EXPECT_FALSE( weakTexture.expired() ); + EXPECT_TRUE( ninePatch->getPixelsSize() == Sizef( 4, 4 ) ); + + ninePatch.reset(); + EXPECT_TRUE( weakNinePatch.expired() ); + EXPECT_TRUE( weakTexture.expired() ); + window->display( false ); + EXPECT_EQ( factory->getPendingReleaseCount(), static_cast( 0 ) ); + Engine::destroySingleton(); +} + UTEST( ResourcePrerequisites, resourceScopesResolveOnlyLocalAndExplicitlyImportedCatalogs ) { EE::Window::Window* window = createLifecycleTestWindow( "Resource scope isolation test" ); TextureFactory* factory = TextureFactory::instance(); @@ -452,6 +483,44 @@ UTEST( ResourcePrerequisites, uiScenesOwnIsolatedScopesThatCanBeSharedExplicitly Engine::destroySingleton(); } +UTEST( ResourcePrerequisites, uiThemeCatalogIsImportedOnlyByItsOwningScene ) { + EE::Window::Window* window = createLifecycleTestWindow( "UI theme resource catalog test" ); + UISceneNode* firstScene = UISceneNode::New( window ); + UISceneNode* secondScene = UISceneNode::New( window ); + TexturePtr texture = TextureFactory::instance()->createEmptyTexture( 4, 4 ); + ASSERT_TRUE( texture != nullptr ); + + UITheme* theme = UITheme::New( "catalog-theme", "catalog-theme" ); + NinePatchPtr ninePatch = NinePatch::New( texture, 1, 1, 1, 1, 1, "theme-nine-patch" ); + theme->getResourceCatalog()->publishDrawable( "theme-nine-patch", ninePatch ); + firstScene->getUIThemeManager()->add( theme ); + + DrawablePtr firstResolved = firstScene->getResourceScope()->findDrawable( "theme-nine-patch" ); + ASSERT_TRUE( firstResolved != nullptr ); + EXPECT_EQ( firstResolved->getDrawableType(), Drawable::NINEPATCH ); + EXPECT_TRUE( secondScene->getResourceScope()->findDrawable( "theme-nine-patch" ) == nullptr ); + + EXPECT_TRUE( firstScene->getUIThemeManager()->remove( theme, false ) ); + EXPECT_TRUE( firstScene->getResourceScope()->findDrawable( "theme-nine-patch" ) == nullptr ); + + UITheme* defaultOnlyTheme = UITheme::New( "default-only-theme", "default-only-theme" ); + defaultOnlyTheme->getResourceCatalog()->publishDrawable( "default-nine-patch", ninePatch ); + secondScene->getUIThemeManager()->setDefaultTheme( defaultOnlyTheme ); + EXPECT_TRUE( secondScene->getResourceScope()->findDrawable( "default-nine-patch" ) != nullptr ); + secondScene->getUIThemeManager()->setDefaultTheme( static_cast( nullptr ) ); + EXPECT_TRUE( secondScene->getResourceScope()->findDrawable( "default-nine-patch" ) == nullptr ); + + firstResolved.reset(); + ninePatch.reset(); + texture.reset(); + eeDelete( defaultOnlyTheme ); + eeDelete( theme ); + eeDelete( secondScene ); + eeDelete( firstScene ); + window->display( false ); + Engine::destroySingleton(); +} + UTEST( ResourcePrerequisites, pendingBatchRetainsTextureUntilDisplayCollection ) { EE::Window::Window* window = createLifecycleTestWindow( "Texture deferred release test" ); TextureFactory* factory = TextureFactory::instance(); @@ -926,9 +995,10 @@ UTEST( ResourcePrerequisites, engineTeardownReleasesGraphicsBeforeContextsAcross TexturePtr texture = TextureFactory::instance()->createEmptyTexture( 4, 4 ); ASSERT_TRUE( texture != nullptr ); - NinePatch* ninePatch = NinePatchManager::instance()->add( - NinePatch::New( texture, 1, 1, 1, 1, 1, "engine-teardown-nine-patch" ) ); + NinePatchPtr ninePatch = + NinePatch::New( texture, 1, 1, 1, 1, 1, "engine-teardown-nine-patch" ); ASSERT_TRUE( ninePatch != nullptr ); + globalResourceCatalog().publishDrawable( "engine-teardown-nine-patch", ninePatch ); auto* scene = UISceneNode::New(); SceneManager::instance()->add( scene ); @@ -945,6 +1015,7 @@ UTEST( ResourcePrerequisites, engineTeardownReleasesGraphicsBeforeContextsAcross auto* batch = GlobalBatchRenderer::instance(); batch->setTexture( texture ); batch->batchQuad( 0, 0, 4, 4 ); + ninePatch.reset(); texture.reset(); Engine::destroySingleton(); @@ -953,7 +1024,6 @@ UTEST( ResourcePrerequisites, engineTeardownReleasesGraphicsBeforeContextsAcross EXPECT_TRUE( Engine::existsSingleton() == nullptr ); EXPECT_TRUE( SceneManager::existsSingleton() == nullptr ); EXPECT_TRUE( GlobalBatchRenderer::existsSingleton() == nullptr ); - EXPECT_TRUE( NinePatchManager::existsSingleton() == nullptr ); EXPECT_TRUE( FontManager::existsSingleton() == nullptr ); EXPECT_TRUE( TextureAtlasManager::existsSingleton() == nullptr ); EXPECT_TRUE( TextureFactory::existsSingleton() == nullptr );