event simplifier

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Json serializer control character escape fixed. Some unicode characters
   were not escaped properly. 
 - Fix dimension label transition on axis and legend.
+- Through event handler call, when a new event handler is registered, undefined behaviour happened.
 
 ### Added
 
@@ -15,6 +16,7 @@
 - Remove cursor modification over logo.
 - Make `channel.step` option to work on dimensions. 
 - When X axis dimension labels are close to each other, they are rotated to avoid overlapping. 
+- The event handler registration order changed. Now the handlers are called in the opposite order of the registration.
 
 ## [0.9.3] - 2023-12-20
 

src/base/util/eventdispatcher.cpp

@@ -5,13 +5,15 @@
 namespace Util
 {
 
-EventDispatcher::Params::Params(const EventTarget *s) : target(s) {}
+EventDispatcher::Params::Params(const EventTarget *target) :
+    target(target)
+{}
 
 std::string EventDispatcher::Params::toJSON() const
 {
 	std::string res;
 	appendToJSON(
-	    Conv::JSONObj{res}("type", event->name())("target", target)
+	    Conv::JSONObj{res}("type", eventName)("target", target)
 	        .key("detail"));
 	return res;
 }
@@ -23,100 +25,77 @@ void EventDispatcher::Params::appendToJSON(Conv::JSON &obj) const
 
 EventDispatcher::Params::~Params() = default;
 
-EventDispatcher::Event::Event(EventDispatcher &owner,
-    const char *name) :
-    uniqueName(name),
-    owner(owner)
+EventDispatcher::Event::Event(std::string_view name) :
+    uniqueName(name)
 {}
 
 EventDispatcher::Event::~Event() = default;
 
-const std::string &EventDispatcher::Event::name() const
+const std::string_view &EventDispatcher::Event::name() const noexcept
 {
 	return uniqueName;
 }
 
-void EventDispatcher::Event::deactivate() { active = false; }
-
 bool EventDispatcher::Event::invoke(Params &&params)
 {
-	params.event = shared_from_this();
-	for (auto &handler : handlers) {
-		params.handler = static_cast<int>(handler.first);
-		currentlyInvoked = params.handler;
-		handler.second(params);
-		currentlyInvoked = 0;
-	}
-	for (auto &item : handlersToRemove) detach(item.first);
-	handlersToRemove.clear();
+	params.eventName = name();
+
+	for (auto iter = handlers.begin(); iter != handlers.end();)
+		iter++->second(params);
+
 	return !params.preventDefault;
 }
 
 void EventDispatcher::Event::attach(std::uint64_t id,
     handler_fn handler)
 {
-	handlers.emplace_back(id, std::move(handler));
+	handlers.emplace_front(id, std::move(handler));
 }
 
 void EventDispatcher::Event::detach(std::uint64_t id)
 {
-	if (currentlyInvoked != 0)
-		handlersToRemove.emplace_back(currentlyInvoked, handler_fn{});
-	else {
-		for (auto iter = handlers.begin(); iter != handlers.end();
-		     ++iter) {
-			if (iter->first == id) {
-				handlers.erase(iter);
-				break;
-			}
+	for (auto oit = handlers.before_begin(), it = std::next(oit);
+	     it != handlers.end();
+	     oit = it++)
+		if (it->first == id) {
+			handlers.erase_after(oit);
+			break;
 		}
-	}
-}
-
-EventDispatcher::Event::operator bool() const
-{
-	return active && !handlers.empty();
 }
 
 bool EventDispatcher::Event::operator()(Params &&params)
 {
 	return invoke(std::move(params));
 }
 
-EventDispatcher::~EventDispatcher()
-{
-	for (auto &event : eventRegistry) { event.second->deactivate(); }
-}
+EventDispatcher::~EventDispatcher() = default;
 
-EventDispatcher::event_ptr EventDispatcher::getEvent(const char *name)
+EventDispatcher::event_ptr EventDispatcher::getEvent(
+    std::string_view name) const
 {
-	auto iter = eventRegistry.find(name);
-	if (iter == eventRegistry.end()) return event_ptr{};
-	return iter->second;
+	if (auto iter = eventRegistry.find(name);
+	    iter != eventRegistry.end())
+		return *iter;
+	return {};
 }
 
-EventDispatcher::event_ptr EventDispatcher::createEvent(
-    const char *name)
+const EventDispatcher::event_ptr &EventDispatcher::createEvent(
+    std::string_view name)
 {
-	auto iter = eventRegistry.find(name);
-	if (iter != eventRegistry.end()) return iter->second;
-	event_ptr event;
-	event = std::make_shared<Event>(*this, name);
-	eventRegistry.insert(std::make_pair(name, event));
-	return event;
+	auto iter_place = eventRegistry.lower_bound(name);
+	if (iter_place == eventRegistry.end()
+	    || (*iter_place)->name() != name)
+		iter_place = eventRegistry.insert(iter_place,
+		    std::make_shared<Event>(name));
+
+	return *iter_place;
 }
 
-bool EventDispatcher::destroyEvent(const char *name)
+bool EventDispatcher::destroyEvent(const event_ptr &event)
 {
-	auto iter = eventRegistry.find(name);
+	auto iter = eventRegistry.find(event->name());
 	if (iter == eventRegistry.end()) return false;
-	iter->second->deactivate();
 	eventRegistry.erase(iter);
 	return true;
 }
-
-bool EventDispatcher::destroyEvent(const event_ptr &event)
-{
-	return destroyEvent(event->name().c_str());
-}
 }

src/base/util/eventdispatcher.h

@@ -1,10 +1,10 @@
 #ifndef UTIL_EVENTDISPATCHER
 #define UTIL_EVENTDISPATCHER
 
+#include <forward_list>
 #include <functional>
-#include <list>
-#include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 #include "base/conv/auto_json.h"
@@ -24,22 +24,18 @@ class EventDispatcher
 public:
 	class Event;
 	class Params;
-	friend class Event;
-	using handler_id = int;
 	using event_ptr = std::shared_ptr<Event>;
 	using handler_fn = std::function<void(Params &)>;
-	using event_map = std::map<std::string, event_ptr>;
 	using handler_list =
-	    std::list<std::pair<std::uint64_t, handler_fn>>;
+	    std::forward_list<std::pair<std::uint64_t, handler_fn>>;
 
 	class Params
 	{
 	public:
-		explicit Params(const EventTarget *sptr = nullptr);
+		explicit Params(const EventTarget *target = nullptr);
 		virtual ~Params();
-		event_ptr event;
+		std::string_view eventName;
 		const EventTarget *target;
-		handler_id handler{0};
 		bool preventDefault{false};
 
 		[[nodiscard]] std::string toJSON() const;
@@ -51,49 +47,52 @@ class EventDispatcher
 		friend class EventDispatcher;
 
 	public:
-		Event(EventDispatcher &owner, const char *name);
+		explicit Event(std::string_view name);
 		virtual ~Event();
 
-		[[nodiscard]] const std::string &name() const;
+		[[nodiscard]] const std::string_view &name() const noexcept;
 		bool invoke(Params &&params = Params{});
 		void attach(std::uint64_t id, handler_fn handler);
 		void detach(std::uint64_t id);
-		explicit operator bool() const;
-		bool operator()(Params &&params);
-
-		template <typename T> void attach(T &handlerOwner)
-		{
-			static_assert(!std::is_const_v<T>);
-			attach(std::hash<T *>{}(std::addressof(handlerOwner)),
-			    std::ref(handlerOwner));
-		}
-
-		template <typename T> void detach(T &handlerOwner)
-		{
-			static_assert(!std::is_const_v<T>);
-			detach(std::hash<T *>{}(std::addressof(handlerOwner)));
-		}
+		[[nodiscard]] bool operator()(Params &&params);
 
 	protected:
-		bool active{true};
-		std::string uniqueName;
+		std::string_view uniqueName;
 		handler_list handlers;
-		EventDispatcher &owner;
-		handler_id currentlyInvoked{0};
-		handler_list handlersToRemove;
-
-		void deactivate();
 	};
 
 	virtual ~EventDispatcher();
 
-	event_ptr getEvent(const char *name);
-	event_ptr createEvent(const char *name);
-	bool destroyEvent(const char *name);
+	[[nodiscard]] event_ptr getEvent(std::string_view name) const;
+
+	[[nodiscard]] const event_ptr &createEvent(std::string_view name);
+
 	bool destroyEvent(const event_ptr &event);
 
 protected:
-	event_map eventRegistry;
+	struct EventPtrComp
+	{
+		using is_transparent = std::true_type;
+		[[nodiscard]] bool operator()(const event_ptr &lhs,
+		    const std::string_view &rhs) const noexcept
+		{
+			return lhs->name() < rhs;
+		}
+
+		[[nodiscard]] bool operator()(const std::string_view &lhs,
+		    const event_ptr &rhs) const noexcept
+		{
+			return lhs < rhs->name();
+		}
+
+		[[nodiscard]] bool operator()(const event_ptr &lhs,
+		    const event_ptr &rhs) const noexcept
+		{
+			return lhs->name() < rhs->name();
+		}
+	};
+
+	std::set<event_ptr, EventPtrComp> eventRegistry;
 };
 
 }

src/chart/main/events.h

@@ -41,19 +41,14 @@ class Events
 		}
 	};
 
-	struct OnDrawEvent : public Util::EventDispatcher::Params
-	{
-		explicit OnDrawEvent(const Util::EventTarget &target) :
-		    Util::EventDispatcher::Params(&target)
-		{}
-	};
+	using OnDrawEvent = Util::EventDispatcher::Params;
 
 	struct OnRectDrawEvent : public OnDrawEvent
 	{
 		Draw::Rect rect;
 		OnRectDrawEvent(const Util::EventTarget &target,
 		    const Draw::Rect &rect) :
-		    OnDrawEvent(target),
+		    OnDrawEvent(&target),
 		    rect(rect)
 		{}
 
@@ -68,7 +63,7 @@ class Events
 		Draw::Line line;
 		OnLineDrawEvent(const Util::EventTarget &target,
 		    const Draw::Line &line) :
-		    OnDrawEvent(target),
+		    OnDrawEvent(&target),
 		    line(line)
 		{}
 
@@ -91,7 +86,7 @@ class Events
 		OnTextDrawEvent(const Util::EventTarget &target,
 		    const Geom::TransformedRect &rect,
 		    const std::string_view &text) :
-		    OnDrawEvent(target),
+		    OnDrawEvent(&target),
 		    OnTextDrawDetail{rect, text}
 		{}
 

src/chart/rendering/drawbackground.cpp

@@ -11,8 +11,8 @@ void DrawBackground::draw(Gfx::ICanvas &canvas,
     Util::EventDispatcher::Event &onDraw,
     std::unique_ptr<Util::EventTarget> &&eventTarget) const
 {
-	Events::OnRectDrawEvent eventObj(*eventTarget, {rect, false});
-	if (!style.borderColor->isTransparent()
+	if (Events::OnRectDrawEvent eventObj(*eventTarget, {rect, false});
+	    !style.borderColor->isTransparent()
 	    || !style.backgroundColor->isTransparent()) {
 		canvas.setBrushColor(*style.backgroundColor);
 		canvas.setLineColor(*style.borderColor);

src/chart/rendering/drawlabel.cpp

@@ -50,7 +50,7 @@ void DrawLabel::draw(Gfx::ICanvas &canvas,
 	trRect.size = textRect.size;
 
 	if (onDraw.invoke(
-	        Events::OnTextDrawEvent(*eventTarget, trRect, text))) {
+	        Events::OnTextDrawEvent{*eventTarget, trRect, text})) {
 		canvas.transform(transform);
 
 		canvas.text(Geom::Rect(Geom::Point(), textRect.size), text);

src/chart/rendering/orientedlabel.cpp

@@ -91,9 +91,8 @@ void OrientedLabel::draw(Gfx::ICanvas &canvas,
 		canvas.save();
 		canvas.setTextColor(textColor);
 
-		Events::OnTextDrawEvent eventObj(*eventTarget, rect, text);
-
-		if (event.invoke(std::move(eventObj))) {
+		if (event.invoke(
+		        Events::OnTextDrawEvent{*eventTarget, rect, text})) {
 			canvas.transform(rect.transform);
 			canvas.text(contentRect, text);
 			renderedChart.emplace(rect, std::move(eventTarget));