From 780ede23f947206daa71bee6665fedd393f264b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Sat, 29 Aug 2026 19:46:37 -0300 Subject: [PATCH] Optimize UISceneNode style invalidation batching Defer dirty-descendant coalescing until style processing instead of scanning the dirty set and walking widget ancestry on every invalidation. Preserve parent-first early-outs and resolve dirty roots against the current widget tree using shared snapshot storage. Clear stale style-state animation entries when widgets are deleted and add regression coverage for large invalidation bursts and reparenting. --- include/eepp/ui/uiscenenode.hpp | 11 +- src/eepp/ui/uiscenenode.cpp | 107 +++++++++---------- src/tests/unit_tests/uiscenenode_tests.cpp | 118 ++++++++++++++++++++- 3 files changed, 172 insertions(+), 64 deletions(-) diff --git a/include/eepp/ui/uiscenenode.hpp b/include/eepp/ui/uiscenenode.hpp index a4d3a119e..b2840353f 100644 --- a/include/eepp/ui/uiscenenode.hpp +++ b/include/eepp/ui/uiscenenode.hpp @@ -490,7 +490,8 @@ class EE_API UISceneNode : public SceneNode { * have its CSS re-applied during the next update cycle. * * @param widget Pointer to the UIWidget to invalidate. - * @param tryReinsert If true, attempts to reposition the widget in the dirty set. + * @param tryReinsert If true, re-evaluates the dirty-ancestor relationship for an already + * queued widget after reparenting. Descendant entries are coalesced during processing. */ void invalidateStyle( UIWidget* widget, bool tryReinsert = false ); @@ -502,7 +503,9 @@ class EE_API UISceneNode : public SceneNode { * * @param widget Pointer to the UIWidget to invalidate. * @param disableCSSAnimations If true, disables CSS animations during the update. - * @param tryReinsert If true, attempts to reposition the widget in the dirty set. + * @param tryReinsert If true, re-evaluates the dirty-ancestor relationship and animation policy + * for an already queued widget after reparenting. Descendant entries are coalesced during + * processing. */ void invalidateStyleState( UIWidget* widget, bool disableCSSAnimations = false, bool tryReinsert = false ); @@ -985,7 +988,9 @@ class EE_API UISceneNode : public SceneNode { UnorderedSet mDirtyStyle; UnorderedSet mDirtyStyleState; UnorderedMap mDirtyStyleStateCSSAnimations; - SmallVector, 64> mDirtyStyleStateSnapshot; + // Shared snapshot storage for style and style-state processing. The bool is ignored by the + // style pass and stores the disable-animations flag for the style-state pass. + SmallVector, 64> mDirtyStylesSnapshot; UnorderedSet mDirtyLayouts; SmallVector mDirtyLayoutsSnapshot; std::vector> mTimes; diff --git a/src/eepp/ui/uiscenenode.cpp b/src/eepp/ui/uiscenenode.cpp index 329f307aa..d1a103597 100644 --- a/src/eepp/ui/uiscenenode.cpp +++ b/src/eepp/ui/uiscenenode.cpp @@ -1241,6 +1241,7 @@ void UISceneNode::onWidgetDelete( Node* node ) { mDirtyStyle.erase( widget ); mDirtyStyleState.erase( widget ); + mDirtyStyleStateCSSAnimations.erase( widget ); } } @@ -1268,6 +1269,17 @@ UIWidget* UISceneNode::getRoot() const { return mRoot; } +template +static bool hasDirtyWidgetAncestor( const UIWidget* node, const DirtyContainer& dirty ) { + Node* parent = node->getParent(); + while ( parent != nullptr ) { + if ( parent->isWidget() && dirty.count( parent->asType() ) > 0 ) + return true; + parent = parent->getParent(); + } + return false; +} + void UISceneNode::invalidateStyle( UIWidget* node, bool tryReinsert ) { eeASSERT( NULL != node ); @@ -1278,27 +1290,8 @@ void UISceneNode::invalidateStyle( UIWidget* node, bool tryReinsert ) { if ( alreadyExists && !tryReinsert ) return; - // Any parent dirty? - Node* parent = node->getParent(); - while ( parent != nullptr ) { - if ( parent->isWidget() && mDirtyStyle.count( parent->asType() ) > 0 ) - return; - parent = parent->getParent(); - } - - // Now that we know we aren't early-outing, handle the reinsertion erase - if ( alreadyExists && tryReinsert ) - mDirtyStyle.erase( node ); - - SmallVector eraseList; - - // Any child in list? remove it - for ( auto widget : mDirtyStyle ) - if ( NULL == widget || node->isParentOf( widget ) ) - eraseList.push_back( widget ); - - for ( auto widget : eraseList ) - mDirtyStyle.erase( widget ); + if ( hasDirtyWidgetAncestor( node, mDirtyStyle ) ) + return; mDirtyStyle.insert( node ); } @@ -1310,33 +1303,12 @@ void UISceneNode::invalidateStyleState( UIWidget* node, bool disableCSSAnimation if ( node->isClosing() ) return; - // Already invalidated? - if ( mDirtyStyleState.count( node ) > 0 ) { - if ( !tryReinsert ) - return; - else - mDirtyStyleState.erase( node ); - } + bool alreadyExists = mDirtyStyleState.count( node ) > 0; + if ( alreadyExists && !tryReinsert ) + return; - // Any parent dirty? - Node* parent = node->getParent(); - while ( parent != nullptr ) { - if ( parent->isWidget() && mDirtyStyleState.count( parent->asType() ) > 0 ) - return; - parent = parent->getParent(); - } - - SmallVector eraseList; - - // Any child in list? remove it - for ( auto widget : mDirtyStyleState ) - if ( NULL == widget || node->isParentOf( widget ) ) - eraseList.push_back( widget ); - - for ( auto widget : eraseList ) { - mDirtyStyleState.erase( widget ); - mDirtyStyleStateCSSAnimations.erase( widget ); - } + if ( hasDirtyWidgetAncestor( node, mDirtyStyleState ) ) + return; mDirtyStyleState.insert( node ); mDirtyStyleStateCSSAnimations[node] = disableCSSAnimations; @@ -1461,11 +1433,26 @@ void UISceneNode::updateDirtyLayouts() { void UISceneNode::updateDirtyStyles() { if ( !mDirtyStyle.empty() ) { Clock clock; - for ( auto& node : mDirtyStyle ) { - node->reloadStyle( true, false, false ); + + // Coalesce only once per pass. Eagerly searching the complete dirty set for descendants on + // every invalidation makes bursts quadratic and repeatedly pointer-chases unrelated widget + // ancestry. Retaining descendant entries until this point turns queueing into an ancestor + // walk plus an O(1) insertion. The current tree also naturally resolves reparented widgets. + // + // Clear the live set before applying styles: style application may create widgets or change + // selectors, and those invalidations must remain queued for the next invalidation-depth + // pass. + mDirtyStylesSnapshot.clear(); + mDirtyStylesSnapshot.reserve( mDirtyStyle.size() ); + for ( UIWidget* node : mDirtyStyle ) { + if ( node != nullptr && !hasDirtyWidgetAncestor( node, mDirtyStyle ) ) + mDirtyStylesSnapshot.emplace_back( node, false ); } mDirtyStyle.clear(); + for ( const auto& dirtyStyle : mDirtyStylesSnapshot ) + dirtyStyle.first->reloadStyle( true, false, false ); + if ( mVerbose ) Log::info( "CSS Styles Reloaded in %.2f ms", clock.getElapsedTime().asMilliseconds() ); } @@ -1475,21 +1462,23 @@ void UISceneNode::updateDirtyStyleStates() { if ( !mDirtyStyleState.empty() ) { Clock clock; - // Applying a style state can create widgets (for example a button icon). Widget - // construction invalidates its style state, so iterating mDirtyStyleState directly would - // mutate and potentially reallocate its vector-backed unordered_dense storage. Snapshot the - // current pass and leave new invalidations queued for the outer invalidation-depth loop. - mDirtyStyleStateSnapshot.clear(); - mDirtyStyleStateSnapshot.reserve( mDirtyStyleState.size() ); + // Applying a style state can create widgets (for example a button icon). Coalesce the + // current roots into the shared snapshot, then leave new invalidations queued for the outer + // invalidation-depth loop. When both an ancestor and descendant are dirty, the ancestor's + // animation policy wins, matching the previous eager-coalescing behavior. + mDirtyStylesSnapshot.clear(); + mDirtyStylesSnapshot.reserve( mDirtyStyleState.size() ); for ( UIWidget* node : mDirtyStyleState ) { - auto animations = mDirtyStyleStateCSSAnimations.find( node ); - mDirtyStyleStateSnapshot.emplace_back( - node, animations != mDirtyStyleStateCSSAnimations.end() && animations->second ); + if ( node != nullptr && !hasDirtyWidgetAncestor( node, mDirtyStyleState ) ) { + auto animations = mDirtyStyleStateCSSAnimations.find( node ); + mDirtyStylesSnapshot.emplace_back( + node, animations != mDirtyStyleStateCSSAnimations.end() && animations->second ); + } } mDirtyStyleState.clear(); mDirtyStyleStateCSSAnimations.clear(); - for ( const auto& dirtyState : mDirtyStyleStateSnapshot ) + for ( const auto& dirtyState : mDirtyStylesSnapshot ) dirtyState.first->reportStyleStateChangeRecursive( dirtyState.second ); if ( mVerbose ) diff --git a/src/tests/unit_tests/uiscenenode_tests.cpp b/src/tests/unit_tests/uiscenenode_tests.cpp index 317c50908..40d737ad1 100644 --- a/src/tests/unit_tests/uiscenenode_tests.cpp +++ b/src/tests/unit_tests/uiscenenode_tests.cpp @@ -29,22 +29,50 @@ UTEST( UISceneNode, CssPointerCursorUsesHandCursor ) { EXPECT_STREQ( Cursor::toName( Cursor::Arrow ), "arrow" ); } -static UISceneNode* init_test_scene_node() { +static void init_test_scene_node( UISceneNode* sceneNode ) { FileSystem::changeWorkingDirectory( Sys::getProcessPath() ); FontTrueType* font = FontTrueType::New( "NotoSans-Regular" ).get(); font->loadFromFile( "../assets/fonts/NotoSans-Regular.ttf" ); FontFamily::loadFromRegular( font ); FontTrueType* monoFont = FontTrueType::New( "monospace" ).get(); monoFont->loadFromFile( "../assets/fonts/NotoSans-Regular.ttf" ); - UISceneNode* sceneNode = UISceneNode::New(); SceneManager::instance()->add( sceneNode ); SceneManager::instance()->setCurrentUISceneNode( sceneNode ); UIThemeManager* themeManager = sceneNode->getUIThemeManager(); themeManager->setDefaultFont( font ); themeManager->applyDefaultTheme( sceneNode->getRoot() ); +} + +static UISceneNode* init_test_scene_node() { + UISceneNode* sceneNode = UISceneNode::New(); + init_test_scene_node( sceneNode ); return sceneNode; } +class InvalidationTestSceneNode : public UISceneNode { + public: + static InvalidationTestSceneNode* New() { return eeNew( InvalidationTestSceneNode, () ); } + + size_t pendingStyleCount() const { return mDirtyStyle.size(); } + + size_t pendingStyleStateCount() const { return mDirtyStyleState.size(); } + + size_t pendingStyleStateAnimationCount() const { return mDirtyStyleStateCSSAnimations.size(); } + + size_t processedStyleRootCount() const { return mDirtyStylesSnapshot.size(); } + + UIWidget* processedStyleRoot( size_t index ) const { + return mDirtyStylesSnapshot[index].first; + } + + bool processedStyleRootDisablesAnimations( size_t index ) const { + return mDirtyStylesSnapshot[index].second; + } + + protected: + InvalidationTestSceneNode() : UISceneNode() {} +}; + UTEST( UISceneNode, ViewportMetricsAreIndependentFromSceneExtent ) { Engine::instance()->createWindow( WindowSettings( 1024, 768, "Scene Viewport Metrics Test", WindowStyle::Default, WindowBackend::Default, @@ -394,6 +422,92 @@ UTEST( UISceneNode, StyleStateUpdateAllowsWidgetCreation ) { Engine::destroySingleton(); } +UTEST( UISceneNode, StyleInvalidationCoalescesAtProcessingTime ) { + Engine::instance()->createWindow( WindowSettings( 1024, 768, "Deferred Style Invalidation Test", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + + auto* sceneNode = InvalidationTestSceneNode::New(); + init_test_scene_node( sceneNode ); + sceneNode->flushDirtyStyleAndLayout(); + + UIWidget* parent = UIWidget::New(); + parent->setParent( sceneNode->getRoot() ); + sceneNode->flushDirtyStyleAndLayout(); + + constexpr size_t childCount = 2048; + UIWidget* firstChild = nullptr; + for ( size_t i = 0; i < childCount; ++i ) { + UIWidget* child = UIWidget::New(); + child->setParent( parent ); + if ( !firstChild ) + firstChild = child; + } + + EXPECT_EQ( childCount, sceneNode->pendingStyleCount() ); + EXPECT_EQ( childCount, sceneNode->pendingStyleStateCount() ); + + sceneNode->invalidateStyle( parent ); + sceneNode->invalidateStyleState( parent, true ); + EXPECT_EQ( childCount + 1, sceneNode->pendingStyleCount() ); + EXPECT_EQ( childCount + 1, sceneNode->pendingStyleStateCount() ); + + sceneNode->updateDirtyStyles(); + EXPECT_EQ( size_t{ 1 }, sceneNode->processedStyleRootCount() ); + EXPECT_EQ( parent, sceneNode->processedStyleRoot( 0 ) ); + EXPECT_EQ( size_t{ 0 }, sceneNode->pendingStyleCount() ); + + sceneNode->updateDirtyStyleStates(); + EXPECT_EQ( size_t{ 1 }, sceneNode->processedStyleRootCount() ); + EXPECT_EQ( parent, sceneNode->processedStyleRoot( 0 ) ); + EXPECT_TRUE( sceneNode->processedStyleRootDisablesAnimations( 0 ) ); + EXPECT_EQ( size_t{ 0 }, sceneNode->pendingStyleStateCount() ); + + // The existing ancestor fast path still avoids queueing descendants when the parent arrives + // first, and the parent's animation policy governs the recursive state update. + sceneNode->invalidateStyle( parent ); + sceneNode->invalidateStyle( firstChild ); + sceneNode->invalidateStyleState( parent, false ); + sceneNode->invalidateStyleState( firstChild, true ); + EXPECT_EQ( size_t{ 1 }, sceneNode->pendingStyleCount() ); + EXPECT_EQ( size_t{ 1 }, sceneNode->pendingStyleStateCount() ); + sceneNode->updateDirtyStyles(); + EXPECT_EQ( parent, sceneNode->processedStyleRoot( 0 ) ); + sceneNode->updateDirtyStyleStates(); + EXPECT_EQ( parent, sceneNode->processedStyleRoot( 0 ) ); + EXPECT_FALSE( sceneNode->processedStyleRootDisablesAnimations( 0 ) ); + + // Reparenting an already-dirty widget under a dirty parent can leave both entries queued. The + // current tree must decide which root gets processed. + UIWidget* reparentTarget = UIWidget::New(); + reparentTarget->setParent( sceneNode->getRoot() ); + sceneNode->flushDirtyStyleAndLayout(); + sceneNode->invalidateStyle( firstChild ); + sceneNode->invalidateStyleState( firstChild ); + sceneNode->invalidateStyle( reparentTarget ); + sceneNode->invalidateStyleState( reparentTarget, true ); + firstChild->setParent( reparentTarget ); + sceneNode->updateDirtyStyles(); + EXPECT_EQ( size_t{ 1 }, sceneNode->processedStyleRootCount() ); + EXPECT_EQ( reparentTarget, sceneNode->processedStyleRoot( 0 ) ); + sceneNode->updateDirtyStyleStates(); + EXPECT_EQ( size_t{ 1 }, sceneNode->processedStyleRootCount() ); + EXPECT_EQ( reparentTarget, sceneNode->processedStyleRoot( 0 ) ); + EXPECT_TRUE( sceneNode->processedStyleRootDisablesAnimations( 0 ) ); + + UIWidget* deletedWidget = UIWidget::New(); + deletedWidget->setParent( sceneNode->getRoot() ); + sceneNode->flushDirtyStyleAndLayout(); + sceneNode->invalidateStyleState( deletedWidget, true ); + EXPECT_EQ( size_t{ 1 }, sceneNode->pendingStyleStateAnimationCount() ); + eeDelete( deletedWidget ); + EXPECT_EQ( size_t{ 0 }, sceneNode->pendingStyleStateCount() ); + EXPECT_EQ( size_t{ 0 }, sceneNode->pendingStyleStateAnimationCount() ); + + Engine::destroySingleton(); +} + UTEST( UIWindow, ModalWindowStopsKeyBindingsFromReachingScene ) { Engine::instance()->createWindow( WindowSettings( 1024, 768, "Modal Key Binding Test", WindowStyle::Default, WindowBackend::Default,