Auburn
commented
1 year ago I added the mouse pos update into Context::updateApplicationCursor because it was convenient and had access to the application. But it could go in it's own function.
Open Auburn opened 1 year ago
Auburn
commented
1 year ago I added the mouse pos update into Context::updateApplicationCursor because it was convenient and had access to the application. But it could go in it's own function.
Auburn
commented
1 year ago Looks like there is no way to warpCursor outside of SDL/GLFW applications, what is the best way to detect these are active?
mosra
commented
1 year ago There's a test that mocks up the Application class in order to be able to verify the behavior in an automated headless setting, so it'd be about adding a warpCursor() implementation around here:
and then ideally also adding a new test case verifying that this API gets correctly called when it should, somewhere near to the other mouse-related tests. Hopefully you can figure that out from the existing test code, feel free to ask if not.
The nastier problem is the fallback for apps without warpCursor() as you mentioned. I think it could be implemented via some SFINAE trick, unfortunately there isn't anything remotely similar to get an inspiration from, only the cursor-related stuff. But I think something like this could work:
namespace Implementation {
template<class Application> void callWarpCursor(Application&, ...) {}
template<class Application, class = decltype(&Application::warpCursor)> void callWarpCursor(Application& application, const Vector2i& position) {
application.warpCursor(position);
}
}and then doing Implementation::callWarpCursor(application, ...) instead of application.warpCursor(). You'll also need some similar trick for setting the ImGuiBackendFlags_HasSetMousePos flag.
No guarantee if this works on all compilers tho :D
because it was convenient
Fine with me :)
io.WantSetMousePoswill never be set unless requested by the user
Can you add this into the docs somewhere? Not sure what's the best place tho, maybe somewhere near the event handling stuff in Context.h.
Auburn
commented
1 year ago You'll also need some similar trick for setting the
ImGuiBackendFlags_HasSetMousePosflag.
The problem with this is there is no reference to the Application class in that function
codecov[bot]
commented
1 year ago Patch coverage: 100.00% and project coverage change: +0.09 :tada:
Comparison is base (
1a66b05) 76.74% compared to head (36f0845) 76.83%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
mosra
commented
1 year ago there is no reference to the Application class in that function
I think this could make it work -- add a new templated Context constructor overloads defined in Context.hpp that take a reference to the Application, delegate to the non-templated ones, and then set up additional flags based on what the application type supports.
// Context.h
template<class Application> explicit Context(const Vector2& size, Application& application);
template<class Application> explicit Context(ImGuiContext& context, const Vector2& size, Application& application);// Context.hpp
// implicitly fetches framebuffer and window size from the application instance
template<class Application> Context::Context(const Vector2& size, Application& application): Context{size, application.windowSize(), application.framebufferSIze()} {
if(application has warp cursor)
ImGui::GetIO().BackendFlags |= ImGuiBackendFlags_HasSetMousePos;
}
template<class Application> Context::Context(ImGuiContext& context, const Vector2& size, Application& application): Context{context size, application.windowSize(), application.framebufferSIze()} {
// same here, or maybe have some shared private helper for this instead?
}The docs should then say something like "using this template constructor is preferrable as it enables additional features based on given Application capabilities". This way it also doesn't introduce any breakage or behavior change for existing users which is nice -- when using the original constructor, cursor warp simply won't be supported, and to make use of it, the users have to adapt their code.
Not sure yet if the original constructor should be deprecated, probably not -- as I said before, I want to eventually split this code into an event handling and rendering part so they can be better mixed with 3rd party application toolkits or 3rd party renderers, so I'll do the deprecation as part of that instead.
io.WantSetMousePoswill never be set unless requested by the user in the config like so:Minor tidy in DrawFrame(), the values in the ImDrawData are set based on the ImGui::GetIO() values so they will be the same