diff --git a/.agent/SOUL.md b/.agent/SOUL.md index 2891b4f84..b9283ded0 100644 --- a/.agent/SOUL.md +++ b/.agent/SOUL.md @@ -10,6 +10,12 @@ Your name is Negen (from negentropy: the process of creating order out of chaos) - Favor stack-allocated memory over heap allocations whenever possible. - Any heap allocation must be heavily justified. - Exercise reason: maximize stack use for speed, but actively calculate boundaries to prevent stack-overflows. + - Before finalizing C++ changes, perform an explicit allocation audit: + - Review every heap allocation, string copy, container insertion, `std::function`, lambda capture, and async handoff introduced or touched by the change. + - Prefer move captures for owned temporary strings, buffers, vectors, and other heap-backed objects passed into lambdas. + - Avoid capturing large objects by value unless lifetime safety requires ownership. + - If a copy is required for async lifetime or thread-safety, make that reason clear in the self-review. + - Do not limit this audit to render-loop code; repeated resource loading, parsing, layout, and async paths can still multiply memory waste. 2. **Protect the Render Loop:** - Render time is critical. diff --git a/src/eepp/ui/uiimage.cpp b/src/eepp/ui/uiimage.cpp index a8ae01903..5acfbcfd5 100644 --- a/src/eepp/ui/uiimage.cpp +++ b/src/eepp/ui/uiimage.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -13,8 +14,31 @@ #include #include +#include + namespace EE { namespace UI { +namespace { + +std::string getTextureCacheName( const Network::URI& uri ) { + std::string filePath( uri.getFSPath() ); + FileSystem::filePathRemoveProcessPath( filePath ); + return filePath; +} + +Texture* loadFileTextureCached( const std::string& filePath, const std::string& cacheName ) { + static std::mutex loadMutex; + std::lock_guard lock( loadMutex ); + + if ( Texture* texture = TextureFactory::instance()->getByName( cacheName ) ) + return texture; + + return TextureFactory::instance()->loadFromFile( + filePath, false, Texture::ClampMode::ClampToEdge, false, false ); +} + +} // namespace + UIImage* UIImage::New() { return eeNew( UIImage, () ); } @@ -305,33 +329,39 @@ bool UIImage::loadFileDrawable( const Network::URI& uri ) { !Window::Engine::instance()->isSharedGLContextEnabled() ) return false; + Uint64 loadId = ++mRemoteImageLoadId; + std::string filePath = uri.getFSPath(); + std::string cacheName = getTextureCacheName( uri ); + if ( Texture* texture = TextureFactory::instance()->getByName( cacheName ) ) { + setDrawable( texture, false ); + return true; + } + auto resourceState = scene->getAsyncResourceLoadState(); Uint64 resourceGeneration = resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; - Uint64 loadId = ++mRemoteImageLoadId; auto alive = mAsyncImageAlive; - const std::string filePath = uri.getFSPath(); - scene->getThreadPool()->run( - [resourceState, resourceGeneration, alive, loadId, filePath, this] { - if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || - !alive || !alive->load( std::memory_order_acquire ) ) - return; + scene->getThreadPool()->run( [resourceState, resourceGeneration, alive, loadId, + filePath = std::move( filePath ), + cacheName = std::move( cacheName ), this] { + if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || + !alive || !alive->load( std::memory_order_acquire ) ) + return; - Texture* texture = TextureFactory::instance()->loadFromFile( - filePath, false, Texture::ClampMode::ClampToEdge, false, false ); - if ( texture == nullptr ) - return; + Texture* texture = loadFileTextureCached( filePath, cacheName ); + if ( texture == nullptr ) + return; - UISceneNode::runAsyncResourceOnMainThread( - resourceState, resourceGeneration, [alive, loadId, texture, this]( UISceneNode* ) { - if ( !alive || !alive->load( std::memory_order_acquire ) || - loadId != mRemoteImageLoadId ) - return; + UISceneNode::runAsyncResourceOnMainThread( + resourceState, resourceGeneration, [alive, loadId, texture, this]( UISceneNode* ) { + if ( !alive || !alive->load( std::memory_order_acquire ) || + loadId != mRemoteImageLoadId ) + return; - setDrawable( texture, false ); - } ); - } ); + setDrawable( texture, false ); + } ); + } ); return true; } @@ -341,24 +371,30 @@ void UIImage::loadRemoteDrawable( const Network::URI& uri ) { if ( !scene ) return; + std::string url = uri.toString(); + if ( Texture* texture = TextureFactory::instance()->getByName( url ) ) { + if ( mDrawable != texture ) { + ++mRemoteImageLoadId; + setDrawable( texture, false ); + } + return; + } + auto resourceState = scene->getAsyncResourceLoadState(); Uint64 resourceGeneration = resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; Uint64 loadId = ++mRemoteImageLoadId; auto alive = mAsyncImageAlive; - const std::string url = uri.toString(); - const std::string textureName = - String::format( "__eepp_ui_image_%p_%llu_%u", this, - static_cast( loadId ), String::hash( url ) ); - Texture* texture = TextureFactory::instance()->createEmptyTexture( - 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, - textureName ); + 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, url ); if ( texture ) - setDrawable( texture, true ); + setDrawable( texture, false ); + Http::Request::FieldTable headers; + if ( !scene->getReferer().empty() ) + headers["referer"] = scene->getReferer().toString(); Http::getAsync( - [resourceState, resourceGeneration, alive, loadId, texture, + [resourceState, resourceGeneration, alive, loadId, texture, url = std::move( url ), this]( const Http&, Http::Request&, Http::Response& response ) { if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || !alive || !alive->load( std::memory_order_acquire ) || texture == nullptr ) @@ -379,9 +415,13 @@ void UIImage::loadRemoteDrawable( const Network::URI& uri ) { if ( image.getPixels() != nullptr ) texture->replace( &image ); } ); + } else { + Log::debug( "UIImage::loadRemoteDrawable: could not download image: %s. Error: " + "%d\n%s", + url, response.getStatus(), response.getBody() ); } }, - uri, Seconds( 5 ), {}, {}, "", true, Http::getEnvProxyURI() ); + uri, Seconds( 5 ), {}, headers, "", true, Http::getEnvProxyURI() ); } void UIImage::onSizeChange() { diff --git a/src/eepp/ui/uinodedrawable.cpp b/src/eepp/ui/uinodedrawable.cpp index ebd6a6081..0f27a680a 100644 --- a/src/eepp/ui/uinodedrawable.cpp +++ b/src/eepp/ui/uinodedrawable.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -698,24 +699,30 @@ bool UINodeDrawable::LayerDrawable::loadRemoteDrawable( const std::string& value if ( value == mDrawableRef && mDrawable != nullptr ) return true; + std::string url = uri.toString(); + if ( Texture* texture = TextureFactory::instance()->getByName( url ) ) { + if ( mDrawable != texture ) { + ++mRemoteDrawableLoadId; + setDrawable( texture, false ); + } + return true; + } + auto resourceState = scene->getAsyncResourceLoadState(); Uint64 resourceGeneration = resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; Uint64 loadId = ++mRemoteDrawableLoadId; auto alive = mAsyncDrawableAlive; - const std::string url = uri.toString(); - const std::string textureName = - String::format( "__eepp_ui_layer_image_%p_%llu_%u", this, - static_cast( loadId ), String::hash( url ) ); - Texture* texture = TextureFactory::instance()->createEmptyTexture( - 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, - textureName ); + 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, url ); if ( texture ) - setDrawable( texture, true ); + setDrawable( texture, false ); + Http::Request::FieldTable headers; + if ( !scene->getReferer().empty() ) + headers["referer"] = scene->getReferer().toString(); Http::getAsync( - [resourceState, resourceGeneration, alive, loadId, texture, + [resourceState, resourceGeneration, alive, loadId, texture, url = std::move( url ), this]( const Http&, Http::Request&, Http::Response& response ) { if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || !alive || !alive->load( std::memory_order_acquire ) || texture == nullptr ) @@ -736,9 +743,13 @@ bool UINodeDrawable::LayerDrawable::loadRemoteDrawable( const std::string& value if ( image.getPixels() != nullptr ) texture->replace( &image ); } ); + } else { + Log::debug( "UINodeDrawable::LayerDrawable::loadRemoteDrawable: could not " + "download image: %s. Error: %d\n%s", + url, response.getStatus(), response.getBody() ); } }, - uri, Seconds( 5 ), {}, {}, "", true, Http::getEnvProxyURI() ); + uri, Seconds( 5 ), {}, headers, "", true, Http::getEnvProxyURI() ); return true; } diff --git a/src/tests/unit_tests/uihtml_tests.cpp b/src/tests/unit_tests/uihtml_tests.cpp index 5b9975231..7b07a914d 100644 --- a/src/tests/unit_tests/uihtml_tests.cpp +++ b/src/tests/unit_tests/uihtml_tests.cpp @@ -3342,6 +3342,39 @@ UTEST( UIBackground, cssFileRelativeSpriteUrlAndNegativePosition ) { Engine::destroySingleton(); } +UTEST( UIBackground, RemoteImageReusesCachedTexture ) { + Engine::instance()->createWindow( WindowSettings( 1024, 768, "remote bg cache reuse", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + UISceneNode* sceneNode = init_test_inline_block(); + const std::string imageURL = "http://127.0.0.1:1/eepp-cached-background.png"; + Texture* cached = TextureFactory::instance()->createEmptyTexture( + 8, 8, 4, Color::White, false, Texture::ClampMode::ClampToEdge, false, false, imageURL ); + ASSERT_TRUE( cached != nullptr ); + + sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( R"html( + +
+
+ + )html" ) ); + sceneNode->updateDirtyLayouts(); + + auto* first = sceneNode->find( "first" ); + auto* second = sceneNode->find( "second" ); + ASSERT_TRUE( first != nullptr ); + ASSERT_TRUE( second != nullptr ); + ASSERT_TRUE( first->getBackground() != nullptr ); + ASSERT_TRUE( second->getBackground() != nullptr ); + ASSERT_TRUE( first->getBackground()->getLayer( 0 ) != nullptr ); + ASSERT_TRUE( second->getBackground()->getLayer( 0 ) != nullptr ); + EXPECT_EQ( cached, first->getBackground()->getLayer( 0 )->getDrawable() ); + EXPECT_EQ( cached, second->getBackground()->getLayer( 0 )->getDrawable() ); + + Engine::destroySingleton(); +} + UTEST( UIBackground, InlineBlockImageSpans ) { auto win = Engine::instance()->createWindow( WindowSettings( 1024, 653, "inline-block image spans", VisualTestWindowStyle, @@ -4257,6 +4290,91 @@ UTEST( UIHTML, DeferredFileImageLoadsAsync ) { Engine::destroySingleton(); } +UTEST( UIHTML, DeferredFileImageReusesCachedTexture ) { + auto win = Engine::instance()->createWindow( + WindowSettings( 1024, 768, "deferred file img cache reuse", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + const std::string processPath = Sys::getProcessPath(); + + UISceneNode* sceneNode = init_test_inline_block(); + sceneNode->setURI( URI( "file://" + processPath ) ); + URI imageURI = sceneNode->solveRelativePath( URI( "../assets/icon/ee.png" ) ); + ASSERT_TRUE( FileSystem::fileExists( imageURI.getFSPath() ) ); + Texture* cached = TextureFactory::instance()->loadFromFile( + imageURI.getFSPath(), false, Texture::ClampMode::ClampToEdge, false, false ); + ASSERT_TRUE( cached != nullptr ); + + sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( R"html( + + + + + + + + )html" ), + sceneNode->getRoot(), String::hash( "deferred-file-cache" ) ); + + win->getInput()->update(); + SceneManager::instance()->update(); + sceneNode->updateDirtyLayouts(); + + auto* firstNode = sceneNode->getRoot()->find( "first-img" ); + auto* secondNode = sceneNode->getRoot()->find( "second-img" ); + ASSERT_TRUE( firstNode != nullptr ); + ASSERT_TRUE( secondNode != nullptr ); + auto* first = firstNode->asType(); + auto* second = secondNode->asType(); + ASSERT_TRUE( first != nullptr ); + ASSERT_TRUE( second != nullptr ); + EXPECT_EQ( cached, first->getDrawable() ); + EXPECT_EQ( cached, second->getDrawable() ); + + Engine::destroySingleton(); +} + +UTEST( UIHTML, RemoteImageReusesCachedTexture ) { + auto win = Engine::instance()->createWindow( + WindowSettings( 1024, 768, "remote img cache reuse", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + const std::string imageURL = "http://127.0.0.1:1/eepp-cached-image.png"; + + UISceneNode* sceneNode = init_test_inline_block(); + Texture* cached = TextureFactory::instance()->createEmptyTexture( + 8, 8, 4, Color::White, false, Texture::ClampMode::ClampToEdge, false, false, imageURL ); + ASSERT_TRUE( cached != nullptr ); + + sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( R"html( + + + + + + + + )html" ), + sceneNode->getRoot(), String::hash( "remote-image-cache" ) ); + + win->getInput()->update(); + SceneManager::instance()->update(); + sceneNode->updateDirtyLayouts(); + + auto* firstNode = sceneNode->getRoot()->find( "first-img" ); + auto* secondNode = sceneNode->getRoot()->find( "second-img" ); + ASSERT_TRUE( firstNode != nullptr ); + ASSERT_TRUE( secondNode != nullptr ); + auto* first = firstNode->asType(); + auto* second = secondNode->asType(); + ASSERT_TRUE( first != nullptr ); + ASSERT_TRUE( second != nullptr ); + EXPECT_EQ( cached, first->getDrawable() ); + EXPECT_EQ( cached, second->getDrawable() ); + + Engine::destroySingleton(); +} + UTEST( UIHTML, DeferredFileImageRelayoutsAncestors ) { auto win = Engine::instance()->createWindow( WindowSettings( 1024, 768, "deferred file img relayout", WindowStyle::Default,