Reduce allocations during Element construction, remove Geometry database, rework some of the Render, Update, and UpdateLayout logic.

Include/Rocket/Core/Context.h

@@ -81,7 +81,6 @@ public:
 	/// @return The current dimensions of the context.
 	const Vector2i& GetDimensions() const;
 
-
 	/// Returns the current state of the view.
 	const ViewState& GetViewState() const noexcept;
 
@@ -92,8 +91,8 @@ public:
 	/// @return The current density-independent pixel ratio of the context.
 	float GetDensityIndependentPixelRatio() const;
 
-
-	/// Updates all elements in the context's documents.
+	/// Updates all elements in the context's documents. 
+	/// This must be called before Context::Render, but after any elements have been changed, added or removed.
 	bool Update();
 	/// Renders all visible elements in the context's documents.
 	bool Render();

Include/Rocket/Core/Element.h

@@ -61,6 +61,7 @@ class FontFaceHandle;
 class PropertyDictionary;
 class RenderInterface;
 class StyleSheet;
+struct ElementMeta;
 
 /**
 	A generic element in the DOM tree.
@@ -77,9 +78,6 @@ public:
 	Element(const String& tag);
 	virtual ~Element();
 
-	void Update();
-	void Render();
-
 	/// Clones this element, returning a new, unparented element.
 	Element* Clone() const;
 
@@ -624,11 +622,11 @@ public:
 	/// Called for every event sent to this element or one of its descendants.
 	/// @param[in] event The event to process.
 	virtual void ProcessEvent(Event& event);
-	
-	/// Update the element's layout if required.
-	void UpdateLayout();
 
 protected:
+	void Update();
+	void Render();
+
 	/// Forces the element to generate a local stacking context, regardless of the value of its z-index
 	/// property.
 	void ForceLocalStackingContext();
@@ -799,6 +797,8 @@ private:
 	ElementAnimationList animations;
 	bool dirty_animation;
 
+	ElementMeta* element_meta;
+
 	friend class Context;
 	friend class ElementStyle;
 	friend class LayoutEngine;

Include/Rocket/Core/ElementDocument.h

@@ -126,20 +126,15 @@ public:
 	/// @param[in] source_name Name of the the script the source comes from, useful for debug information.
 	virtual void LoadScript(Stream* stream, const String& source_name);
 
-	/// Updates the layout if necessary.
-	/// The extra argument was added such that layout updates can be done only once per frame
-	inline void UpdateLayout(bool i_really_mean_it = false) { if (i_really_mean_it && layout_dirty && lock_layout == 0) _UpdateLayout(); }
-
-	/// Updates the position of the document based on the style properties.
-	void UpdatePosition();
+	/// Updates the document, including its layout. Users must call this manually before requesting information such as 
+	/// size or position of an element if any element in the document was recently changed, unless Context::Update has
+	/// already been called after the change. This has a perfomance penalty, only call when strictly necessary.
+	void UpdateDocument();
 	
 	/// Increment/Decrement the layout lock
 	void LockLayout(bool lock);
 	
 protected:
-	/// Refreshes the document layout if required.
-	virtual void OnUpdate();
-
 	/// Repositions the document if necessary.
 	virtual void OnPropertyChange(const PropertyNameList& changed_properties);
 
@@ -156,11 +151,17 @@ protected:
 	virtual void ProcessEvent(Event& event);
 
 private:
-	// Find the next element to focus, starting at the current element
+	/// Find the next element to focus, starting at the current element
 	bool FocusNextTabElement(Element* current_element, bool forward);
 	/// Searches forwards or backwards for a focusable element in the given substree
 	bool SearchFocusSubtree(Element* element, bool forward);
 
+	/// Updates the layout if necessary.
+	void UpdateLayout();
+
+	/// Updates the position of the document based on the style properties.
+	void UpdatePosition();
+
 	// Title of the document
 	String title;
 
@@ -181,8 +182,7 @@ private:
 
 	friend class Context;
 	friend class Factory;
-	
-	void _UpdateLayout();
+
 };
 
 }

Include/Rocket/Core/ElementScroll.h

@@ -53,7 +53,7 @@ public:
 	};
 
 	ElementScroll(Element* element);
-	virtual ~ElementScroll();
+	~ElementScroll();
 
 	/// Updates the increment / decrement arrows.
 	void Update();
@@ -82,6 +82,9 @@ public:
 	/// Formats the enabled scrollbars based on the current size of the host element.
 	void FormatScrollbars();
 
+	/// Clears the scrollbars, resetting it to initial conditions.
+	void ClearScrollbars();
+
 protected:
 	/// Handles the 'onchange' events for the scrollbars.
 	void ProcessEvent(Event& event);

Samples/basic/animation/src/main.cpp

@@ -38,8 +38,6 @@
 
 // Animations TODO:
 //  - Update transform animations / resolve keys again when parent box size changes.
-//  - RCSS support? Both @keyframes and transition, maybe.
-//  - Profiling
 //  - [offtopic] Improve performance of transform parser (hashtable)
 //  - [offtopic] Use double for absolute time, get and cache time for each render/update loop
 
@@ -134,17 +132,17 @@ public:
 		  FPS values
 		  Original: 18.5
 		  Without property counter: 22.0
-		  With std::string: 23.0
-		  robin_hood unordered_flat_map: 24.0
+		  With std::string: 23.0  [603fd40]
+		  robin_hood unordered_flat_map: 24.0  [709852f]
 		  Avoid dirtying em's: 27.5
+		  Restructuring update loop: 34.5  [f9892a9]
+		  Element constructor, remove geometry database, remove update() from Context::render: 38.0
 		
 		*/
 		
 		if (!document)
 			return;
 
-		auto el = document->GetElementById("performance");
-
 		Rocket::Core::String rml;
 
 		for (int i = 0; i < 50; i++)
@@ -173,7 +171,7 @@ public:
 			rml += rml_row;
 		}
 
-		if (el)
+		if (auto el = document->GetElementById("performance"))
 			el->SetInnerRML(rml);
 	}
 

Source/Core/Context.cpp

@@ -166,8 +166,8 @@ float Context::GetDensityIndependentPixelRatio() const
 // Updates all elements in the element tree.
 bool Context::Update()
 {
-#ifdef _DEBUG
-	// Reset all document layout locks (work-around due to leak, possibly in select element?)
+#ifdef ROCKET_DEBUG
+	// Look for leaks in layout lock
 	for (int i = 0; i < root->GetNumChildren(); ++i)
 	{
 		ElementDocument* document = root->GetChild(i)->GetOwnerDocument();
@@ -181,6 +181,10 @@ bool Context::Update()
 
 	root->Update();
 
+	for (int i = 0; i < root->GetNumChildren(); ++i)
+		if (auto doc = root->GetChild(i)->GetOwnerDocument())
+			doc->UpdateLayout();
+
 	// Release any documents that were unloaded during the update.
 	ReleaseUnloadedDocuments();
 
@@ -194,14 +198,6 @@ bool Context::Render()
 	if (render_interface == NULL)
 		return false;
 
-	root->Update();
-
-	// Update the layout for all documents in the root. This is done now as events during the
-	// update may have caused elements to require an update.
-	for (int i = 0; i < root->GetNumChildren(); ++i)
-		if (auto doc = root->GetChild(i)->GetOwnerDocument())
-			doc->UpdateLayout(true);
-
 	render_interface->context = this;
 	ElementUtilities::ApplyActiveClipRegion(this, render_interface);
 
@@ -254,7 +250,6 @@ ElementDocument* Context::CreateDocument(const String& tag)
 // Load a document into the context.
 ElementDocument* Context::LoadDocument(const String& document_path)
 {	
-	// Open the stream based on the file path
 	StreamFile* stream = new StreamFile();
 	if (!stream->Open(document_path))
 	{
@@ -262,7 +257,6 @@ ElementDocument* Context::LoadDocument(const String& document_path)
 		return NULL;
 	}
 
-	// Load the document from the stream
 	ElementDocument* document = LoadDocument(stream);
 
 	stream->RemoveReference();
@@ -275,18 +269,15 @@ ElementDocument* Context::LoadDocument(Stream* stream)
 {
 	PluginRegistry::NotifyDocumentOpen(this, stream->GetSourceURL().GetURL());
 
-	// Load the document from the stream.
 	ElementDocument* document = Factory::InstanceDocumentStream(this, stream);
 	if (!document)
 		return NULL;
 
 	root->AppendChild(document);
 
-	// Bind the events, run the layout and fire the 'onload' event.
 	ElementUtilities::BindEventAttributes(document);
-	document->UpdateLayout();
+	document->UpdateDocument();
 
-	// Dispatch the load notifications.
 	PluginRegistry::NotifyDocumentLoad(document);
 	document->DispatchEvent(LOAD, Dictionary(), false);
 
@@ -1002,10 +993,6 @@ void Context::UpdateHoverChain(const Dictionary& parameters, const Dictionary& d
 // Returns the youngest descendent of the given element which is under the given point in screen coodinates.
 Element* Context::GetElementAtPoint(const Vector2f& point, const Element* ignore_element, Element* element)
 {
-	// Update the layout on all documents prior to this call.
-	for (int i = 0; i < GetNumDocuments(); ++i)
-		GetDocument(i)->UpdateLayout();
-
 	if (element == NULL)
 	{
 		if (ignore_element == root)

Source/Core/Element.cpp

@@ -77,6 +77,18 @@ public:
 	}
 };
 
+// Meta objects for element collected in a single struct to reduce memory allocations
+struct ElementMeta 
+{
+	ElementMeta(Element* el) : event_dispatcher(el), style(el), background(el), border(el), decoration(el), scroll(el)  {}
+	EventDispatcher event_dispatcher;
+	ElementStyle style;
+	ElementBackground background;
+	ElementBorder border;
+	ElementDecoration decoration;
+	ElementScroll scroll;
+};
+
 /// Constructs a new libRocket element.
 Element::Element(const String& _tag) : relative_offset_base(0, 0), relative_offset_position(0, 0), absolute_offset(0, 0), scroll_offset(0, 0), boxes(1), content_offset(0, 0), content_box(0, 0), 
 transform_state(), transform_state_perspective_dirty(true), transform_state_transform_dirty(true), transform_state_parent_transform_dirty(true), dirty_animation(false)
@@ -111,12 +123,14 @@ transform_state(), transform_state_perspective_dirty(true), transform_state_tran
 	clipping_enabled = false;
 	clipping_state_dirty = true;
 
-	event_dispatcher = new EventDispatcher(this);
-	style = new ElementStyle(this);
-	background = new ElementBackground(this);
-	border = new ElementBorder(this);
-	decoration = new ElementDecoration(this);
-	scroll = new ElementScroll(this);
+	element_meta = new ElementMeta(this);
+
+	event_dispatcher = &element_meta->event_dispatcher;
+	style = &element_meta->style;
+	background = &element_meta->background;
+	border = &element_meta->border;
+	decoration = &element_meta->decoration;
+	scroll = &element_meta->scroll;
 }
 
 Element::~Element()
@@ -125,8 +139,8 @@ Element::~Element()
 
 	PluginRegistry::NotifyElementDestroy(this);
 
-	// Delete the scroll funtionality before we delete the children!
-	delete scroll;
+	// Remove scrollbar elements before we delete the children!
+	scroll->ClearScrollbars();
 
 	while (!children.empty())
 	{
@@ -144,11 +158,7 @@ Element::~Element()
 	// Release all deleted children.
 	ReleaseElements(deleted_children);
 
-	delete decoration;
-	delete border;
-	delete background;
-	delete style;
-	delete event_dispatcher;
+	delete element_meta;
 
 	if (font_face_handle != NULL)
 		font_face_handle->RemoveReference();
@@ -162,29 +172,22 @@ void Element::Update()
 	ReleaseElements(deleted_children);
 	active_children = children;
 
-	// Force a definition reload, if necessary.
+	OnUpdate();
+
 	style->UpdateDefinition();
 	scroll->Update();
 
 	UpdateDirtyProperties();
 
-	// Update and advance animations, if necessary.
 	UpdateAnimation();
 	AdvanceAnimations();
 
-	// Update the transform state, if necessary.
 	UpdateTransformState();
 
 	for (size_t i = 0; i < active_children.size(); i++)
 		active_children[i]->Update();
-
-	UpdateDirtyProperties();
-
-	OnUpdate();
 }
 
-
-
 void Element::Render()
 {
 	// Rebuild our stacking context if necessary.
@@ -354,14 +357,12 @@ void Element::SetOffset(const Vector2f& offset, Element* _offset_parent, bool _o
 // Returns the position of the top-left corner of one of the areas of this element's primary box.
 Vector2f Element::GetRelativeOffset(Box::Area area)
 {
-	UpdateLayout();
 	return relative_offset_base + relative_offset_position + GetBox().GetPosition(area);
 }
 
 // Returns the position of the top-left corner of one of the areas of this element's primary box.
 Vector2f Element::GetAbsoluteOffset(Box::Area area)
 {
-	UpdateLayout();
 	if (offset_dirty)
 	{
 		offset_dirty = false;
@@ -451,8 +452,6 @@ void Element::AddBox(const Box& box)
 // Returns one of the boxes describing the size of the element.
 const Box& Element::GetBox(int index)
 {
-	UpdateLayout();
-
 	if (index < 0)
 		return boxes[0];
 	else if (index >= GetNumBoxes())
@@ -464,7 +463,6 @@ const Box& Element::GetBox(int index)
 // Returns the number of boxes making up this element's geometry.
 int Element::GetNumBoxes()
 {
-	UpdateLayout();
 	return (int) boxes.size();
 }
 
@@ -1055,28 +1053,24 @@ float Element::GetAbsoluteTop()
 // Gets the width of the left border of an element.
 float Element::GetClientLeft()
 {
-	UpdateLayout();
 	return GetBox().GetPosition(client_area).x;
 }
 
 // Gets the height of the top border of an element.
 float Element::GetClientTop()
 {
-	UpdateLayout();
 	return GetBox().GetPosition(client_area).y;
 }
 
 // Gets the inner width of the element.
 float Element::GetClientWidth()
 {
-	UpdateLayout();
 	return GetBox().GetSize(client_area).x - scroll->GetScrollbarSize(ElementScroll::VERTICAL);
 }
 
 // Gets the inner height of the element.
 float Element::GetClientHeight()
 {
-	UpdateLayout();
 	return GetBox().GetSize(client_area).y - scroll->GetScrollbarSize(ElementScroll::HORIZONTAL);
 }
 
@@ -1089,35 +1083,30 @@ Element* Element::GetOffsetParent()
 // Gets the distance from this element's left border to its offset parent's left border.
 float Element::GetOffsetLeft()
 {
-	UpdateLayout();
 	return relative_offset_base.x + relative_offset_position.x;
 }
 
 // Gets the distance from this element's top border to its offset parent's top border.
 float Element::GetOffsetTop()
 {
-	UpdateLayout();
 	return relative_offset_base.y + relative_offset_position.y;
 }
 
 // Gets the width of the element, including the client area, padding, borders and scrollbars, but not margins.
 float Element::GetOffsetWidth()
 {
-	UpdateLayout();
 	return GetBox().GetSize(Box::BORDER).x;
 }
 
 // Gets the height of the element, including the client area, padding, borders and scrollbars, but not margins.
 float Element::GetOffsetHeight()
 {
-	UpdateLayout();
 	return GetBox().GetSize(Box::BORDER).y;
 }
 
 // Gets the left scroll offset of the element.
 float Element::GetScrollLeft()
 {
-	UpdateLayout();
 	return scroll_offset.x;
 }
 
@@ -1134,7 +1123,6 @@ void Element::SetScrollLeft(float scroll_left)
 // Gets the top scroll offset of the element.
 float Element::GetScrollTop()
 {
-	UpdateLayout();
 	return scroll_offset.y;
 }
 
@@ -2043,14 +2031,6 @@ void Element::OnChildRemove(Element* child)
 		parent->OnChildRemove(child);
 }
 
-// Update the element's layout if required.
-void Element::UpdateLayout()
-{
-	ElementDocument* document = GetOwnerDocument();
-	if (document != NULL)
-		document->UpdateLayout();
-}
-
 // Forces a re-layout of this element, and any other children required.
 void Element::DirtyLayout()
 {
@@ -2062,7 +2042,7 @@ void Element::DirtyLayout()
 /// Increment/Decrement the layout lock
 void Element::LockLayout(bool lock)
 {
-	Element* document = GetOwnerDocument();
+	ElementDocument* document = GetOwnerDocument();
 	if (document != NULL)
 		document->LockLayout(lock);
 }

Source/Core/ElementDocument.cpp

@@ -300,15 +300,22 @@ void ElementDocument::LoadScript(Stream* ROCKET_UNUSED_PARAMETER(stream), const
 	ROCKET_UNUSED(source_name);
 }
 
-// Updates the layout if necessary.
-void ElementDocument::_UpdateLayout()
+// Updates the document, including its layout
+void ElementDocument::UpdateDocument()
 {
-	const int max_updates = 3;
+	Element::Update();
+	UpdateLayout();
+}
 
-	lock_layout++;
-	if(layout_dirty)//for(int i = 0; layout_dirty && i < max_updates; i++)
+// Updates the layout if necessary.
+void ElementDocument::UpdateLayout()
+{
+	// The lock_layout currently has no effect. Instead, we carefully consider
+	// when we need to call this function.
+	if(layout_dirty)
 	{
 		layout_dirty = false;
+		lock_layout++;
 
 		Vector2f containing_block(0, 0);
 		if (GetParentNode() != NULL)
@@ -316,8 +323,8 @@ void ElementDocument::_UpdateLayout()
 
 		LayoutEngine layout_engine;
 		layout_engine.FormatElement(this, containing_block);
+		lock_layout--;
 	}
-	lock_layout--;
 }
 
 // Updates the position of the document based on the style properties.
@@ -377,12 +384,6 @@ void ElementDocument::DirtyDpProperties()
 	GetStyle()->DirtyDpProperties();
 }
 
-// Refreshes the document layout if required.
-void ElementDocument::OnUpdate()
-{
-	UpdateLayout();
-}
-
 // Repositions the document if necessary.
 void ElementDocument::OnPropertyChange(const PropertyNameList& changed_properties)
 {

Source/Core/ElementScroll.cpp

@@ -44,11 +44,25 @@ ElementScroll::ElementScroll(Element* _element)
 }
 
 ElementScroll::~ElementScroll()
+{
+	ClearScrollbars();
+}
+
+void ElementScroll::ClearScrollbars()
 {
 	for (int i = 0; i < 2; i++)
 	{
 		if (scrollbars[i].element != NULL)
+		{
 			scrollbars[i].element->RemoveEventListener("scrollchange", this);
+			scrollbars[i] = Scrollbar();
+		}
+	}
+
+	if (corner != NULL)
+	{
+		corner->GetParentNode()->RemoveChild(element);
+		corner = NULL;
 	}
 }
 

Source/Core/ElementStyle.cpp

@@ -873,7 +873,6 @@ void ElementStyle::DirtyProperties(const PropertyNameList& properties, bool clea
 
 	// And send the event.
 	element->DirtyProperties(properties);
-	//element->OnPropertyChange(properties);
 }
 
 // Sets a list of our potentially inherited properties as dirtied by an ancestor.

Source/Core/GeometryDatabase.cpp

@@ -32,26 +32,27 @@
 namespace Rocket {
 namespace Core {
 
-typedef std::set< Geometry* > GeometrySet;
-GeometrySet geometries;
+typedef std::unordered_set< Geometry* > GeometrySet;
+//static GeometrySet geometries;
 
 // Adds a geometry to the database.
 void GeometryDatabase::AddGeometry(Geometry* geometry)
 {
-	geometries.insert(geometry);
+	//geometries.insert(geometry);
 }
 
 // Removes a geometry from the database.
 void GeometryDatabase::RemoveGeometry(Geometry* geometry)
 {
-	geometries.erase(geometry);
+	//geometries.erase(geometry);
 }
 
 // Releases all compiled geometries.
 void GeometryDatabase::ReleaseGeometries()
 {
-	for (GeometrySet::iterator i = geometries.begin(); i != geometries.end(); ++i)
-		(*i)->Release();
+	Log::Message(Log::LT_WARNING, "ReleaseGeometry not currently implemented for performance reasons.");
+	//for (GeometrySet::iterator i = geometries.begin(); i != geometries.end(); ++i)
+	//	(*i)->Release();
 }
 
 }

Source/Core/LayoutInlineBoxText.cpp

@@ -153,9 +153,7 @@ void LayoutInlineBoxText::BuildWordBox()
 	{
 		height = 0;
 		baseline = 0;
-
-		// For unknown reasons, this gets triggered during document load. Seems to cause no trouble, remove warning.
-		Log::Message(Log::LT_WARNING, "No font face defined on element %s. Please specify a font-family in your RCSS.", text_element->GetAddress().c_str());
+		Log::Message(Log::LT_WARNING, "No font face defined on element %s. Please specify a font-family in your RCSS, otherwise make sure Context::Update is run after new elements are constructed, before Context::Render.", text_element->GetAddress().c_str());
 		return;
 	}