Преглед на файлове

Optimize GetOwnerDocument: Instead of recursively finding the ancestor document, set it directly when adding children

Michael Ragazzon преди 6 години
родител
ревизия
9bb89061a2
променени са 4 файла, в които са добавени 34 реда и са изтрити 24 реда
  1. 4 2
      Include/Rocket/Core/Element.h
  2. 0 3
      Include/Rocket/Core/ElementDocument.h
  3. 29 14
      Source/Core/Element.cpp
  4. 1 5
      Source/Core/ElementDocument.cpp

+ 4 - 2
Include/Rocket/Core/Element.h

@@ -422,11 +422,11 @@ public:
 
 	/// Gets the object representing the declarations of an element's style attributes.
 	/// @return The element's style.
-	ElementStyle* GetStyle();
+	ElementStyle* GetStyle() const;
 
 	/// Gets the document this element belongs to.
 	/// @return This element's document.
-	virtual ElementDocument* GetOwnerDocument();
+	ElementDocument* GetOwnerDocument() const;
 
 	/// Gets this element's parent node.
 	/// @return This element's parent.
@@ -623,6 +623,8 @@ protected:
 	/// @param[out] content The content of this element and those under it, in XML form.
 	virtual void GetRML(String& content);
 
+	void SetOwnerDocument(ElementDocument* document);
+
 	virtual void OnReferenceDeactivate();
 
 private:

+ 0 - 3
Include/Rocket/Core/ElementDocument.h

@@ -61,9 +61,6 @@ public:
 	/// Process given document header
 	void ProcessHeader(const DocumentHeader* header);
 
-	/// Returns itself as the current document
-	ElementDocument* GetOwnerDocument() override;
-
 	/// Returns the document's context.
 	Context* GetContext();
 

+ 29 - 14
Source/Core/Element.cpp

@@ -1119,21 +1119,23 @@ float Element::GetScrollHeight()
 }
 
 // Gets the object representing the declarations of an element's style attributes.
-ElementStyle* Element::GetStyle()
+ElementStyle* Element::GetStyle() const
 {
 	return style;
 }
 
 // Gets the document this element belongs to.
-ElementDocument* Element::GetOwnerDocument()
+ElementDocument* Element::GetOwnerDocument() const
 {
-	if (parent == NULL)
-		return NULL;
-	
-	if (!owner_document)
+#ifdef ROCKET_DEBUG
+	if (parent && !owner_document)
 	{
-		owner_document = parent->GetOwnerDocument();
+		// Since we have a parent but no owner_document, then we must be a 'loose' element -- that is, constructed
+		// outside of a document and not attached to a child of any element in the hierarchy of a document.
+		// This check ensures that we didn't just forget to set the owner document.
+		ROCKET_ASSERT(!parent->GetOwnerDocument());
 	}
+#endif
 
 	return owner_document;
 }
@@ -1426,9 +1428,6 @@ void Element::InsertBefore(Element* child, Element* adjacent_element)
 // Replaces the second node with the first node.
 bool Element::ReplaceChild(Element* inserted_element, Element* replaced_element)
 {
-	inserted_element->AddReference();
-	inserted_element->SetParent(this);
-
 	ElementList::iterator insertion_point = children.begin();
 	while (insertion_point != children.end() && *insertion_point != replaced_element)
 	{
@@ -1441,6 +1440,9 @@ bool Element::ReplaceChild(Element* inserted_element, Element* replaced_element)
 		return false;
 	}
 
+	inserted_element->AddReference();
+	inserted_element->SetParent(this);
+
 	children.insert(insertion_point, inserted_element);
 	RemoveChild(replaced_element);
 
@@ -2017,6 +2019,17 @@ void Element::GetRML(String& content)
 	}
 }
 
+void Element::SetOwnerDocument(ElementDocument* document)
+{
+	// If this element is a document, then never change owner_document. Otherwise, this can happen when attaching to the root element.
+	if(owner_document != document && owner_document != this)
+	{
+		owner_document = document;
+		for (Element* child : children)
+			child->SetOwnerDocument(document);
+	}
+}
+
 void Element::SetParent(Element* _parent)
 {	
 	// If there's an old parent, detach from it first.
@@ -2024,8 +2037,9 @@ void Element::SetParent(Element* _parent)
 		parent != _parent)
 		parent->RemoveChild(this);
 
-	// Save our parent
 	parent = _parent;
+
+	SetOwnerDocument(parent ? parent->GetOwnerDocument() : nullptr);
 }
 
 void Element::ReleaseElements(ElementList& released_elements)
@@ -2046,7 +2060,10 @@ void Element::ReleaseElements(ElementList& released_elements)
 
 		// Set the parent to NULL unless it's been reparented already.
 		if (element->GetParentNode() == this)
-			element->parent = NULL;
+		{
+			element->parent = nullptr;
+			element->SetOwnerDocument(nullptr);
+		}
 
 		element->RemoveReference();
 	}
@@ -2194,13 +2211,11 @@ void Element::DirtyStackingContext()
 void Element::DirtyStructure()
 {
 	// Clear the cached owner document
-	owner_document = NULL;
 	structure_dirty = true;
 }
 
 void Element::DirtyParentStructure()
 {
-	owner_document = NULL;
 	parent_structure_dirty = true;
 }
 

+ 1 - 5
Source/Core/ElementDocument.cpp

@@ -53,6 +53,7 @@ ElementDocument::ElementDocument(const String& tag) : Element(tag)
 	position_dirty = false;
 
 	ForceLocalStackingContext();
+	SetOwnerDocument(this);
 
 	SetProperty(POSITION, Property(Style::Position::Absolute));
 }
@@ -152,11 +153,6 @@ void ElementDocument::ProcessHeader(const DocumentHeader* document_header)
 	SetProperty(VISIBILITY, Property(Style::Visibility::Hidden));
 }
 
-ElementDocument* ElementDocument::GetOwnerDocument()
-{
-	return this;
-}
-
 // Returns the document's context.
 Context* ElementDocument::GetContext()
 {