From 5ff638bf7a5d45a007fd95e4baec8f1101f9e264 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= <spartanj@gmail.com>
Date: Tue, 5 Dec 2023 10:42:08 -0300
Subject: [PATCH] Some performance improvements on styles recomputing. Fix in
 UIPushButton.

---
 include/eepp/ui/css/animationdefinition.hpp  |   5 +-
 include/eepp/ui/css/stylesheet.hpp           |  14 +-
 include/eepp/ui/css/transitiondefinition.hpp |   4 +-
 include/eepp/ui/uistyle.hpp                  |  10 +-
 src/eepp/ui/css/animationdefinition.cpp      |   2 +-
 src/eepp/ui/css/stylesheet.cpp               |  24 ++-
 src/eepp/ui/css/transitiondefinition.cpp     |   2 +-
 src/eepp/ui/uipushbutton.cpp                 |  12 +-
 src/eepp/ui/uistyle.cpp                      | 151 ++++++++++---------
 src/eepp/ui/uitabwidget.cpp                  |   2 +-
 10 files changed, 123 insertions(+), 103 deletions(-)

diff --git a/include/eepp/ui/css/animationdefinition.hpp b/include/eepp/ui/css/animationdefinition.hpp
index 81c586275..7f91b296b 100644
--- a/include/eepp/ui/css/animationdefinition.hpp
+++ b/include/eepp/ui/css/animationdefinition.hpp
@@ -4,7 +4,6 @@
 #include <eepp/math/ease.hpp>
 #include <eepp/system/time.hpp>
 #include <eepp/ui/css/stylesheetproperty.hpp>
-#include <unordered_map>
 #include <vector>
 
 using namespace EE::Math;
@@ -14,7 +13,7 @@ namespace EE { namespace UI { namespace CSS {
 
 class EE_API AnimationDefinition {
   public:
-	static std::unordered_map<std::string, AnimationDefinition>
+	static UnorderedMap<std::string, AnimationDefinition>
 	parseAnimationProperties( const std::vector<const StyleSheetProperty*>& stylesheetProperties );
 
 	/* https://developer.mozilla.org/en-US/docs/Web/CSS/animation-direction */
@@ -106,7 +105,7 @@ inline bool operator!=( const AnimationDefinition& a, const AnimationDefinition&
 	return !( a == b );
 }
 
-typedef std::unordered_map<std::string, AnimationDefinition> AnimationsMap;
+typedef UnorderedMap<std::string, AnimationDefinition> AnimationsMap;
 
 }}} // namespace EE::UI::CSS
 
diff --git a/include/eepp/ui/css/stylesheet.hpp b/include/eepp/ui/css/stylesheet.hpp
index 5337edabc..f0b6c68e9 100644
--- a/include/eepp/ui/css/stylesheet.hpp
+++ b/include/eepp/ui/css/stylesheet.hpp
@@ -6,7 +6,6 @@
 #include <eepp/ui/css/mediaquery.hpp>
 #include <eepp/ui/css/stylesheetstyle.hpp>
 #include <memory>
-#include <unordered_map>
 
 namespace EE { namespace UI { namespace CSS {
 
@@ -35,9 +34,9 @@ class EE_API StyleSheet {
 
 	StyleSheetStyleVector getStyleSheetStyleByAtRule( const AtRuleType& atRuleType ) const;
 
-	bool isKeyframesDefined( const std::string& keyframesName );
+	bool isKeyframesDefined( const std::string& keyframesName ) const;
 
-	const KeyframesDefinition& getKeyframesDefinition( const std::string& keyframesName );
+	const KeyframesDefinition& getKeyframesDefinition( const std::string& keyframesName ) const;
 
 	void addKeyframes( const KeyframesDefinition& keyframes );
 
@@ -45,8 +44,6 @@ class EE_API StyleSheet {
 
 	const KeyframesDefinitionMap& getKeyframes() const;
 
-	static size_t nodeHash( const std::string& tag, const std::string& id );
-
 	void invalidateCache();
 
 	const Uint32& getMarker() const;
@@ -58,11 +55,14 @@ class EE_API StyleSheet {
 	bool markerExists( const Uint32& marker ) const;
 
 	std::vector<std::shared_ptr<StyleSheetStyle>>
-	findStyleFromSelectorName( const std::string& selector );
+	findStyleFromSelectorName( const std::string& selector ) const;
 
 	bool refreshCacheFromStyles( const std::vector<std::shared_ptr<StyleSheetStyle>>& styles );
 
+	const Uint64& getVersion() const;
+
   protected:
+	Uint64 mVersion{ 1 };
 	Uint32 mMarker{ 0 };
 	std::vector<std::shared_ptr<StyleSheetStyle>> mNodes;
 	UnorderedMap<size_t, StyleSheetStyleVector> mNodeIndex;
@@ -71,6 +71,8 @@ class EE_API StyleSheet {
 	using ElementDefinitionCache = UnorderedMap<size_t, std::shared_ptr<ElementDefinition>>;
 	mutable ElementDefinitionCache mNodeCache;
 
+	static size_t nodeHash( const std::string& tag, const std::string& id );
+
 	void addMediaQueryList( MediaQueryList::ptr list );
 
 	bool addStyleToNodeIndex( StyleSheetStyle* style );
diff --git a/include/eepp/ui/css/transitiondefinition.hpp b/include/eepp/ui/css/transitiondefinition.hpp
index 3c74a2e63..f8fb2583f 100644
--- a/include/eepp/ui/css/transitiondefinition.hpp
+++ b/include/eepp/ui/css/transitiondefinition.hpp
@@ -14,7 +14,7 @@ namespace EE { namespace UI { namespace CSS {
 
 class EE_API TransitionDefinition {
   public:
-	static std::map<std::string, TransitionDefinition>
+	static UnorderedMap<std::string, TransitionDefinition>
 	parseTransitionProperties( const std::vector<const StyleSheetProperty*>& styleSheetProperties );
 
 	TransitionDefinition() : timingFunction( Ease::Linear ) {}
@@ -36,7 +36,7 @@ class EE_API TransitionDefinition {
 	Time duration = Time::Zero;
 };
 
-typedef std::map<std::string, TransitionDefinition> TransitionsMap;
+typedef UnorderedMap<std::string, TransitionDefinition> TransitionsMap;
 
 }}} // namespace EE::UI::CSS
 
diff --git a/include/eepp/ui/uistyle.hpp b/include/eepp/ui/uistyle.hpp
index 4bc893b4b..074f63741 100644
--- a/include/eepp/ui/uistyle.hpp
+++ b/include/eepp/ui/uistyle.hpp
@@ -9,9 +9,6 @@
 #include <eepp/ui/css/stylesheetstyle.hpp>
 #include <eepp/ui/css/transitiondefinition.hpp>
 #include <eepp/ui/uistate.hpp>
-#include <functional>
-#include <set>
-#include <unordered_set>
 
 namespace EE { namespace Graphics {
 class Font;
@@ -86,10 +83,11 @@ class EE_API UIStyle : public UIState {
 	std::shared_ptr<CSS::ElementDefinition> mDefinition;
 	CSS::TransitionsMap mTransitions;
 	CSS::AnimationsMap mAnimations;
-	std::set<UIWidget*> mRelatedWidgets;
-	std::set<UIWidget*> mSubscribedWidgets;
+	UnorderedSet<UIWidget*> mRelatedWidgets;
+	UnorderedSet<UIWidget*> mSubscribedWidgets;
 	UnorderedSet<UIWidget*> mStructurallyVolatileChilds;
-	size_t mStateDepthCounter{ 0 };
+	Uint32 mStateDepthCounter{ 0 };
+	Uint64 mLoadedVersion{ 0 };
 	bool mChangingState;
 	bool mForceReapplyProperties;
 	bool mDisableAnimations;
diff --git a/src/eepp/ui/css/animationdefinition.cpp b/src/eepp/ui/css/animationdefinition.cpp
index b13dd5e84..6dffc0077 100644
--- a/src/eepp/ui/css/animationdefinition.cpp
+++ b/src/eepp/ui/css/animationdefinition.cpp
@@ -9,7 +9,7 @@ inline bool isTimingFunction( const std::string& str ) {
 	return Ease::Interpolation::None != Ease::fromName( str, Ease::Interpolation::None );
 }
 
-std::unordered_map<std::string, AnimationDefinition> AnimationDefinition::parseAnimationProperties(
+UnorderedMap<std::string, AnimationDefinition> AnimationDefinition::parseAnimationProperties(
 	const std::vector<const StyleSheetProperty*>& stylesheetProperties ) {
 	AnimationsMap animations;
 	std::vector<std::string> names;
diff --git a/src/eepp/ui/css/stylesheet.cpp b/src/eepp/ui/css/stylesheet.cpp
index 0a8a645ec..fc9841ee9 100644
--- a/src/eepp/ui/css/stylesheet.cpp
+++ b/src/eepp/ui/css/stylesheet.cpp
@@ -4,7 +4,6 @@
 #include <eepp/ui/css/stylesheetproperty.hpp>
 #include <eepp/ui/css/stylesheetselector.hpp>
 #include <eepp/ui/uiwidget.hpp>
-#include <iostream>
 
 namespace EE { namespace UI { namespace CSS {
 
@@ -26,6 +25,7 @@ size_t StyleSheet::nodeHash( const std::string& tag, const std::string& id ) {
 
 void StyleSheet::invalidateCache() {
 	mNodeCache.clear();
+	mVersion++;
 }
 
 const Uint32& StyleSheet::getMarker() const {
@@ -107,7 +107,7 @@ bool StyleSheet::markerExists( const Uint32& marker ) const {
 }
 
 std::vector<std::shared_ptr<StyleSheetStyle>>
-StyleSheet::findStyleFromSelectorName( const std::string& selector ) {
+StyleSheet::findStyleFromSelectorName( const std::string& selector ) const {
 	std::vector<std::shared_ptr<StyleSheetStyle>> found;
 	for ( const auto& node : mNodes ) {
 		if ( selector == node->getSelector().getName() )
@@ -121,10 +121,11 @@ bool StyleSheet::refreshCacheFromStyles(
 	bool refreshed = false;
 	for ( const auto& style : styles ) {
 		for ( auto& node : mNodeCache ) {
-			for ( auto& nodeStyle : node.second->getStyles() ) {
+			for ( const auto& nodeStyle : node.second->getStyles() ) {
 				if ( nodeStyle == style.get() ) {
 					node.second->refresh();
 					refreshed = true;
+					mVersion++;
 				}
 			}
 		}
@@ -132,6 +133,10 @@ bool StyleSheet::refreshCacheFromStyles(
 	return refreshed;
 }
 
+const Uint64& StyleSheet::getVersion() const {
+	return mVersion;
+}
+
 bool StyleSheet::addStyleToNodeIndex( StyleSheetStyle* style ) {
 	const std::string& id = style->getSelector().getSelectorId();
 	const std::string& tag = style->getSelector().getSelectorTagName();
@@ -154,6 +159,7 @@ void StyleSheet::addStyle( std::shared_ptr<StyleSheetStyle> node ) {
 		mNodes.push_back( node );
 	}
 	addMediaQueryList( node->getMediaQueryList() );
+	mVersion++;
 }
 
 bool StyleSheet::isEmpty() const {
@@ -237,7 +243,7 @@ std::shared_ptr<ElementDefinition> StyleSheet::getElementStyles( UIWidget* eleme
 
 	auto cacheIterator = mNodeCache.find( seed );
 	if ( cacheIterator != mNodeCache.end() ) {
-		std::shared_ptr<ElementDefinition>& definition = ( *cacheIterator ).second;
+		const std::shared_ptr<ElementDefinition>& definition = ( *cacheIterator ).second;
 		return definition;
 	}
 
@@ -270,6 +276,9 @@ bool StyleSheet::updateMediaLists( const MediaFeatures& features ) {
 			updateStyles = true;
 	}
 
+	if ( updateStyles )
+		mVersion++;
+
 	return updateStyles;
 }
 
@@ -306,11 +315,12 @@ StyleSheetStyleVector StyleSheet::getStyleSheetStyleByAtRule( const AtRuleType&
 	return vector;
 }
 
-bool StyleSheet::isKeyframesDefined( const std::string& keyframesName ) {
+bool StyleSheet::isKeyframesDefined( const std::string& keyframesName ) const {
 	return mKeyframesMap.find( keyframesName ) != mKeyframesMap.end();
 }
 
-const KeyframesDefinition& StyleSheet::getKeyframesDefinition( const std::string& keyframesName ) {
+const KeyframesDefinition&
+StyleSheet::getKeyframesDefinition( const std::string& keyframesName ) const {
 	static KeyframesDefinition EMPTY;
 	const auto& it = mKeyframesMap.find( keyframesName );
 	if ( it != mKeyframesMap.end() ) {
@@ -323,6 +333,8 @@ void StyleSheet::addKeyframes( const KeyframesDefinition& keyframes ) {
 	// "none" is a reserved keyword
 	if ( keyframes.getName() != "none" )
 		mKeyframesMap[keyframes.getName()] = keyframes;
+
+	mVersion++;
 }
 
 void StyleSheet::addKeyframes( const KeyframesDefinitionMap& keyframesMap ) {
diff --git a/src/eepp/ui/css/transitiondefinition.cpp b/src/eepp/ui/css/transitiondefinition.cpp
index 9062dbd98..2cf4b5dd1 100644
--- a/src/eepp/ui/css/transitiondefinition.cpp
+++ b/src/eepp/ui/css/transitiondefinition.cpp
@@ -6,7 +6,7 @@
 
 namespace EE { namespace UI { namespace CSS {
 
-std::map<std::string, TransitionDefinition> TransitionDefinition::parseTransitionProperties(
+UnorderedMap<std::string, TransitionDefinition> TransitionDefinition::parseTransitionProperties(
 	const std::vector<const StyleSheetProperty*>& styleSheetProperties ) {
 	std::vector<std::string> properties;
 	std::vector<Time> durations;
diff --git a/src/eepp/ui/uipushbutton.cpp b/src/eepp/ui/uipushbutton.cpp
index 2885fb5b7..bc70e7033 100644
--- a/src/eepp/ui/uipushbutton.cpp
+++ b/src/eepp/ui/uipushbutton.cpp
@@ -376,8 +376,9 @@ UIImage* UIPushButton::getIcon() {
 		mIcon->addEventListener( Event::OnVisibleChange, cb );
 
 		if ( mIconMinSize != Sizei::Zero ) {
+			auto iconMinSize = mIconMinSize;
 			mIconMinSize = Sizei{ -1, -1 }; // force refresh
-			setIconMinimumSize( mIconMinSize );
+			setIconMinimumSize( iconMinSize );
 		}
 	}
 	return mIcon;
@@ -453,9 +454,9 @@ void UIPushButton::setIconMinimumSize( const Sizei& minIconSize ) {
 	if ( minIconSize != mIconMinSize ) {
 		mIconMinSize = minIconSize;
 
-		if ( mIcon && mIconMinSize.x != 0 && mIconMinSize.y != 0 ) {
-			mIcon->setMinSizeEq( String::fromFloat( mIconMinSize.x, "dp" ),
-								 String::fromFloat( mIconMinSize.y, "dp" ) );
+		if ( mIconMinSize.x != 0 && mIconMinSize.y != 0 ) {
+			getIcon()->setMinSizeEq( String::fromFloat( mIconMinSize.x, "dp" ),
+									 String::fromFloat( mIconMinSize.y, "dp" ) );
 		}
 	}
 }
@@ -639,8 +640,7 @@ bool UIPushButton::applyProperty( const StyleSheetProperty& attribute ) {
 			setTextAsFallback( attribute.asBool() );
 			break;
 		case PropertyId::Tint:
-			if ( mIcon )
-				mIcon->setColor( attribute.asColor() );
+			getIcon()->setColor( attribute.asColor() );
 			break;
 		case PropertyId::Color:
 		case PropertyId::TextShadowColor:
diff --git a/src/eepp/ui/uistyle.cpp b/src/eepp/ui/uistyle.cpp
index 1a83977fb..42443cf41 100644
--- a/src/eepp/ui/uistyle.cpp
+++ b/src/eepp/ui/uistyle.cpp
@@ -55,8 +55,16 @@ void UIStyle::setStyleSheetProperty( const StyleSheetProperty& property ) {
 }
 
 void UIStyle::resetGlobalDefinition() {
-	mGlobalDefinition =
-		mWidget->getUISceneNode()->getStyleSheet().getElementStyles( mWidget, false );
+	const auto& stylesheet = mWidget->getUISceneNode()->getStyleSheet();
+
+	if ( stylesheet.getVersion() == mLoadedVersion )
+		return;
+
+	auto prefDef = mGlobalDefinition;
+
+	mGlobalDefinition = stylesheet.getElementStyles( mWidget, false );
+
+	mLoadedVersion = stylesheet.getVersion();
 }
 
 void UIStyle::load() {
@@ -227,88 +235,89 @@ void UIStyle::applyVarValues( StyleSheetProperty* property ) {
 }
 
 void UIStyle::onStateChange() {
-	if ( NULL != mWidget && NULL != mWidget->getUISceneNode() ) {
-		mChangingState = true;
+	if ( NULL == mWidget || NULL == mWidget->getUISceneNode() )
+		return;
 
-		std::shared_ptr<ElementDefinition> prevDefinition = mDefinition;
-		std::shared_ptr<ElementDefinition> newDefinition =
-			mWidget->getUISceneNode()->getStyleSheet().getElementStyles( mWidget, true );
+	mChangingState = true;
 
-		if ( newDefinition != mDefinition || mForceReapplyProperties ) {
-			PropertyIdSet changedProperties;
+	std::shared_ptr<ElementDefinition> prevDefinition = mDefinition;
+	std::shared_ptr<ElementDefinition> newDefinition =
+		mWidget->getUISceneNode()->getStyleSheet().getElementStyles( mWidget, true );
 
-			if ( mDefinition )
-				changedProperties = mDefinition->getPropertyIds();
+	if ( newDefinition != mDefinition || mForceReapplyProperties ) {
+		PropertyIdSet changedProperties;
 
-			if ( newDefinition )
-				changedProperties |= newDefinition->getPropertyIds();
+		if ( mDefinition )
+			changedProperties = mDefinition->getPropertyIds();
 
-			if ( !mForceReapplyProperties ) {
-				if ( nullptr != newDefinition && !newDefinition->getPropertyIds().empty() ) {
-					if ( nullptr != mDefinition ) {
-						const PropertyIdSet propertiesInBothDefinitions =
-							( mDefinition->getPropertyIds() & newDefinition->getPropertyIds() );
+		if ( newDefinition )
+			changedProperties |= newDefinition->getPropertyIds();
 
-						for ( Uint32 id : propertiesInBothDefinitions ) {
-							const StyleSheetProperty* p0 = mDefinition->getProperty( id );
-							const StyleSheetProperty* p1 = newDefinition->getProperty( id );
-							if ( nullptr != p0 && nullptr != p1 && *p0 == *p1 )
-								changedProperties.erase( id );
-						}
+		if ( !mForceReapplyProperties ) {
+			if ( nullptr != newDefinition && !newDefinition->getPropertyIds().empty() ) {
+				if ( nullptr != mDefinition ) {
+					const PropertyIdSet propertiesInBothDefinitions =
+						( mDefinition->getPropertyIds() & newDefinition->getPropertyIds() );
+
+					for ( Uint32 id : propertiesInBothDefinitions ) {
+						const StyleSheetProperty* p0 = mDefinition->getProperty( id );
+						const StyleSheetProperty* p1 = newDefinition->getProperty( id );
+						if ( nullptr != p0 && nullptr != p1 && *p0 == *p1 )
+							changedProperties.erase( id );
 					}
 				}
 			}
-
-			mDefinition = newDefinition;
-
-			mForceReapplyProperties = false;
-
-			mWidget->beginAttributesTransaction();
-
-			if ( nullptr != mDefinition && !mDefinition->getTransitionProperties().empty() ) {
-				mTransitions = TransitionDefinition::parseTransitionProperties(
-					mDefinition->getTransitionProperties() );
-			}
-
-			for ( auto prop : changedProperties ) {
-				StyleSheetProperty* property = getLocalProperty( prop );
-
-				if ( nullptr == property || NULL == property->getPropertyDefinition() )
-					continue;
-
-				applyVarValues( property );
-
-				if ( property->getPropertyDefinition()->isIndexed() ) {
-					for ( size_t i = 0; i < property->getPropertyIndexCount(); i++ ) {
-						applyVarValues( property->getPropertyIndexRef( i ) );
-
-						applyStyleSheetProperty( property->getPropertyIndex( i ), prevDefinition );
-					}
-				} else {
-					applyStyleSheetProperty( *property, prevDefinition );
-				}
-			}
-
-			updateAnimations();
-
-			mWidget->endAttributesTransaction();
-		}
-
-		mStateDepthCounter++;
-
-		if ( mStateDepthCounter <= 1 ) {
-			for ( auto& related : mRelatedWidgets ) {
-				if ( NULL != related->getUIStyle() ) {
-					related->getUIStyle()->onStateChange();
-				}
-			}
 		}
 
-		mStateDepthCounter--;
+		mDefinition = newDefinition;
 
-		mChangingState = false;
-		mFirstState = false;
+		mForceReapplyProperties = false;
+
+		mWidget->beginAttributesTransaction();
+
+		if ( nullptr != mDefinition && !mDefinition->getTransitionProperties().empty() ) {
+			mTransitions = TransitionDefinition::parseTransitionProperties(
+				mDefinition->getTransitionProperties() );
+		}
+
+		for ( auto prop : changedProperties ) {
+			StyleSheetProperty* property = getLocalProperty( prop );
+
+			if ( nullptr == property || NULL == property->getPropertyDefinition() )
+				continue;
+
+			applyVarValues( property );
+
+			if ( property->getPropertyDefinition()->isIndexed() ) {
+				for ( size_t i = 0; i < property->getPropertyIndexCount(); i++ ) {
+					applyVarValues( property->getPropertyIndexRef( i ) );
+
+					applyStyleSheetProperty( property->getPropertyIndex( i ), prevDefinition );
+				}
+			} else {
+				applyStyleSheetProperty( *property, prevDefinition );
+			}
+		}
+
+		updateAnimations();
+
+		mWidget->endAttributesTransaction();
 	}
+
+	mStateDepthCounter++;
+
+	if ( mStateDepthCounter <= 1 ) {
+		for ( auto& related : mRelatedWidgets ) {
+			if ( NULL != related->getUIStyle() ) {
+				related->getUIStyle()->onStateChange();
+			}
+		}
+	}
+
+	mStateDepthCounter--;
+
+	mChangingState = false;
+	mFirstState = false;
 }
 
 const StyleSheetProperty*
diff --git a/src/eepp/ui/uitabwidget.cpp b/src/eepp/ui/uitabwidget.cpp
index 56346160f..2c2e17b68 100644
--- a/src/eepp/ui/uitabwidget.cpp
+++ b/src/eepp/ui/uitabwidget.cpp
@@ -482,7 +482,7 @@ UITab* UITabWidget::createTab( const String& text, UINode* nodeOwned, Drawable*
 	tab->setVisible( true );
 	tab->setEnabled( true );
 	tab->setOwnedWidget( nodeOwned );
-	tab->reloadStyle( true, true, true );
+	tab->reloadStyle( true, true, false );
 	if ( tab->getCloseButton() ) {
 		tab->getCloseButton()
 			->setVisible( mStyleConfig.TabsClosable && mStyleConfig.TabCloseButtonVisible )