diff --git a/include/eepp/graphics/resource.hpp b/include/eepp/graphics/resource.hpp index 0083331dc..ac603e37e 100644 --- a/include/eepp/graphics/resource.hpp +++ b/include/eepp/graphics/resource.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -28,6 +29,42 @@ class ResourceId { Uint64 mValue{ 0 }; }; +/** + * Strong 64-bit hash of a semantic resource name. + * + * This is a fast, process-local convenience key for trusted resource names. It is deliberately + * distinct from String::HashType so legacy 32-bit hashes cannot enter resource-name APIs through + * an implicit integer conversion. Complete ResourceKey values remain the authoritative identity + * whenever collision safety or persistence is required. + */ +class ResourceNameHash { + public: + constexpr ResourceNameHash() = default; + explicit constexpr ResourceNameHash( Uint64 value ) : mValue( value ) {} + + constexpr Uint64 value() const { return mValue; } + explicit constexpr operator bool() const { return mValue != 0; } + + constexpr bool operator==( const ResourceNameHash& other ) const { + return mValue == other.mValue; + } + constexpr bool operator!=( const ResourceNameHash& other ) const { return !( *this == other ); } + constexpr bool operator<( const ResourceNameHash& other ) const { + return mValue < other.mValue; + } + + private: + Uint64 mValue{ 0 }; +}; + +/** + * Hashes a semantic resource name with the vendored wyhash implementation used by + * UnorderedMap. Do not serialize the result: use the complete ResourceKey in persistent formats. + */ +inline ResourceNameHash resourceNameHash( std::string_view name ) { + return ResourceNameHash( ankerl::unordered_dense::hash{}( name ) ); +} + /** Immutable semantic lookup key. Catalog equality always compares the complete key value. */ class ResourceKey { public: @@ -66,6 +103,12 @@ template <> struct hash { } }; +template <> struct hash { + std::size_t operator()( const EE::Graphics::ResourceNameHash& hash ) const noexcept { + return std::hash{}( hash.value() ); + } +}; + } // namespace std #endif diff --git a/include/eepp/graphics/resourcecatalog.hpp b/include/eepp/graphics/resourcecatalog.hpp index 6d7940b58..25cd36dfd 100644 --- a/include/eepp/graphics/resourcecatalog.hpp +++ b/include/eepp/graphics/resourcecatalog.hpp @@ -29,10 +29,12 @@ using ResourceCatalogPtr = ResourcePtr; * released after dropping the catalog mutex so resource destruction and callbacks never execute * while the catalog is locked. * - * Drawable and font hash lookups use String::hash() of the semantic key as a weak secondary index. - * They are compatibility/convenience lookups and do not replace full-key lookup when collision-safe - * identity is required. Textures use ResourceId for process-wide object identity elsewhere; the - * hash accepted by findDrawable() and findFont() is not a ResourceId. + * Drawable and font hash lookups use resourceNameHash() as a direct secondary index. The dedicated + * 64-bit hash provides a cheap expected-O(1) convenience lookup for trusted names without changing + * the repository-wide 32-bit String::HashType. A hash is not mathematically collision-free, so + * complete ResourceKey lookup remains authoritative and must be used for attacker-controlled names + * or persistent identity. ResourceNameHash values are process-local conveniences and must not be + * serialized. * * ResourceCatalog performs no parent, scene, live-registry, filesystem, or fallback search. A * ResourceScope defines lookup precedence by searching its local catalog and then explicitly @@ -82,11 +84,17 @@ class EE_API ResourceCatalog { /** @copydoc findDrawable(const ResourceKey&)const */ DrawablePtr findDrawable( const std::string& key ) const; /** - * @brief Looks up a drawable through the weak String::hash(key) secondary index. - * @return An owning handle when the indexed drawable is still present, otherwise an empty - * handle. + * @brief Looks up a drawable through the resourceNameHash(key) convenience index. */ - DrawablePtr findDrawable( const String::HashType& id ) const; + DrawablePtr findDrawable( ResourceNameHash hash ) const; + /** + * @brief Legacy lookup for persisted 32-bit drawable hashes. + * + * This compatibility path scans full bindings and should not be used by new code. Persistent + * formats should migrate to complete ResourceKey values rather than serializing another hash. + * If several names share @p legacyHash, which matching drawable is returned is unspecified. + */ + DrawablePtr findDrawable( String::HashType legacyHash ) const; /** @return The texture atlas bound to @p key, or an empty handle when it is not present. */ TextureAtlasPtr findAtlas( const ResourceKey& key ) const; @@ -100,10 +108,9 @@ class EE_API ResourceCatalog { /** @copydoc findFont(const ResourceKey&)const */ FontPtr findFont( const std::string& key ) const; /** - * @brief Looks up a font through the weak String::hash(key) secondary index. - * @return An owning handle when the indexed font is still present, otherwise an empty handle. + * @brief Looks up a font through the resourceNameHash(key) convenience index. */ - FontPtr findFont( const String::HashType& id ) const; + FontPtr findFont( ResourceNameHash hash ) const; /** @return An owning snapshot of all fonts currently published in this catalog. */ std::vector getFonts() const; @@ -155,10 +162,10 @@ class EE_API ResourceCatalog { mutable System::Mutex mMutex; UnorderedMap mTextures; UnorderedMap mDrawables; - UnorderedMap mDrawablesById; + UnorderedMap mDrawablesByNameHash; UnorderedMap mAtlases; UnorderedMap mFonts; - UnorderedMap mFontsById; + UnorderedMap mFontsByNameHash; UnorderedMap mShaderPrograms; }; diff --git a/include/eepp/graphics/resourcescope.hpp b/include/eepp/graphics/resourcescope.hpp index f4ac210a0..600f9ad90 100644 --- a/include/eepp/graphics/resourcescope.hpp +++ b/include/eepp/graphics/resourcescope.hpp @@ -41,13 +41,14 @@ class EE_API ResourceScope { 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; + DrawablePtr findDrawable( ResourceNameHash hash ) const; + DrawablePtr findDrawable( String::HashType legacyHash ) const; TextureAtlasPtr findAtlas( const ResourceKey& key ) const; TextureAtlasPtr findAtlas( const std::string& key ) const; std::vector getAtlases() const; FontPtr findFont( const ResourceKey& key ) const; FontPtr findFont( const std::string& key ) const; - FontPtr findFont( const String::HashType& id ) const; + FontPtr findFont( ResourceNameHash hash ) const; std::vector getFonts() const; ShaderProgramPtr findShaderProgram( const ResourceKey& key ) const; ShaderProgramPtr findShaderProgram( const std::string& key ) const; diff --git a/include/eepp/ui/drawableresolver.hpp b/include/eepp/ui/drawableresolver.hpp index 463348e43..ee4c94d54 100644 --- a/include/eepp/ui/drawableresolver.hpp +++ b/include/eepp/ui/drawableresolver.hpp @@ -15,7 +15,8 @@ class EE_API DrawableResolver { explicit DrawableResolver( Graphics::ResourceScope& resourceScope ); Graphics::DrawablePtr resolve( const std::string& name, bool firstSearchSprite = false ) const; - Graphics::DrawablePtr resolveById( const Uint32& id ) const; + Graphics::DrawablePtr resolveById( Graphics::ResourceNameHash hash ) const; + Graphics::DrawablePtr resolveById( String::HashType legacyHash ) const; void setPrintWarnings( bool printWarnings ); bool getPrintWarnings() const; diff --git a/src/eepp/graphics/fontservice.cpp b/src/eepp/graphics/fontservice.cpp index 43c3ea539..250fd4dc6 100644 --- a/src/eepp/graphics/fontservice.cpp +++ b/src/eepp/graphics/fontservice.cpp @@ -15,7 +15,7 @@ ResourceScope& FontService::getResourceScope() const { FontPtr FontService::findHandle( Font* font ) const { if ( !font ) return {}; - FontPtr handle = mResourceScope.findFont( font->getId() ); + FontPtr handle = mResourceScope.findFont( font->getName() ); return handle.get() == font ? handle : FontPtr{}; } diff --git a/src/eepp/graphics/resourcecatalog.cpp b/src/eepp/graphics/resourcecatalog.cpp index 71597f1cc..92c083273 100644 --- a/src/eepp/graphics/resourcecatalog.cpp +++ b/src/eepp/graphics/resourcecatalog.cpp @@ -53,19 +53,19 @@ void ResourceCatalog::publishDrawable( std::string key, DrawablePtr drawable ) { } DrawablePtr previous; - String::HashType id = String::hash( key ); + ResourceNameHash hash = resourceNameHash( key ); { Lock lock( mMutex ); auto it = mDrawables.find( key ); if ( it == mDrawables.end() ) { mDrawables.emplace( std::move( key ), drawable ); - mDrawablesById[id] = drawable; + mDrawablesByNameHash[hash] = drawable; return; } previous = std::move( it->second ); it->second = drawable; - mDrawablesById[id] = drawable; + mDrawablesByNameHash[hash] = drawable; } previous.reset(); @@ -111,18 +111,18 @@ void ResourceCatalog::publishFont( std::string key, FontPtr font ) { } FontPtr previous; - String::HashType id = String::hash( key ); + ResourceNameHash hash = resourceNameHash( key ); { Lock lock( mMutex ); auto it = mFonts.find( key ); if ( it == mFonts.end() ) { mFonts.emplace( std::move( key ), font ); - mFontsById[id] = font; + mFontsByNameHash[hash] = font; return; } previous = std::move( it->second ); it->second = font; - mFontsById[id] = font; + mFontsByNameHash[hash] = font; } previous.reset(); } @@ -172,10 +172,19 @@ DrawablePtr ResourceCatalog::findDrawable( const std::string& key ) const { return it != mDrawables.end() ? it->second : DrawablePtr{}; } -DrawablePtr ResourceCatalog::findDrawable( const String::HashType& id ) const { +DrawablePtr ResourceCatalog::findDrawable( ResourceNameHash hash ) const { Lock lock( mMutex ); - auto it = mDrawablesById.find( id ); - return it != mDrawablesById.end() ? it->second.lock() : DrawablePtr{}; + auto it = mDrawablesByNameHash.find( hash ); + return it != mDrawablesByNameHash.end() ? it->second.lock() : DrawablePtr{}; +} + +DrawablePtr ResourceCatalog::findDrawable( String::HashType legacyHash ) const { + Lock lock( mMutex ); + for ( const auto& [key, drawable] : mDrawables ) { + if ( String::hash( key ) == legacyHash ) + return drawable; + } + return {}; } TextureAtlasPtr ResourceCatalog::findAtlas( const ResourceKey& key ) const { @@ -207,10 +216,10 @@ FontPtr ResourceCatalog::findFont( const std::string& key ) const { return it != mFonts.end() ? it->second : FontPtr{}; } -FontPtr ResourceCatalog::findFont( const String::HashType& id ) const { +FontPtr ResourceCatalog::findFont( ResourceNameHash hash ) const { Lock lock( mMutex ); - auto it = mFontsById.find( id ); - return it != mFontsById.end() ? it->second.lock() : FontPtr{}; + auto it = mFontsByNameHash.find( hash ); + return it != mFontsByNameHash.end() ? it->second.lock() : FontPtr{}; } std::vector ResourceCatalog::getFonts() const { @@ -275,9 +284,7 @@ bool ResourceCatalog::eraseDrawable( const std::string& key ) { drawable = std::move( it->second ); mDrawables.erase( it ); - auto idIt = mDrawablesById.find( String::hash( key ) ); - if ( idIt != mDrawablesById.end() ) - mDrawablesById.erase( idIt ); + mDrawablesByNameHash.erase( resourceNameHash( key ) ); } drawable.reset(); @@ -315,9 +322,7 @@ bool ResourceCatalog::eraseFont( const std::string& key ) { return false; font = std::move( it->second ); mFonts.erase( it ); - auto idIt = mFontsById.find( String::hash( key ) ); - if ( idIt != mFontsById.end() ) - mFontsById.erase( idIt ); + mFontsByNameHash.erase( resourceNameHash( key ) ); } font.reset(); return true; @@ -334,9 +339,7 @@ bool ResourceCatalog::eraseFont( Font* font ) { return false; removed = std::move( it->second ); mFonts.erase( it ); - auto idIt = mFontsById.find( font->getId() ); - if ( idIt != mFontsById.end() ) - mFontsById.erase( idIt ); + mFontsByNameHash.erase( resourceNameHash( font->getName() ) ); } removed.reset(); return true; @@ -363,28 +366,28 @@ bool ResourceCatalog::eraseShaderProgram( const std::string& key ) { void ResourceCatalog::clear() { UnorderedMap textures; UnorderedMap drawables; - UnorderedMap drawablesById; + UnorderedMap drawablesByNameHash; UnorderedMap atlases; UnorderedMap fonts; - UnorderedMap fontsById; + UnorderedMap fontsByNameHash; UnorderedMap shaderPrograms; { Lock lock( mMutex ); textures = std::move( mTextures ); drawables = std::move( mDrawables ); - drawablesById = std::move( mDrawablesById ); + drawablesByNameHash = std::move( mDrawablesByNameHash ); atlases = std::move( mAtlases ); fonts = std::move( mFonts ); - fontsById = std::move( mFontsById ); + fontsByNameHash = std::move( mFontsByNameHash ); shaderPrograms = std::move( mShaderPrograms ); } textures.clear(); drawables.clear(); - drawablesById.clear(); + drawablesByNameHash.clear(); atlases.clear(); fonts.clear(); - fontsById.clear(); + fontsByNameHash.clear(); shaderPrograms.clear(); } diff --git a/src/eepp/graphics/resourcescope.cpp b/src/eepp/graphics/resourcescope.cpp index 1631ccf14..cfb77fdf6 100644 --- a/src/eepp/graphics/resourcescope.cpp +++ b/src/eepp/graphics/resourcescope.cpp @@ -121,12 +121,12 @@ FontPtr ResourceScope::findFont( const std::string& key ) const { return {}; } -FontPtr ResourceScope::findFont( const String::HashType& id ) const { - if ( FontPtr font = mLocalCatalog->findFont( id ) ) +FontPtr ResourceScope::findFont( ResourceNameHash hash ) const { + if ( FontPtr font = mLocalCatalog->findFont( hash ) ) return font; Lock lock( mMutex ); for ( const ResourceCatalogPtr& catalog : mImports ) { - if ( FontPtr font = catalog->findFont( id ) ) + if ( FontPtr font = catalog->findFont( hash ) ) return font; } return {}; @@ -288,12 +288,24 @@ DrawablePtr ResourceScope::findDrawable( const std::string& name, bool firstSear return texture ? texture->clone() : DrawablePtr{}; } -DrawablePtr ResourceScope::findDrawable( const Uint32& id ) const { - DrawablePtr source = mLocalCatalog->findDrawable( id ); +DrawablePtr ResourceScope::findDrawable( ResourceNameHash hash ) const { + DrawablePtr source = mLocalCatalog->findDrawable( hash ); if ( !source ) { Lock lock( mMutex ); for ( const ResourceCatalogPtr& catalog : mImports ) { - if ( ( source = catalog->findDrawable( id ) ) ) + if ( ( source = catalog->findDrawable( hash ) ) ) + break; + } + } + return source ? source->clone() : DrawablePtr{}; +} + +DrawablePtr ResourceScope::findDrawable( String::HashType legacyHash ) const { + DrawablePtr source = mLocalCatalog->findDrawable( legacyHash ); + if ( !source ) { + Lock lock( mMutex ); + for ( const ResourceCatalogPtr& catalog : mImports ) { + if ( ( source = catalog->findDrawable( legacyHash ) ) ) break; } } @@ -383,7 +395,7 @@ bool ResourceScope::eraseLocalFont( const std::string& key ) { bool ResourceScope::eraseLocalFont( Font* font ) { if ( !font ) return false; - FontPtr handle = mLocalCatalog->findFont( font->getId() ); + FontPtr handle = mLocalCatalog->findFont( font->getName() ); if ( handle.get() != font ) return false; if ( !mLocalCatalog->eraseFont( font ) ) diff --git a/src/eepp/ui/drawableresolver.cpp b/src/eepp/ui/drawableresolver.cpp index d287ad94f..cd3b1866e 100644 --- a/src/eepp/ui/drawableresolver.cpp +++ b/src/eepp/ui/drawableresolver.cpp @@ -155,11 +155,20 @@ DrawablePtr DrawableResolver::resolve( const std::string& name, bool firstSearch return drawable; } -DrawablePtr DrawableResolver::resolveById( const Uint32& id ) const { +DrawablePtr DrawableResolver::resolveById( ResourceNameHash hash ) const { ResourceScope& resourceScope = mSceneNode ? *mSceneNode->getResourceScope() : *mResourceScope; - DrawablePtr drawable = resourceScope.findDrawable( id ); + DrawablePtr drawable = resourceScope.findDrawable( hash ); if ( !drawable && mPrintWarnings ) - Log::warning( "DrawableResolver::resolveById: \"%ld\" not found", id ); + Log::warning( "DrawableResolver::resolveById: \"%llu\" not found", + static_cast( hash.value() ) ); + return drawable; +} + +DrawablePtr DrawableResolver::resolveById( String::HashType legacyHash ) const { + ResourceScope& resourceScope = mSceneNode ? *mSceneNode->getResourceScope() : *mResourceScope; + DrawablePtr drawable = resourceScope.findDrawable( legacyHash ); + if ( !drawable && mPrintWarnings ) + Log::warning( "DrawableResolver::resolveById: legacy hash \"%u\" not found", legacyHash ); return drawable; } diff --git a/src/tests/unit_tests/resource_prerequisite_tests.cpp b/src/tests/unit_tests/resource_prerequisite_tests.cpp index 236e45214..58acf7a45 100644 --- a/src/tests/unit_tests/resource_prerequisite_tests.cpp +++ b/src/tests/unit_tests/resource_prerequisite_tests.cpp @@ -466,6 +466,51 @@ UTEST( ResourcePrerequisites, resourceCatalogRemovalPreservesRetainedDrawableCon Engine::destroySingleton(); } +UTEST( ResourcePrerequisites, resourceNameHashSeparatesLegacyDrawableCollision ) { + constexpr const char* firstKey = "BA"; + constexpr const char* secondKey = "Ab"; + ASSERT_EQ( String::hash( firstKey ), String::hash( secondKey ) ); + const ResourceNameHash firstHash = resourceNameHash( firstKey ); + const ResourceNameHash secondHash = resourceNameHash( secondKey ); + ASSERT_NE( firstHash.value(), secondHash.value() ); + + auto catalog = ResourceCatalog::New(); + auto first = makeResource(); + auto second = makeResource(); + catalog->publishDrawable( firstKey, first ); + catalog->publishDrawable( secondKey, second ); + EXPECT_EQ( first.get(), catalog->findDrawable( firstHash ).get() ); + EXPECT_EQ( second.get(), catalog->findDrawable( secondHash ).get() ); + + EXPECT_TRUE( catalog->eraseDrawable( firstKey ) ); + EXPECT_TRUE( catalog->findDrawable( firstHash ) == nullptr ); + EXPECT_EQ( second.get(), catalog->findDrawable( secondHash ).get() ); + EXPECT_TRUE( catalog->eraseDrawable( secondKey ) ); + EXPECT_TRUE( catalog->findDrawable( secondHash ) == nullptr ); +} + +UTEST( ResourcePrerequisites, resourceNameHashSeparatesLegacyFontCollision ) { + constexpr const char* firstKey = "BA"; + constexpr const char* secondKey = "Ab"; + ASSERT_EQ( String::hash( firstKey ), String::hash( secondKey ) ); + const ResourceNameHash firstHash = resourceNameHash( firstKey ); + const ResourceNameHash secondHash = resourceNameHash( secondKey ); + ASSERT_NE( firstHash.value(), secondHash.value() ); + + auto scope = ResourceScope::New(); + auto first = FontBMFont::New( firstKey, *scope ); + auto second = FontBMFont::New( secondKey, *scope ); + auto catalog = scope->getLocalCatalog(); + EXPECT_EQ( first.get(), catalog->findFont( firstHash ).get() ); + EXPECT_EQ( second.get(), catalog->findFont( secondHash ).get() ); + + EXPECT_TRUE( scope->eraseLocalFont( firstKey ) ); + EXPECT_TRUE( catalog->findFont( firstHash ) == nullptr ); + EXPECT_EQ( second.get(), catalog->findFont( secondHash ).get() ); + EXPECT_TRUE( scope->eraseLocalFont( secondKey ) ); + EXPECT_TRUE( catalog->findFont( secondHash ) == nullptr ); +} + UTEST( ResourcePrerequisites, resourceScopesResolveOnlyLocalAndExplicitlyImportedCatalogs ) { EE::Window::Window* window = createLifecycleTestWindow( "Resource scope isolation test" ); TextureFactory* factory = TextureFactory::instance();