Fix broken resource cache for images and layout drawables.

This commit is contained in:
Martín Lucas Golini
2026-07-05 00:55:37 -03:00
parent ffbfe0a646
commit 4f3e5309a4
4 changed files with 214 additions and 39 deletions
+6
View File
@@ -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. - Favor stack-allocated memory over heap allocations whenever possible.
- Any heap allocation must be heavily justified. - Any heap allocation must be heavily justified.
- Exercise reason: maximize stack use for speed, but actively calculate boundaries to prevent stack-overflows. - 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:** 2. **Protect the Render Loop:**
- Render time is critical. - Render time is critical.
+69 -29
View File
@@ -6,6 +6,7 @@
#include <eepp/graphics/texturefactory.hpp> #include <eepp/graphics/texturefactory.hpp>
#include <eepp/network/http.hpp> #include <eepp/network/http.hpp>
#include <eepp/system/filesystem.hpp> #include <eepp/system/filesystem.hpp>
#include <eepp/system/log.hpp>
#include <eepp/ui/css/drawableimageparser.hpp> #include <eepp/ui/css/drawableimageparser.hpp>
#include <eepp/ui/css/propertydefinition.hpp> #include <eepp/ui/css/propertydefinition.hpp>
#include <eepp/ui/uiicon.hpp> #include <eepp/ui/uiicon.hpp>
@@ -13,8 +14,31 @@
#include <eepp/ui/uiscenenode.hpp> #include <eepp/ui/uiscenenode.hpp>
#include <eepp/window/engine.hpp> #include <eepp/window/engine.hpp>
#include <mutex>
namespace EE { namespace UI { 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<std::mutex> 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() { UIImage* UIImage::New() {
return eeNew( UIImage, () ); return eeNew( UIImage, () );
} }
@@ -305,33 +329,39 @@ bool UIImage::loadFileDrawable( const Network::URI& uri ) {
!Window::Engine::instance()->isSharedGLContextEnabled() ) !Window::Engine::instance()->isSharedGLContextEnabled() )
return false; 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(); auto resourceState = scene->getAsyncResourceLoadState();
Uint64 resourceGeneration = Uint64 resourceGeneration =
resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0;
Uint64 loadId = ++mRemoteImageLoadId;
auto alive = mAsyncImageAlive; auto alive = mAsyncImageAlive;
const std::string filePath = uri.getFSPath();
scene->getThreadPool()->run( scene->getThreadPool()->run( [resourceState, resourceGeneration, alive, loadId,
[resourceState, resourceGeneration, alive, loadId, filePath, this] { filePath = std::move( filePath ),
if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || cacheName = std::move( cacheName ), this] {
!alive || !alive->load( std::memory_order_acquire ) ) if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) ||
return; !alive || !alive->load( std::memory_order_acquire ) )
return;
Texture* texture = TextureFactory::instance()->loadFromFile( Texture* texture = loadFileTextureCached( filePath, cacheName );
filePath, false, Texture::ClampMode::ClampToEdge, false, false ); if ( texture == nullptr )
if ( texture == nullptr ) return;
return;
UISceneNode::runAsyncResourceOnMainThread( UISceneNode::runAsyncResourceOnMainThread(
resourceState, resourceGeneration, [alive, loadId, texture, this]( UISceneNode* ) { resourceState, resourceGeneration, [alive, loadId, texture, this]( UISceneNode* ) {
if ( !alive || !alive->load( std::memory_order_acquire ) || if ( !alive || !alive->load( std::memory_order_acquire ) ||
loadId != mRemoteImageLoadId ) loadId != mRemoteImageLoadId )
return; return;
setDrawable( texture, false ); setDrawable( texture, false );
} ); } );
} ); } );
return true; return true;
} }
@@ -341,24 +371,30 @@ void UIImage::loadRemoteDrawable( const Network::URI& uri ) {
if ( !scene ) if ( !scene )
return; 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(); auto resourceState = scene->getAsyncResourceLoadState();
Uint64 resourceGeneration = Uint64 resourceGeneration =
resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0;
Uint64 loadId = ++mRemoteImageLoadId; Uint64 loadId = ++mRemoteImageLoadId;
auto alive = mAsyncImageAlive; auto alive = mAsyncImageAlive;
const std::string url = uri.toString();
const std::string textureName =
String::format( "__eepp_ui_image_%p_%llu_%u", this,
static_cast<unsigned long long>( loadId ), String::hash( url ) );
Texture* texture = TextureFactory::instance()->createEmptyTexture( Texture* texture = TextureFactory::instance()->createEmptyTexture(
1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, url );
textureName );
if ( texture ) if ( texture )
setDrawable( texture, true ); setDrawable( texture, false );
Http::Request::FieldTable headers;
if ( !scene->getReferer().empty() )
headers["referer"] = scene->getReferer().toString();
Http::getAsync( Http::getAsync(
[resourceState, resourceGeneration, alive, loadId, texture, [resourceState, resourceGeneration, alive, loadId, texture, url = std::move( url ),
this]( const Http&, Http::Request&, Http::Response& response ) { this]( const Http&, Http::Request&, Http::Response& response ) {
if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) ||
!alive || !alive->load( std::memory_order_acquire ) || texture == nullptr ) !alive || !alive->load( std::memory_order_acquire ) || texture == nullptr )
@@ -379,9 +415,13 @@ void UIImage::loadRemoteDrawable( const Network::URI& uri ) {
if ( image.getPixels() != nullptr ) if ( image.getPixels() != nullptr )
texture->replace( &image ); 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() { void UIImage::onSizeChange() {
+21 -10
View File
@@ -7,6 +7,7 @@
#include <eepp/math/easing.hpp> #include <eepp/math/easing.hpp>
#include <eepp/network/http.hpp> #include <eepp/network/http.hpp>
#include <eepp/system/functionstring.hpp> #include <eepp/system/functionstring.hpp>
#include <eepp/system/log.hpp>
#include <eepp/ui/css/stylesheetlength.hpp> #include <eepp/ui/css/stylesheetlength.hpp>
#include <eepp/ui/css/stylesheetspecification.hpp> #include <eepp/ui/css/stylesheetspecification.hpp>
#include <eepp/ui/uinode.hpp> #include <eepp/ui/uinode.hpp>
@@ -698,24 +699,30 @@ bool UINodeDrawable::LayerDrawable::loadRemoteDrawable( const std::string& value
if ( value == mDrawableRef && mDrawable != nullptr ) if ( value == mDrawableRef && mDrawable != nullptr )
return true; 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(); auto resourceState = scene->getAsyncResourceLoadState();
Uint64 resourceGeneration = Uint64 resourceGeneration =
resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0; resourceState ? resourceState->generation.load( std::memory_order_acquire ) : 0;
Uint64 loadId = ++mRemoteDrawableLoadId; Uint64 loadId = ++mRemoteDrawableLoadId;
auto alive = mAsyncDrawableAlive; 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<unsigned long long>( loadId ), String::hash( url ) );
Texture* texture = TextureFactory::instance()->createEmptyTexture( Texture* texture = TextureFactory::instance()->createEmptyTexture(
1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, 1, 1, 4, Color::Transparent, false, Texture::ClampMode::ClampToEdge, false, false, url );
textureName );
if ( texture ) if ( texture )
setDrawable( texture, true ); setDrawable( texture, false );
Http::Request::FieldTable headers;
if ( !scene->getReferer().empty() )
headers["referer"] = scene->getReferer().toString();
Http::getAsync( Http::getAsync(
[resourceState, resourceGeneration, alive, loadId, texture, [resourceState, resourceGeneration, alive, loadId, texture, url = std::move( url ),
this]( const Http&, Http::Request&, Http::Response& response ) { this]( const Http&, Http::Request&, Http::Response& response ) {
if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) || if ( !UISceneNode::isAsyncResourceLoadCurrent( resourceState, resourceGeneration ) ||
!alive || !alive->load( std::memory_order_acquire ) || texture == nullptr ) !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 ) if ( image.getPixels() != nullptr )
texture->replace( &image ); 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; return true;
} }
+118
View File
@@ -3342,6 +3342,39 @@ UTEST( UIBackground, cssFileRelativeSpriteUrlAndNegativePosition ) {
Engine::destroySingleton(); 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(
<body>
<div id="first" style="width:10px;height:10px;background-image:url('http://127.0.0.1:1/eepp-cached-background.png')"></div>
<div id="second" style="width:10px;height:10px;background-image:url('http://127.0.0.1:1/eepp-cached-background.png')"></div>
</body>
)html" ) );
sceneNode->updateDirtyLayouts();
auto* first = sceneNode->find<UIWidget>( "first" );
auto* second = sceneNode->find<UIWidget>( "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 ) { UTEST( UIBackground, InlineBlockImageSpans ) {
auto win = Engine::instance()->createWindow( auto win = Engine::instance()->createWindow(
WindowSettings( 1024, 653, "inline-block image spans", VisualTestWindowStyle, WindowSettings( 1024, 653, "inline-block image spans", VisualTestWindowStyle,
@@ -4257,6 +4290,91 @@ UTEST( UIHTML, DeferredFileImageLoadsAsync ) {
Engine::destroySingleton(); 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(
<!doctype html>
<html>
<body>
<img id="first-img" defer src="../assets/icon/ee.png">
<img id="second-img" defer src="../assets/icon/ee.png">
</body>
</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<UIHTMLImage>();
auto* second = secondNode->asType<UIHTMLImage>();
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(
<!doctype html>
<html>
<body>
<img id="first-img" src="http://127.0.0.1:1/eepp-cached-image.png">
<img id="second-img" src="http://127.0.0.1:1/eepp-cached-image.png">
</body>
</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<UIHTMLImage>();
auto* second = secondNode->asType<UIHTMLImage>();
ASSERT_TRUE( first != nullptr );
ASSERT_TRUE( second != nullptr );
EXPECT_EQ( cached, first->getDrawable() );
EXPECT_EQ( cached, second->getDrawable() );
Engine::destroySingleton();
}
UTEST( UIHTML, DeferredFileImageRelayoutsAncestors ) { UTEST( UIHTML, DeferredFileImageRelayoutsAncestors ) {
auto win = Engine::instance()->createWindow( auto win = Engine::instance()->createWindow(
WindowSettings( 1024, 768, "deferred file img relayout", WindowStyle::Default, WindowSettings( 1024, 768, "deferred file img relayout", WindowStyle::Default,