Clean up dirty properties in element style

Include/Rocket/Core/Types.h

@@ -120,8 +120,8 @@ using SmallUnorderedSet = chobo::flat_set< T >;
 typedef std::vector< Element* > ElementList;
 typedef std::vector< String > PseudoClassList;
 typedef std::vector< ElementAnimation > ElementAnimationList;
-typedef SmallUnorderedSet< String > PropertyNameList;
 typedef SmallUnorderedSet< String > AttributeNameList;
+typedef SmallOrderedSet< String > PropertyNameList;
 typedef UnorderedMap< String, Property > PropertyMap;
 typedef SmallUnorderedMap< String, Variant > Dictionary;
 typedef Dictionary ElementAttributes;

Source/Core/Element.cpp

@@ -217,7 +217,9 @@ void Element::Update()
 	UpdateAnimation();
 	AdvanceAnimations();
 
+	if(style->AnyPropertiesDirty())
 	{
+		// @performance: Maybe pass the following as function arguments?
 		using namespace Style;
 		const ComputedValues* parent_values = (parent ? &parent->GetComputedValues() : nullptr);
 		float dp_ratio = 1.0f;

Source/Core/ElementStyle.cpp

@@ -550,12 +550,6 @@ void ElementStyle::DirtyChildDefinitions()
 		element->GetChild(i)->GetStyle()->DirtyDefinition();
 }
 
-// Dirties em-relative properties.
-void ElementStyle::DirtyEmProperties()
-{
-	dirty_properties.SetEmRelativeDirty();
-}
-
 // Dirties rem properties.
 void ElementStyle::DirtyRemProperties()
 {
@@ -599,25 +593,55 @@ void ElementStyle::DirtyDpProperties()
 		element->GetChild(i)->GetStyle()->DirtyDpProperties();
 }
 
+bool ElementStyle::AnyPropertiesDirty() const 
+{
+	return !dirty_properties.Empty(); 
+}
+
 // Sets a single property as dirty.
 void ElementStyle::DirtyProperty(const String& property)
 {
-	dirty_properties.SetDirty(property);
+	dirty_properties.Insert(property);
 }
 
 // Sets a list of properties as dirty.
 void ElementStyle::DirtyProperties(const PropertyNameList& properties)
 {
-	dirty_properties.SetDirty(properties);
+	dirty_properties.Insert(properties);
 }
 
 // Sets a list of our potentially inherited properties as dirtied by an ancestor.
 void ElementStyle::DirtyInheritedProperties(const PropertyNameList& properties)
 {
-	dirty_properties.SetDirty(properties);
+	dirty_properties.Insert(properties);
 	return;
 }
 
+static void DirtyEmProperties(DirtyPropertyList& dirty_properties, Element* element)
+{
+	// Either we can dirty every property, or we can iterate over all properties and see if anyone uses em-units.
+	// Choose whichever is fastest based on benchmarking.
+#if 1
+	// Dirty every property
+	dirty_properties.DirtyAll();
+#else
+	if (dirty_properties.AllDirty())
+		return;
+
+	// Check if any of these are currently em-relative. If so, dirty them.
+	for (auto& property_name : StyleSheetSpecification::GetRegisteredProperties())
+	{
+		// Skip font-size; this is relative to our parent's em, not ours.
+		if (property_name == FONT_SIZE)
+			continue;
+
+		// Get this property from this element. If this is em-relative, then add it to the list to
+		// dirty.
+		if (element->GetProperty(property_name)->unit == Property::EM)
+			dirty_properties.Insert(property_name);
+	}
+#endif
+}
 
 
 DirtyPropertyList ElementStyle::ComputeValues(Style::ComputedValues& values, const Style::ComputedValues* parent_values, const Style::ComputedValues* document_values, bool values_are_default_initialized, float dp_ratio)
@@ -648,8 +672,8 @@ DirtyPropertyList ElementStyle::ComputeValues(Style::ComputedValues& values, con
 		else if (parent_values)
 			values.font_size = parent_values->font_size;
 		
-		if(font_size_before != values.font_size)
-			dirty_properties.SetEmRelativeDirty();
+		if (font_size_before != values.font_size)
+			DirtyEmProperties(dirty_properties, element);
 	}
 
 	const float font_size = values.font_size;
@@ -673,7 +697,7 @@ DirtyPropertyList ElementStyle::ComputeValues(Style::ComputedValues& values, con
 		}
 
 		if(line_height_before != values.line_height.value)
-			dirty_properties.SetDirty(VERTICAL_ALIGN);
+			dirty_properties.Insert(VERTICAL_ALIGN);
 	}
 
 
@@ -866,12 +890,37 @@ DirtyPropertyList ElementStyle::ComputeValues(Style::ComputedValues& values, con
 	}
 
 
-	if (dirty_properties.AnyInheritedDirty())
+	// Next, pass inheritable dirty properties onto our children
+
+	// Static to avoid repeated allocations, requires non-recursion.
+	static PropertyNameList dirty_inherited;
+	dirty_inherited.clear();
+
+	bool all_inherited_dirty = false;
+
+	if (dirty_properties.AllDirty())
+	{
+		all_inherited_dirty = true;
+	}
+	else
+	{
+		// Find all dirtied properties which are also inherited
+		const auto& inherited_properties = StyleSheetSpecification::GetRegisteredInheritedProperties();
+		std::set_union(
+			inherited_properties.begin(), inherited_properties.end(), 
+			dirty_properties.GetList().begin(), dirty_properties.GetList().end(), 
+			std::back_inserter(dirty_inherited.modify_container())
+		);
+	}
+
+	if (all_inherited_dirty || dirty_inherited.size() > 0)
 	{
+		// Dirty inherited properties in our children
+		const auto& dirty_inherited_ref = (all_inherited_dirty ? StyleSheetSpecification::GetRegisteredInheritedProperties() : dirty_inherited);
 		for (int i = 0; i < element->GetNumChildren(true); i++)
 		{
 			auto child = element->GetChild(i);
-			child->GetStyle()->dirty_properties.SetDirty(StyleSheetSpecification::GetRegisteredInheritedProperties());
+			child->GetStyle()->dirty_properties.Insert(dirty_inherited_ref);
 		}
 	}
 

Source/Core/ElementStyle.h

@@ -38,53 +38,35 @@ namespace Core {
 class DirtyPropertyList {
 private:
 	bool all_dirty = false;
-	bool inherited_dirty = false;
-	bool em_relative_dirty = false;
 	PropertyNameList dirty_list;
 
-	inline void CheckInheritance(const String& property_name) {
-		const auto& inherited_properties = StyleSheetSpecification::GetRegisteredInheritedProperties();
-		auto it = inherited_properties.find(property_name);
-		if (it != inherited_properties.end())
-			inherited_dirty = true;
-	}
-
 public:
 	DirtyPropertyList(bool all_dirty = false) : all_dirty(all_dirty) {}
 
-	void SetDirty(const String& property_name) {
+	void Insert(const String& property_name) {
 		if (all_dirty) return;
-		auto result = dirty_list.insert(property_name);
-		if (!inherited_dirty && result.second)
-			CheckInheritance(property_name);
+		dirty_list.insert(property_name);
 	}
-	void SetDirty(const PropertyNameList& properties) {
+	void Insert(const PropertyNameList& properties) {
 		if (all_dirty) return;
 		// @performance: Can be made O(N+M)
 		dirty_list.insert(properties.begin(), properties.end());
-		for(size_t i = 0; i < properties.size() && !inherited_dirty; i++)
-			CheckInheritance(properties.container()[i]);
 	}
-	void SetAllDirty() {
-		Clear();
+	void DirtyAll() {
 		all_dirty = true;
-	}
-	void SetEmRelativeDirty() {
-		em_relative_dirty = true;
+		dirty_list.clear();
 	}
 
 	void Clear() {
 		all_dirty = false;
-		inherited_dirty = false;
-		em_relative_dirty = false;
 		dirty_list.clear();
 	}
 
 	bool Empty() const {
-		return !(all_dirty || inherited_dirty || em_relative_dirty || !dirty_list.empty());
+		return !all_dirty && dirty_list.empty();
 	}
 
-	bool IsDirty(const String & name) const {
+	bool Contains(const String & name) const {
 		if (all_dirty)
 			return true;
 		auto it = dirty_list.find(name);
@@ -95,14 +77,6 @@ public:
 		return all_dirty;
 	}
 
-	bool IsEmRelativeDirty() const {
-		return (all_dirty || em_relative_dirty);
-	}
-
-	bool AnyInheritedDirty() const {
-		return (all_dirty || inherited_dirty);
-	}
-
 	const PropertyNameList& GetList() const {
 		return dirty_list;
 	}
@@ -205,13 +179,14 @@ public:
 	/// Dirty all child definitions
 	void DirtyChildDefinitions();
 
-	// Dirties em-relative properties.
-	void DirtyEmProperties();
-	// Dirties rem properties.
+	/// Dirties rem properties.
 	void DirtyRemProperties();
-	// Dirties dp properties.
+	/// Dirties dp properties.
 	void DirtyDpProperties();
 
+	/// Returns true if any properties are dirty such that computed values need to be recomputed
+	bool AnyPropertiesDirty() const;
+
 	/// Turns the local and inherited properties into computed values for this element. These values can in turn be used during the layout procedure.
 	/// Must be called in correct order, always parent before its children.
 	DirtyPropertyList ComputeValues(Style::ComputedValues& values, const Style::ComputedValues* parent_values, const Style::ComputedValues* document_values, bool values_are_default_initialized, float dp_ratio);