Browse Source

Don't dirty every property during ElementStyle initialization. Only dirty as necessary. Make sure we dirty inherited properties when inserted into a new parent.

Michael Ragazzon 6 years ago
parent
commit
4b539c167d

+ 3 - 18
Include/RmlUi/Core/PropertyIdSet.h

@@ -57,12 +57,6 @@ public:
 			custom_ids.insert(id);
 	}
 
-	void SetAll() {
-		// We are using PropertyId::Invalid (0) as a flag for all properties set.
-		// However, we set the whole set so that we don't have to do the extra check
-		// for the first bit in Contains().
-		defined_ids.set();
-	}
 	void Clear() {
 		defined_ids.reset();
 		custom_ids.clear();
@@ -72,7 +66,6 @@ public:
 			defined_ids.reset((size_t)id);
 		else
 			custom_ids.erase(id);
-		defined_ids.reset(0);
 	}
 
 	bool Empty() const {
@@ -81,20 +74,12 @@ public:
 	bool Contains(PropertyId id) const {
 		if ((size_t)id < N)
 			return defined_ids.test((size_t)id);
-		else if (defined_ids.test(0))
-			return true;
 		else
 			return custom_ids.count(id) == 1;
 	}
-	bool IsAllSet() const {
-		return defined_ids.test(0);
-	}
 
 	size_t Size() const {
-		if(defined_ids.test(0))
-			return N - 1 + custom_ids.size(); // (don't count PropertyId::Invalid)
-		else
-			return defined_ids.count() + custom_ids.size();
+		return defined_ids.count() + custom_ids.size();
 	}
 
 	// Union with another set
@@ -116,7 +101,7 @@ public:
 	PropertyIdSet& operator&=(const PropertyIdSet& other)
 	{
 		defined_ids &= other.defined_ids;
-		if (custom_ids.size() > 0 && other.custom_ids.size() > 0 && !defined_ids.test(0))
+		if (custom_ids.size() > 0 && other.custom_ids.size() > 0)
 		{
 			for (auto it = custom_ids.begin(); it != custom_ids.end();)
 				if (other.custom_ids.count(*it) == 0)
@@ -135,7 +120,7 @@ public:
 	{
 		PropertyIdSet result;
 		result.defined_ids = (defined_ids & other.defined_ids);
-		if (custom_ids.size() > 0 && other.custom_ids.size() > 0 && !defined_ids.test(0))
+		if (custom_ids.size() > 0 && other.custom_ids.size() > 0)
 		{
 			for (PropertyId id : custom_ids)
 				if (other.custom_ids.count(id) == 1)

+ 7 - 6
Source/Core/Element.cpp

@@ -1410,8 +1410,6 @@ Element* Element::AppendChild(ElementPtr child, bool dom_element)
 		num_non_dom_children++;
 	}
 
-	child_ptr->GetStyle()->DirtyDefinition();
-
 	Element* ancestor = child_ptr;
 	for (int i = 0; i <= ChildNotifyLevels && ancestor; i++, ancestor = ancestor->GetParentNode())
 		ancestor->OnChildAdd(child_ptr);
@@ -1461,8 +1459,6 @@ Element* Element::InsertBefore(ElementPtr child, Element* adjacent_element)
 
 		children.insert(children.begin() + child_index, std::move(child));
 
-		child_ptr->GetStyle()->DirtyDefinition();
-
 		Element* ancestor = child_ptr;
 		for (int i = 0; i <= ChildNotifyLevels && ancestor; i++, ancestor = ancestor->GetParentNode())
 			ancestor->OnChildAdd(child_ptr);
@@ -1501,8 +1497,6 @@ ElementPtr Element::ReplaceChild(ElementPtr inserted_element, Element* replaced_
 	children.insert(insertion_point, std::move(inserted_element));
 	ElementPtr result = RemoveChild(replaced_element);
 
-	inserted_element_ptr->GetStyle()->DirtyDefinition();
-
 	Element* ancestor = inserted_element_ptr;
 	for (int i = 0; i <= ChildNotifyLevels && ancestor; i++, ancestor = ancestor->GetParentNode())
 		ancestor->OnChildAdd(inserted_element_ptr);
@@ -2062,6 +2056,13 @@ void Element::SetParent(Element* _parent)
 
 	parent = _parent;
 
+	if (parent)
+	{
+		// We need to update our definition and make sure we inherit the properties of our new parent.
+		style->DirtyDefinition();
+		style->DirtyInheritedProperties();
+	}
+
 	SetOwnerDocument(parent ? parent->GetOwnerDocument() : nullptr);
 }
 

+ 15 - 40
Source/Core/ElementStyle.cpp

@@ -56,7 +56,6 @@ ElementStyle::ElementStyle(Element* _element)
 	definition = nullptr;
 	element = _element;
 
-	dirty_properties.SetAll();
 	definition_dirty = true;
 }
 
@@ -178,14 +177,7 @@ void ElementStyle::UpdateDefinition()
 		}
 		
 		// Switch the property definitions if the definition has changed.
-		if (!definition && new_definition)
-		{
-			// Since we had no definition before there is a likelihood that everything is dirty.
-			// We could do as in the next else-if block, but this is considerably faster.
-			dirty_properties.SetAll();
-			definition = new_definition;
-		}
-		else if (new_definition != definition)
+		if (new_definition != definition)
 		{
 			PropertyIdSet changed_properties;
 			
@@ -207,9 +199,10 @@ void ElementStyle::UpdateDefinition()
 					if (p0 && p1 && *p0 == *p1)
 						changed_properties.Erase(id);
 				}
-			}
 
-			TransitionPropertyChanges(element, changed_properties, inline_properties, definition.get(), new_definition.get());
+				// Transition changed properties if transition property is set
+				TransitionPropertyChanges(element, changed_properties, inline_properties, definition.get(), new_definition.get());
+			}
 
 			definition = new_definition;
 			
@@ -227,7 +220,6 @@ void ElementStyle::UpdateDefinition()
 // Sets or removes a pseudo-class on the element.
 void ElementStyle::SetPseudoClass(const String& pseudo_class, bool activate)
 {
-
 	bool changed = false;
 
 	if (activate)
@@ -428,6 +420,11 @@ void ElementStyle::DirtyDefinition()
 	definition_dirty = true;
 }
 
+void ElementStyle::DirtyInheritedProperties()
+{
+	dirty_properties = StyleSheetSpecification::GetRegisteredInheritedProperties();
+}
+
 void ElementStyle::DirtyChildDefinitions()
 {
 	for (int i = 0; i < element->GetNumChildren(true); i++)
@@ -492,33 +489,6 @@ void ElementStyle::DirtyProperties(const PropertyIdSet& properties)
 	dirty_properties |= properties;
 }
 
-static void DirtyEmProperties(PropertyIdSet& 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.SetAll();
-#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
-}
-
-
 PropertyIdSet ElementStyle::ComputeValues(Style::ComputedValues& values, const Style::ComputedValues* parent_values, const Style::ComputedValues* document_values, bool values_are_default_initialized, float dp_ratio)
 {
 	if (dirty_properties.Empty())
@@ -543,6 +513,8 @@ PropertyIdSet ElementStyle::ComputeValues(Style::ComputedValues& values, const S
 		values = DefaultComputedValues;
 	}
 
+	bool dirty_em_properties = false;
+
 	// Always do font-size first if dirty, because of em-relative values
 	if(dirty_properties.Contains(PropertyId::FontSize))
 	{
@@ -552,7 +524,7 @@ PropertyIdSet ElementStyle::ComputeValues(Style::ComputedValues& values, const S
 			values.font_size = parent_values->font_size;
 		
 		if (font_size_before != values.font_size)
-			DirtyEmProperties(dirty_properties, element);
+			dirty_em_properties = true;
 	}
 	else
 	{
@@ -624,6 +596,9 @@ PropertyIdSet ElementStyle::ComputeValues(Style::ComputedValues& values, const S
 		const PropertyId id = name_property_pair.first;
 		const Property* p = &name_property_pair.second;
 
+		if (dirty_em_properties && p->unit == Property::EM)
+			dirty_properties.Insert(id);
+
 		using namespace Style;
 
 		switch (id)

+ 6 - 1
Source/Core/ElementStyle.h

@@ -112,9 +112,14 @@ public:
 	/// Number and percentages are resolved by scaling the size of the specified target.
 	float ResolveNumberLengthPercentage(const Property* property, RelativeTarget relative_target) const;
 
-	/// Mark definition and all children dirty
+	/// Mark definition and all children dirty.
 	void DirtyDefinition();
 
+	/// Mark inherited properties dirty.
+	/// Inherited properties will automatically be set when parent inherited properties are changed. However,
+	/// some operations may require to dirty these manually, such as when moving an element into another.
+	void DirtyInheritedProperties();
+
 	/// Dirties all properties with a given unit on the current element and recursively on all children.
 	void DirtyPropertiesWithUnitRecursive(Property::Unit unit);