Clean up the event dispatcher

Include/RmlUi/Core/ObserverPtr.h

@@ -157,7 +157,7 @@ protected:
 
 private:
 
-	// The observer ptr block is assumed to be uniquely owned by us, copy operations must never be implemented.
+	// The observer ptr block is assumed to be uniquely owned by us, copy operations should not be implemented.
 	EnableObserverPtr<T>(const EnableObserverPtr<T>&) = delete;
 	EnableObserverPtr<T>& operator=(const EnableObserverPtr<T>&) = delete;
 

Source/Core/Element.cpp

@@ -1208,21 +1208,21 @@ void Element::RemoveEventListener(EventId id, EventListener* listener, bool in_c
 bool Element::DispatchEvent(const String& type, const Dictionary& parameters)
 {
 	const EventSpecification& specification = EventSpecificationInterface::GetOrInsert(type);
-	return event_dispatcher->TrueDispatchEvent(this, specification.id, type, parameters, specification.interruptible, specification.bubbles, specification.default_action_phase);
+	return EventDispatcher::DispatchEvent(this, specification.id, type, parameters, specification.interruptible, specification.bubbles, specification.default_action_phase);
 }
 
 // Dispatches the specified event
 bool Element::DispatchEvent(const String& type, const Dictionary& parameters, bool interruptible, bool bubbles)
 {
 	const EventSpecification& specification = EventSpecificationInterface::GetOrInsert(type);
-	return event_dispatcher->TrueDispatchEvent(this, specification.id, type, parameters, interruptible, bubbles, specification.default_action_phase);
+	return EventDispatcher::DispatchEvent(this, specification.id, type, parameters, interruptible, bubbles, specification.default_action_phase);
 }
 
 // Dispatches the specified event
 bool Element::DispatchEvent(EventId id, const Dictionary& parameters)
 {
 	const EventSpecification& specification = EventSpecificationInterface::Get(id);
-	return event_dispatcher->TrueDispatchEvent(this, specification.id, specification.type, parameters, specification.interruptible, specification.bubbles, specification.default_action_phase);
+	return EventDispatcher::DispatchEvent(this, specification.id, specification.type, parameters, specification.interruptible, specification.bubbles, specification.default_action_phase);
 }
 
 // Scrolls the parent element's contents so that this element is visible.

Source/Core/EventDispatcher.cpp

@@ -108,195 +108,66 @@ void EventDispatcher::DetachAllEvents()
 		element->GetChild(i)->GetEventDispatcher()->DetachAllEvents();
 }
 
-bool EventDispatcher::DispatchEvent(Element* target_element, EventId id, const String& type, const Dictionary& parameters, bool interruptible, bool bubbles, DefaultActionPhase default_action_phase)
-{
-	EventPtr event = Factory::InstanceEvent(target_element, id, type, parameters, interruptible);
-	if (!event)
-		return false;
-
-	// Build the element traversal from the tree
-	typedef std::vector<Element*> Elements;
-	Elements elements;
-
-	Element* walk_element = target_element->GetParentNode();
-	while (walk_element) 
-	{
-		elements.push_back(walk_element);
-		walk_element = walk_element->GetParentNode();
-	}
+/*
+	EventListenersToExecute
 
-	event->SetPhase(EventPhase::Capture);
-	// Capture phase - root to target (only triggers event listeners that are registered with capture phase)
-	// Note: We walk elements in REVERSE as they're placed in the list from the elements parent to the root
-	for (int i = (int)elements.size() - 1; i >= 0 && event->IsPropagating(); i--) 
-	{
-		EventDispatcher* dispatcher = elements[i]->GetEventDispatcher();
-		event->SetCurrentElement(elements[i]);
-		dispatcher->TriggerEvents(*event, default_action_phase);
-	}
+	When dispatching an event we collect all possible event listeners and default actions to execute.
+	They are stored in observer pointers, so that we can safely check if they have been destroyed since the previous listener execution.
+*/
 
-	// Target phase - direct at the target
-	if (event->IsPropagating()) 
-	{
-		event->SetPhase(EventPhase::Target);
-		event->SetCurrentElement(target_element);
-		target_element->GetEventDispatcher()->TriggerEvents(*event, default_action_phase);
-	}
+struct EventListenersToExecute {
 
-	// Bubble phase - target to root (normal event bindings)
-	if (bubbles && event->IsPropagating())
+	EventListenersToExecute(Element* _element, EventListener* _listener, int dom_distance_from_target, bool in_capture_phase) : element(_element->GetObserverPtr()), listener(_listener ? _listener->GetObserverPtr() : nullptr)
 	{
-		event->SetPhase(EventPhase::Bubble);
-		for (size_t i = 0; i < elements.size() && event->IsPropagating(); i++) 
+		sort = dom_distance_from_target * (in_capture_phase ? -1 : 1);
+		if (!_listener)
 		{
-			EventDispatcher* dispatcher = elements[i]->GetEventDispatcher();
-			event->SetCurrentElement(elements[i]);
-			dispatcher->TriggerEvents(*event, default_action_phase);
+			// No listener means default action
+			sort += DefaultActionAdd;
 		}
 	}
 
-	bool propagating = event->IsPropagating();
-
-	return propagating;
-}
+	static constexpr int DefaultActionMin = 50'000;
+	static constexpr int DefaultActionAdd = 100'000;
 
-String EventDispatcher::ToString() const
-{
-	String result;
+	// The sort value is determined by the distance of the element to the target element in the DOM.
+	// Capture phase is given negative values. Default actions add the large number DefaultActionAdd so they are sorted last.
+	int sort = 0;
 
-	if (listeners.empty())
-		return result;
+	ObserverPtr<Element> element;
+	ObserverPtr<EventListener> listener;
 
-	auto add_to_result = [&result](EventId id, int count) {
-		const EventSpecification& specification = EventSpecificationInterface::Get(id);
-		result += CreateString(specification.type.size() + 32, "%s (%d), ", specification.type.c_str(), count);
-	};
+	// Default actions are returned by EventPhase::None.
+	EventPhase GetPhase() const { return sort < 0 ? EventPhase::Capture : (sort == 0 ? EventPhase::Target : (sort >= DefaultActionMin ? EventPhase::None : EventPhase::Bubble)); }
 
-	EventId previous_id = listeners[0].id;
-	int count = 0;
-	for (const auto& listener : listeners)
-	{
-		if (listener.id != previous_id)
-		{
-			add_to_result(previous_id, count);
-			previous_id = listener.id;
-			count = 0;
-		}
-		count++;
+	bool operator<(const EventListenersToExecute& other) const {
+		return sort < other.sort;
 	}
+};
 
-	if (count > 0)
-		add_to_result(previous_id, count);
-
-	if (result.size() > 2) 
-		result.resize(result.size() - 2);
-
-	return result;
-}
-
-void EventDispatcher::TriggerEvents(Event& event, DefaultActionPhase default_action_phase)
-{
-	const EventPhase phase = event.GetPhase();
-
-	// Find the range of entries with matching id and phase, given that listeners are sorted by (id,phase).
-	// In the case of target phase we will match any listener phase.
-	Listeners::iterator begin, end;
-	if (phase == EventPhase::Capture)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event.GetId(), nullptr, true), CompareIdPhase());
-	else if (phase == EventPhase::Target)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event.GetId(), nullptr, false), CompareId());
-	else if (phase == EventPhase::Bubble)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event.GetId(), nullptr, false), CompareIdPhase());
-
-	// Copy the range in case the original list of listeners get modified during ProcessEvent.
-	const Listeners listeners_range(begin, end);
-
-	for(const EventListenerEntry& entry : listeners_range)
-	{
-		entry.listener->ProcessEvent(event);
-		
-		if (!event.IsImmediatePropagating())
-			break;
-	}
-
-	const bool do_default_action = ((unsigned int)phase & (unsigned int)default_action_phase);
-
-	// Do the default action unless we have been cancelled.
-	if (do_default_action && event.IsPropagating())
-	{
-		element->ProcessDefaultAction(event);
-	}
-}
-
-
-void EventDispatcher::AddEvents(std::vector<ListenerDesc>& add_listeners, std::vector<ObserverPtr<Element>>& default_action_elements, const EventId event_id, const EventPhase phase, DefaultActionPhase default_action_phase)
-{
-	// Find the range of entries with matching id and phase, given that listeners are sorted by (id,phase).
-	// In the case of target phase we will match any listener phase.
-	Listeners::iterator begin, end;
-	if (phase == EventPhase::Capture)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event_id, nullptr, true), CompareIdPhase());
-	else if (phase == EventPhase::Target)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event_id, nullptr, false), CompareId());
-	else if (phase == EventPhase::Bubble)
-		std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event_id, nullptr, false), CompareIdPhase());
-
-	// Copy the range in case the original list of listeners get modified during ProcessEvent.
-	const Listeners listeners_range(begin, end);
-
-	for (auto it = begin; it != end; ++it)
-	{
-		add_listeners.push_back({ element->GetObserverPtr(), it->listener->GetObserverPtr(), phase });
-	}
-
-	const bool do_default_action = ((unsigned int)phase & (unsigned int)default_action_phase);
-
-	// Do the default action unless we have been cancelled.
-	if (do_default_action)
-	{
-		default_action_elements.push_back(element->GetObserverPtr());
-	}
-}
 
-bool EventDispatcher::TrueDispatchEvent(Element* target_element, EventId id, const String& type, const Dictionary& parameters, bool interruptible, bool bubbles, DefaultActionPhase default_action_phase)
+bool EventDispatcher::DispatchEvent(Element* target_element, EventId id, const String& type, const Dictionary& parameters, bool interruptible, bool bubbles, DefaultActionPhase default_action_phase)
 {
-	std::vector<ListenerDesc> listeners;
-	std::vector<ObserverPtr<Element>> default_action_elements;
-
-
+	std::vector<EventListenersToExecute> listeners;
 
-	// Build the element traversal from the tree
-	typedef std::vector<Element*> Elements;
-	Elements elements;
-
-	Element* walk_element = target_element->GetParentNode();
+	const EventPhase phases_to_execute = EventPhase((int)EventPhase::Capture | (int)EventPhase::Target | (bubbles ? (int)EventPhase::Bubble : 0));
+	
+	// Walk the DOM tree from target to root, collecting all possible listeners and elements with default actions in the process.
+	int dom_distance_from_target = 0;
+	Element* walk_element = target_element;
 	while (walk_element)
 	{
-		elements.push_back(walk_element);
-		walk_element = walk_element->GetParentNode();
-	}
+		EventDispatcher* dispatcher = walk_element->GetEventDispatcher();
+		dispatcher->CollectListeners(dom_distance_from_target, id, phases_to_execute, default_action_phase, listeners);
 
-	// Capture phase - root to target (only triggers event listeners that are registered with capture phase)
-	// Note: We walk elements in REVERSE as they're placed in the list from the elements parent to the root
-	for (int i = (int)elements.size() - 1; i >= 0; i--)
-	{
-		EventDispatcher* dispatcher = elements[i]->GetEventDispatcher();
-		dispatcher->AddEvents(listeners, default_action_elements, id, EventPhase::Capture, default_action_phase);
+		walk_element = walk_element->GetParentNode();
+		dom_distance_from_target += 1;
 	}
 
-	target_element->GetEventDispatcher()->AddEvents(listeners, default_action_elements, id, EventPhase::Target, default_action_phase);
+	// Use stable_sort so that the order of the listeners in a given element is maintained.
+	std::stable_sort(listeners.begin(), listeners.end());
 
-	// Bubble phase - target to root (normal event bindings)
-	if (bubbles)
-	{
-		for (Element* el : elements)
-		{
-			EventDispatcher* dispatcher = el->GetEventDispatcher();
-			dispatcher->AddEvents(listeners, default_action_elements, id, EventPhase::Bubble, default_action_phase);
-		}
-	}
-
-	if (listeners.empty() && default_action_elements.empty())
+	if (listeners.empty())
 		return true;
 
 	// Instance event
@@ -304,52 +175,117 @@ bool EventDispatcher::TrueDispatchEvent(Element* target_element, EventId id, con
 	if (!event)
 		return false;
 
-	Element* previous_element = nullptr;
+	int previous_sort_value = INT_MAX;
+	EventPhase current_phase = EventPhase::None;
 
-	// Process event in each listener
+	// Process the event in each listener.
 	for (auto& listener_desc : listeners)
 	{
-		auto element = listener_desc.element.get();
-		auto listener = listener_desc.listener.get();
+		Element* element = listener_desc.element.get();
+		EventListener* listener = listener_desc.listener.get();
 
-		if (previous_element != element)
+		if (listener_desc.sort != previous_sort_value)
 		{
-			event->SetCurrentElement(element);
+			// New sort values represent a new level in the DOM, thus, set the new element and possibly new phase.
 			if (!event->IsPropagating())
 				break;
-			previous_element = element;
+			event->SetCurrentElement(element);
+			current_phase = listener_desc.GetPhase();
+			event->SetPhase(current_phase);
+			previous_sort_value = listener_desc.sort;
 		}
 
 		if (element && listener)
 		{
-			event->SetPhase(listener_desc.phase);
+			// We have a valid event listener
 			listener->ProcessEvent(*event);
 		}
+		else if (element && current_phase == EventPhase::None)
+		{
+			// EventPhase::None means default actions. We assume default actions only execute in either target or bubble phase.
+			event->SetPhase(element == target_element ? EventPhase::Target : EventPhase::Bubble);
+			element->ProcessDefaultAction(*event);
+		}
 
 		if (!event->IsImmediatePropagating())
 			break;
 	}
 
-	// Process default actions
-	for (auto& element_observer : default_action_elements)
+	bool propagating = event->IsPropagating();
+
+	return propagating;
+}
+
+
+void EventDispatcher::CollectListeners(int dom_distance_from_target, const EventId event_id, const EventPhase event_executes_in_phases, const DefaultActionPhase default_action_phase, std::vector<EventListenersToExecute>& collect_listeners)
+{
+	const bool in_target_phase = (dom_distance_from_target == 0);
+
+	// Find all the entries with a matching id, given that listeners are sorted by id first.
+	Listeners::iterator begin, end;
+	std::tie(begin, end) = std::equal_range(listeners.begin(), listeners.end(), EventListenerEntry(event_id, nullptr, false), CompareId());
+
+	for (auto it = begin; it != end; ++it)
 	{
-		if (!event->IsPropagating())
-			break;
+		if (in_target_phase)
+		{
+			// Listeners always attach to target phase, but make sure event can actually execute in target phase.
+			if ((int)event_executes_in_phases & (int)EventPhase::Target)
+				collect_listeners.emplace_back(element, it->listener, dom_distance_from_target, it->in_capture_phase);
+		}
+		else
+		{
+			// Listeners will either attach to capture or bubble phase, make sure event can execute in the same phase.
+			const EventPhase listener_executes_in_phase = (it->in_capture_phase ? EventPhase::Capture : EventPhase::Bubble);
+			if ((int)event_executes_in_phases & (int)listener_executes_in_phase)
+				collect_listeners.emplace_back(element, it->listener, dom_distance_from_target, it->in_capture_phase);
+		}
+	}
+
+	RMLUI_ASSERTMSG(!((int)default_action_phase & (int)EventPhase::Capture), "We assume here that the default action phases cannot include capture phase.");
+
+	// Add the default action
+	EventPhase phase_for_default_action = (in_target_phase ? EventPhase::Target : EventPhase::Bubble);
+
+	if ((int)phase_for_default_action & (int)default_action_phase)
+		collect_listeners.emplace_back(element, nullptr, dom_distance_from_target, false);
+}
 
-		auto element = element_observer.get();
 
-		if (element)
+String EventDispatcher::ToString() const
+{
+	String result;
+
+	if (listeners.empty())
+		return result;
+
+	auto add_to_result = [&result](EventId id, int count) {
+		const EventSpecification& specification = EventSpecificationInterface::Get(id);
+		result += CreateString(specification.type.size() + 32, "%s (%d), ", specification.type.c_str(), count);
+	};
+
+	EventId previous_id = listeners[0].id;
+	int count = 0;
+	for (const auto& listener : listeners)
+	{
+		if (listener.id != previous_id)
 		{
-			event->SetPhase(element == target_element ? EventPhase::Target : EventPhase::Bubble);
-			event->SetCurrentElement(element);
-			element->ProcessDefaultAction(*event);
+			add_to_result(previous_id, count);
+			previous_id = listener.id;
+			count = 0;
 		}
+		count++;
 	}
 
-	bool propagating = event->IsPropagating();
+	if (count > 0)
+		add_to_result(previous_id, count);
 
-	return propagating;
+	if (result.size() > 2)
+		result.resize(result.size() - 2);
+
+	return result;
 }
 
+
 }
 }

Source/Core/EventDispatcher.h

@@ -37,6 +37,8 @@ namespace Core {
 
 class Element;
 class EventListener;
+struct EventListenersToExecute;
+
 struct EventListenerEntry {
 	EventListenerEntry(EventId id, EventListener* listener, bool in_capture_phase) : id(id), in_capture_phase(in_capture_phase), listener(listener) {}
 	EventId id;
@@ -45,12 +47,6 @@ struct EventListenerEntry {
 };
 
 
-struct ListenerDesc {
-	ObserverPtr<Element> element;
-	ObserverPtr<EventListener> listener;
-	EventPhase phase;
-};
-
 /**
 	The Event Dispatcher manages a list of event listeners and triggers the events via EventHandlers
 	whenever requested.
@@ -83,10 +79,14 @@ public:
 	/// Detaches all events from this dispatcher and all child dispatchers.
 	void DetachAllEvents();
 
-	/// Dispatches the specified event, targeting the element associated with this dispatcher.
-	/// @param[in] name The name of the event
+	/// Dispatches the specified event.
+	/// @param[in] target_element The element to target
+	/// @param[in] id The id of the event
+	/// @param[in] type The type of the event
 	/// @param[in] parameters The event parameters
 	/// @param[in] interruptible Can the event propagation be stopped
+	/// @param[in] bubbles True if the event should execute the bubble phase
+	/// @param[in] default_action_phase The phases to execute default actions in
 	/// @return True if the event was not consumed (ie, was prevented from propagating by an element), false if it was.
 	static bool DispatchEvent(Element* target_element, EventId id, const String& type, const Dictionary& parameters, bool interruptible, bool bubbles, DefaultActionPhase default_action_phase);
 
@@ -94,13 +94,6 @@ public:
 	/// @return Summary of attached listeners.
 	String ToString() const;
 
-
-	static bool TrueDispatchEvent(Element* target_element, EventId id, const String& type, const Dictionary& parameters, bool interruptible, bool bubbles, DefaultActionPhase default_action_phase);
-
-
-	void EventDispatcher::AddEvents(std::vector<ListenerDesc>& add_listeners, std::vector<ObserverPtr<Element>>& default_action_elements, const EventId event_id, const EventPhase phase, DefaultActionPhase default_action_phase);
-
-
 private:
 	Element* element;
 
@@ -109,7 +102,8 @@ private:
 	typedef std::vector< EventListenerEntry > Listeners;
 	Listeners listeners;
 
-	void TriggerEvents(Event& event, DefaultActionPhase default_action_phase);
+	// Collect all the listeners from this dispatcher that are allowed to execute given the input arguments.
+	void CollectListeners(int dom_distance_from_target, EventId event_id, EventPhase phases_to_execute, DefaultActionPhase default_action_phase, std::vector<EventListenersToExecute>& collect_listeners);
 };