Nabla
Nabla copied to clipboard
Example event callbacks need a way to access example members
Description
With our current event handling design we need an instance of the event handler in order to get the events, the default one worked well until we needed to handle some events, like we have with the resize event on http://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/8
Now the problem statement is "How can the event handler access the members of our example class" i.e the relation between the two. This might get used for other event handlers in examples in the future.
Description of the related problem
http://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/8
Solution proposal
Solution 1 - examples inherit CommonAPIEventHandler
Let the examples that use window inherit from CommonAPIEventHandler and then user passes the event handler itself in InitParams instead of letting CommonAPI construct it for them.
this way user can override the callbacks it needs.
I'm not so sure about this one because it has a dependency on logger and input system, which common API provides for us, but we can simply have setter functins for them.
If we make it work, this solution would be convenient and better than the currentnt one and we wouldn't even need to have ~~Resize~~EventHandler
Solution 2 - register std::function
Our examples don't inherit from anything but ~~Resize~~EventHandler contains a bunch of std::functions with correct signatures like std::function<void(uint32_t,uint32_t)>
unless there is some weird reason not to use a std::function this is a pretty good choice.
Solution 3 - the event functions in ApplicationFramework
This is the currently implemented mitigation for resize event callbacks on the afforementioned PR.
This solution has the GraphicalApplication (or maybe IGraphicalApplicationFramework) define virtual functions like onResize, and the CommonAPI::CommonAPIEventCallback stores a IGraphicalApplicationFramework* and has a function like setGraphicalApp and we can so something like:
bool onWindowResized_impl(uint32_t w, uint32_t h) override
{
m_logger.log("Window resized to { %u, %u }", nbl::system::ILogger::ELL_DEBUG, w, h);
if (graphicalApp)
graphicalApp->onResize(w,h);
return true;
}
The issue with this solution is we're re-defining some function like onResize similar to onWindowResized_impl again as opposed to Solution#1 where we straight up inherit from CommonAPI::CommonAPIEventCallback and the example will be its own event handler, if we had more callbacks we'd need to duplicate them in IGraphicalApplicationFramework as well.
The problem with Solution#1 is having to decide at compile time whether our example will need window and events but It's not a big deal.
We can combine this with Solution#1 and Have GraphicalApplication inherit ICommonAPIEventCallback or even one step further and nbl::ui::IGraphicalApplicationFramework inherit nbl::ui::IWindow::IEventCallback -- this has the same downside of Solution#1 where we have to move the callback creation out of CommonAPI::Init and probably put it in InitParams.
Additional context
See new comments on your PR, esp: https://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/8/files#r940222261
Description
With our current event handling design we need an instance of the event handler in order to get the events, the default one worked well until we needed to handle some events, like we have with the resize event on http://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/8
Now the problem statement is "How can the event handler access the members of our example class" i.e the relation between the two. This might get used for other event handlers in examples in the future.
Description of the related problem
http://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/8
Solution proposal
Solution 1 - examples inherit
CommonAPIEventHandlerLet the examples that use window inherit from
CommonAPIEventHandlerand then user passes the event handler itself inInitParamsinstead of letting CommonAPI construct it for them. this way user can override the callbacks it needs.I'm not so sure about this one because it kinda has dependancy on logger and input system which common apu provides for us, but we can simply have
setterfunctins for them.IF we make it work This solution would be convenient and better than the currentnt one and we wouldn't even need to have ~Resize~EventHandler
Solution 2 - register std::function
Our examples don't inherit from anything but ~Resize~EventHandler contains a bunch of
std::functions with correct signatures likestd::function<void(uint32_t,uint32_t)>unless there is some weird reason not to use a std::function this is a pretty good choice.
Solution 3 - the event functions in ApplicationFramework
This is the currently implemented mitigation for resize event callbacks on the afforementioned PR.
This solution has the GraphicalApplication (or maybe IGraphicalApplicationFramework) define virtual functions like
onResize, and theCommonAPI::CommonAPIEventCallbackstores aIGraphicalApplicationFramework*and has a function likesetGraphicalAppand we can so something like:bool onWindowResized_impl(uint32_t w, uint32_t h) override { m_logger.log("Window resized to { %u, %u }", nbl::system::ILogger::ELL_DEBUG, w, h); if (graphicalApp) graphicalApp->onResize(w,h); return true; }The issue with this solution is we're re-defining some function like
onResizesimilar toonWindowResized_implagain as opposed to Solution#1 where we straight up inherit fromCommonAPI::CommonAPIEventCallbackand the example will be it's own event handler, if we had more callbacks we'd need to duplicate them inIGraphicalApplicationFrameworkas well.The problem with Solution#1 is having to decide at compile time whether our example will need window and events but It's not a big deal.
We can combine this with Solution#1 and Have
GraphicalApplication inherit ICommonAPIEventCallbackor even one step further andnbl::ui::IGraphicalApplicationFramework inherit nbl::ui::IWindow::IEventCallback-- this has the same downside of Solution#1 where we have to move the callback creation out ofCommonAPI::Initand probably put it inInitParams.Additional context
Solution 1
Is fair idea, if you need special/overriden beehaviour, then inherit and override.
Solution 2
A plain no, function signatures are ugly and provide no encapsulation
Solution 3
Its a pain to read, I don't think this idea was thought out.
Solution 1 is the only one I'll accept.
@deprilula28 Please put this on your TODO as Solution 3 which was the easiest at the moment was implemented
Solution 1 is the only one I'll accept.
I misread the original description, what I meant is..
If you want the handler to do something else define a custom EventHandler which inherits from CommonAPIEventHandler in the application main.cpp then it can be aware of all the extra stuff and the actual type of the application which itself inherits from IGraphicalApplication
UNDER NO CIRCUMSTANCE have the application inherit from event handler