Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API::emit_event behavior is different from a "real" event #631

Open
DinkydauSet opened this issue Aug 17, 2021 · 3 comments
Open

API::emit_event behavior is different from a "real" event #631

DinkydauSet opened this issue Aug 17, 2021 · 3 comments

Comments

@DinkydauSet
Copy link

DinkydauSet commented Aug 17, 2021

The issue / question

I found this problem because my program was too slow. I use a class called subclass found at: http://nanapro.sourceforge.net/faq.htm It allows to redefine the window procedure for a nana window, to intercept windows api messages before nana's window procedure is called.

For some reason, event handlers called by nana in response to real mouse clicks and such are much faster than actions that are initiated in another way. Here's an example (complete compilable code follows):

class test_form : public form
{
public:
	button slowActionButton{ *this, "click here" };

	void slowAction() {
		
		// This doesn't help:
		//auto& brock = detail::bedrock::instance();
		//detail::bedrock::root_guard(brock, *this);
		//internal_scope_guard isg;

		for (int i=0; i<10000; i++) {
			bgcolor(color(i % 255, i % 255, i % 255));
		}
	}

	test_form() {
		slowActionButton.events().click([this](const arg_click& arg)
		{
			// Calling the function from here is fast. (in an event). I see the color change once.
			slowAction();
		});

		div("<x><y>");
		get_place()["x"] << slowActionButton;
		get_place().collocate();
	}
};


int main()
{
	test_form form;
	subclass sub(form);

	//action before WM_KEYDOWN is handled by nana's window procedure
	sub.make_before(WM_KEYDOWN, [&form](UINT, WPARAM wParam, LPARAM, LRESULT*)
	{
		if (wParam == 'A') { //A was pressed
			// Calling the function from here is slow. I actually see all the intermediate colors.
			form.slowAction();
		}
		return true;
	});

	form.show();
	exec();
	
	return 0;
}

I tried calling the event handler by using the API:

	sub.make_before(WM_KEYDOWN, [&form](UINT, WPARAM wParam, LPARAM, LRESULT*)
	{
		if (wParam == 'A') {
			API::emit_event<arg_click>(event_code::click, form.slowActionButton, arg_click());
		}
		return true;
	});

but that's also slow. I can see the background color cycle through all the colors. Why is the behavior different when the event is emitted by a real mouse click? Is this a problem with nana?

additional information

I measured the time. Here's a screenshot. First I clicked the button. Then I pressed A:

image

I looked at the source code of nana for a hint why this could be. Why does a real mouse click result in much better speed? How does it know what is and what is not a real click? I see nana's window procedure uses the class root_guard and a comment mentions "lazy update" ( line 46). I thought that had to be it. I tried adding this:

auto& brock = detail::bedrock::instance();
detail::bedrock::root_guard(brock, *this);

It doesn't help at all. In fact it makes the event slow as well.

I can't figure it out. Anyway, here's a full program with the subclass code that exhibits the behavior:

slow_event.cpp
#include <map>
#include <chrono>
#include <functional>
#include <mutex>
#include <iostream>

#include <nana/gui.hpp>
#include <nana/gui/widgets/button.hpp>
#include <nana/gui/detail/bedrock.hpp>

#include <windows.h>

using namespace std;
using namespace nana;

// Based on code found at: http://nanapro.sourceforge.net/faq.htm
/*
 *	A helper class for subclassing under Windows.
 *	This is a demo for Nana C++ Library.
 *
 *	Nana library does not provide functions to access the Windows Messages. But
 *	the demo is intend to define a helper to access the Window Messages by subclassing
 *	it.
 *
 *	A C++11 compiler is required for the demo.
 */
class subclass
{
	struct msg_pro
	{
		std::function<bool(UINT, WPARAM, LPARAM, LRESULT*)> before;
		std::function<bool(UINT, WPARAM, LPARAM, LRESULT*)> after;
	};

	typedef std::lock_guard<std::recursive_mutex> lock_guard;
public:
	subclass(nana::window wd)
		:	native_(reinterpret_cast<HWND>(nana::API::root(wd))),
			old_proc_(nullptr)
	{
	}

	~subclass()
	{
		clear();
	}

	void make_before(UINT msg, std::function<bool(UINT, WPARAM, LPARAM, LRESULT*)> fn)
	{
		lock_guard lock(mutex_);
		msg_table_[msg].before = std::move(fn);
		_m_subclass(true);
	}

	void make_after(UINT msg, std::function<bool(UINT, WPARAM, LPARAM, LRESULT*)> fn)
	{
		lock_guard lock(mutex_);
		msg_table_[msg].after = std::move(fn);
		_m_subclass(true);
	}

	void umake_before(UINT msg)
	{
		lock_guard lock(mutex_);
		auto i = msg_table_.find(msg);
		if(msg_table_.end() != i)
		{
			i->second.before = nullptr;
			if(nullptr == i->second.after)
			{
				msg_table_.erase(msg);
				if(msg_table_.empty())
					_m_subclass(false);
			}
		}
	}

	void umake_after(UINT msg)
	{
		lock_guard lock(mutex_);
		auto i = msg_table_.find(msg);
		if(msg_table_.end() != i)
		{
			i->second.after = nullptr;
			if(nullptr == i->second.before)
			{
				msg_table_.erase(msg);
				if(msg_table_.empty())
					_m_subclass(false);
			}
		}
	}

	void umake(UINT msg)
	{
		lock_guard lock(mutex_);
		msg_table_.erase(msg);

		if(msg_table_.empty())
			_m_subclass(false);
	}

	void clear()
	{
		lock_guard lock(mutex_);
		msg_table_.clear();
		_m_subclass(false);
	}
private:
	void _m_subclass(bool enable)
	{
		lock_guard lock(mutex_);

		if(enable)
		{
			if(native_ && (nullptr == old_proc_))
			{
				old_proc_ = (WNDPROC)::SetWindowLongPtr(native_, GWLP_WNDPROC, (LONG_PTR)_m_subclass_proc);
				if(old_proc_)
					table_[native_] = this;
			}
		}
		else
		{
			if(old_proc_)
			{
				table_.erase(native_);
				::SetWindowLongPtr(native_, GWLP_WNDPROC, (LONG_PTR)old_proc_);
				old_proc_ = nullptr;

			}
		}
	}

	static bool _m_call_before(msg_pro& pro, UINT msg, WPARAM wp, LPARAM lp, LRESULT* res)
	{
		return (pro.before ? pro.before(msg, wp, lp, res) : true);
	}

	static bool _m_call_after(msg_pro& pro, UINT msg, WPARAM wp, LPARAM lp, LRESULT* res)
	{
		return (pro.after ? pro.after(msg, wp, lp, res) : true);
	}
private:
	static LRESULT CALLBACK _m_subclass_proc(HWND wd, UINT msg, WPARAM wp, LPARAM lp)
	{
		lock_guard lock(mutex_);

		subclass * self = _m_find(wd);
		if(nullptr == self || nullptr == self->old_proc_)
			return 0;

		auto i = self->msg_table_.find(msg);
		if(self->msg_table_.end() == i)
			return ::CallWindowProc(self->old_proc_, wd, msg, wp, lp);

		LRESULT res = 0;
		bool continue_ = self->_m_call_before(i->second, msg, wp, lp, &res);
		if(continue_)
		{
			res = ::CallWindowProc(self->old_proc_, wd, msg, wp, lp);
			self->_m_call_after(i->second, msg, wp, lp, &res);
			return res;
		}

		if(WM_DESTROY == msg)
			self->clear();

		return res;
	}

	static subclass * _m_find(HWND wd)
	{
		lock_guard lock(mutex_);
		std::map<HWND, subclass*>::iterator i = table_.find(wd);
		if(i != table_.end())
			return i->second;

		return 0;
	}
private:
	HWND native_;
	WNDPROC old_proc_;
	std::map<UINT, msg_pro> msg_table_;

	static std::recursive_mutex mutex_;
	static std::map<HWND, subclass*> table_;
};

std::recursive_mutex subclass::mutex_;
std::map<HWND, subclass*> subclass::table_;



//macro to easily time pieces of code
#define timerstart { auto start = chrono::high_resolution_clock::now();
#define timerend(name) \
	auto end = chrono::high_resolution_clock::now(); \
	chrono::duration<double> elapsed = end - start; \
	cout << #name << " took " << elapsed.count() << " seconds" << endl; }



//The example:

class test_form : public form
{
public:
	button slowActionButton{ *this, "click here" };

	void slowAction() {
		
		// This doesn't help:
		//auto& brock = detail::bedrock::instance();
		//detail::bedrock::root_guard(brock, *this);
		//internal_scope_guard isg;

		timerstart //THIS IS THE SLOW PART:
		for (int i=0; i<10000; i++) {
			bgcolor(color(i % 255, i % 255, i % 255));
		}
		timerend("slow action")
	}

	test_form() {
		slowActionButton.events().click([this](const arg_click& arg)
		{
			slowAction(); // Calling the function from here is fast. (in an event)
		});

		div("<x><y>");
		get_place()["x"] << slowActionButton;
		get_place().collocate();
	}
};


int main()
{
	test_form form;
	subclass sub(form);

	sub.make_before(WM_KEYDOWN, [&form](UINT, WPARAM wParam, LPARAM, LRESULT*)
	{
		if (wParam == 'A') {
			form.slowAction(); // Calling the function from here is slow.
			
			//Even emitting the event from here is slow:
			//API::emit_event<arg_click>(event_code::click, form.slowActionButton, arg_click());
		}
		return true;
	});

	form.show();
	exec();
	
	return 0;
}

I compiled it with this command:

g++ slow_event.cpp -Llibrary -lnana -Iinclude -std=c++17 -s -m64 -lgdi32 -lcomdlg32 -o slow_event.exe
@DinkydauSet
Copy link
Author

DinkydauSet commented Aug 18, 2021

I have found that there are more ways to get the slow behavior, even with "real" events:

using a timer event:

timer t(chrono::milliseconds(5000));
t.elapse([&form]()
{
	form.slowAction(); //always slow
});

using a programatically triggered slider value_changed event:

// s is a slider
s.events().value_changed([this](const arg_slider& arg)
{
    slowAction(); //slow if triggered programatically; fast if triggered manually
});
s.value(10); //slow

Triggering it manually makes the action fast. This also happens with a scroll.

@ashbob999
Copy link

The way you are using root_guard is incorrect, it is a class so it must be tied to a variable.
e.g. detail::bedrock::root_guard rg{brock, *this};.

Also I think the root_guard should be just before the call to slowAction, where you want the gui updates to be lazy, otherwise since it sets lazy_update to true then false any other events would then not be lazy.
Also you would then have to manually refresh the window after root_guard has been destroyed.
So in the sub.make_before function you should have:

sub.make_before(WM_KEYDOWN, [&form](UINT, WPARAM wParam, LPARAM, LRESULT*)
	{
		if (wParam == 'A') {
                        {
                                // for lazy updates
                                auto& brock = detail::bedrock::instance();
                                detail::bedrock::root_guard rg{brock, form};

		        	form.slowAction(); // Calling the function from here is slow.
			
		        	//Even emitting the event from here is slow:
		        	//API::emit_event<arg_click>(event_code::click, form.slowActionButton, arg_click());
		        } // root_guard destroyed
                        api::refresh_window(form); // refresh the form
                }
		return true;
	});

Results using windows 10 64-bit msvc debug mode.

Real - 0.258553 seconds
Fake - 0.2592434 seconds

api::refresh_window is very slightly faster than api::update_window but I'm not sure of the difference in this scenario.

@DinkydauSet
Copy link
Author

Thank you for finding that mistake! I tried it and the speed problem in my program is now gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants