Fix GLFW_CURSOR_DISABLED being ignored when updating cursor position

[scorpion-26]

The GLFW backend currently ignores that the cursor is disabled (GLFW_CURSOR_DISABLED is set by glfwSetInputMode).
This leads to an invisible cursor, that still highlights new ImGui elements when moving the mouse around, even though the cursor should remain static and only highlight the element it was last over before being disabled. This PR adds an check for the flag and returns out of function before ImGuiIO's position is updated.

The current behavior stems from the fact, that GLFW still provides updated cursor position when GLFW_CURSOR_DISABLED is set. I don't know if this is intended though (The documentation is a bit unclear about that, I think).

[ocornut]

Hello,

Thanks for the PR.

I believe it is debatable as to whether we want the backend to honor this GLFW setting.
User might want to toggle GLFW_CURSOR_DISABLED for the purpose of their underlying application, and still expect Dear ImGui to be usable (with e.g. a software rendered cursor). I agree this is however unlikely but with this we'd be making this impossible. I'm however OK to merge this as I believe it would align with the expectation of a majority, but if someone shows up with that use case I might revert.

Note that you can set io.ConfigFlags |= ImGuiConfigFlags_NoMouse for the same result.

The PR technically is missing a few spots (ImGui_ImplGlfw_CursorEnterCallback should not submit an event, ImGui_ImplGlfw_UpdateMouseData is_app_focused app should not either, and entering GLFW_CURSOR_DISABLED state should submit a mouse disabled event io.AddMousePosEvent(-FLT_MAX, -FLT_MAX) to imgui, probably can be done in ImGui_ImplGlfw_UpdateMouseData(). And then need to verify that support for multi-viewports docking branch will be easy to merge/fix (I can do that).

[scorpion-26]

Hello,
as for your concerns about the first use case you mentioned about:
I think you're missing the fact (please correct me, if I'm wrong though) that a user can just set GLFW_CURSOR_HIDDEN(this would still work just like GLFW_CURSOR_NORMAL from ImGui's side) and then render a custom cursor. The only time, when GLFW_CURSOR_DISABLED should be set and a custom cursor is rendered, if an user would want to have infinite movement. I however see your concern about making certain things impossible.
For your alternative suggestion however, just setting ImGuiConfigFlags_NoMouse is not enough for enabling unlimited movement, which is GLFW_CURSOR_DISABLED's primary use case. A user would have to set both in ImGui for things to behave like I described.

[ocornut]

I think you're missing the fact (please correct me, if I'm wrong though) that a user can just set GLFW_CURSOR_HIDDEN(this would still work just like GLFW_CURSOR_NORMAL from ImGui's side) and then render a custom cursor.

Good point.

For your alternative suggestion however, just setting ImGuiConfigFlags_NoMouse is not enough for enabling unlimited movement, which is GLFW_CURSOR_DISABLED's primary use case. A user would have to set both in ImGui for things to behave like I described.

Indeed. What I was suggesting initially what that setting ImGuiConfigFlags_NoMouse on your side would be equivalent to getting this PR merged.

But based on your first point above I agree we should merge this. Let me know if you want to finish the PR otherwise I can do it.

[scorpion-26]

I can finish the PR. For docking, do I need to open a seperate PR?

[scorpion-26]

Your suggested changes

ImGui_ImplGlfw_CursorEnterCallback should not submit an event, ImGui_ImplGlfw_UpdateMouseData is_app_focused app should not either, and entering GLFW_CURSOR_DISABLED state should submit a mouse disabled event io.AddMousePosEvent(-FLT_MAX, -FLT_MAX) to imgui, probably can be done in ImGui_ImplGlfw_UpdateMouseData()

have now been implemented. Let me know if there is anything missing. If not, I would open a seperate PR with the same changes for the docking branch.

[ocornut]

Do the PR on master and i’ll look if it’s easy to merge on docking i can do that part. Thank you.

[scorpion-26]

Do the PR on master

Sorry if I'm misunderstanding, but the changes are already pushed in this PR

[ocornut]

This is now merged as 5867a43.
I applied small tweaks, fixed clearing mouse data when not focused, and it merged in docking without an issue.

Thank you!

For reference, a super minimal test bed is running:

ImGui::GetIO()->ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard; = true;
static bool DISABLED = false;
ImGui::Checkbox("DISABLED", &DISABLED);
glfwSetInputMode(window, GLFW_CURSOR, DISABLED ? GLFW_CURSOR_DISABLED : GLFW_CURSOR_NORMAL);

[pborsutzki]

The only time, when GLFW_CURSOR_DISABLED should be set and a custom cursor is rendered, if an user would want to have infinite movement.

So how can I now achieve exactly this behavior? I have a viewer application that supports a fullscreen mode. While being windowed, you press e.g. a mouse button to drag the current view on a 3d scene around, which is handled through ImGUIs mouse events. When going to fullscreen, the component that handles these events is stretched over the full screen. For multi-screen systems it is useful when the mouse never leaves the current screen and the cursor still can send events, even when it would hit a screen border. I also don't need any cursor in that mode. Imagine it like a game editor where you can switch to an FPS mode by pressing a button.

But now with this change I cannot receive mouse events any more in fullscreen. To work around this, I'd now have to directly grab the mouse events from GLFW, which breaks the abstraction through ImGUI. So, for me, this fix introduced a new bug.

So why exactly is setting io.ConfigFlags |= ImGuiConfigFlags_NoMouse and GLFW_CURSOR_DISABLED not sufficient for the original request? Why implement the combination of the two directly into the GLFW backend without any choice of opting out?

[ocornut]

So why exactly is setting io.ConfigFlags |= ImGuiConfigFlags_NoMouse and GLFW_CURSOR_DISABLED not sufficient for the original request? Why implement the combination of the two directly into the GLFW backend without any choice of opting out?

Reopening as this seems like a reasonable statement.
I'd appreciate if you two could investigate this further!

[scorpion-26]

But now with this change I cannot receive mouse events any more in fullscreen. To work around this, I'd now have to directly grab the mouse events from GLFW, which breaks the abstraction through ImGUI. So, for me, this fix introduced a new bug.

So why exactly is setting io.ConfigFlags |= ImGuiConfigFlags_NoMouse and GLFW_CURSOR_DISABLED not sufficient for the original request? Why implement the combination of the two directly into the GLFW backend without any choice of opting out?

You're right, that's something I didn't keep in mind when writing the patch. Since GLFW_CURSOR_DISABLED is intended to fixate the real cursor in place while still providing proper delta movement with a virtual cursor, I think it is fair to assume that ImGui won't respect the cursor in its rendering/item context (so ImGui doesn't highlight things it shouldn't due to the virtual cursor) without the user needing to specify another flag (io.ConfigFlags |= ImGuiConfigFlags_NoMouse), but still send callbacks to apps interested in the virtual cursor coordinates for delta movement.

I'm thinking of (somehow) intercepting the ImGui's consumption of the coordinates instead of the emission by the backend, so cursor callbacks could still be called if needed.

@ocornut Feel free to revert this commit (I'm unfortunately currently unable to) until I submit a rewrite of this PR, because breaking functionality is probably more serious than a mostly visual issue.

[scorpion-26]

@pborsutzki I just opened another PR, which should fix your issue. Can you confirm that this is the case?

[ocornut]

Hello @scorpion-26,

Thanks for your PR.
I am not sure I agree with either solution you posted in #6609

• for one: backend definitively shouldn't manipulate ConfigFlags
• for the other: it would work but seems unnecessary.

As the user/app is in control of manipulating GLFW cursor state, it could freely set/clear ImGuiConfigFlags_NoMouse itself?
Therefore based on this logic we may only want to revert the initial commit.

This strategy would always work.

[ocornut]

I have pushed the revert only (e5977f0) presently I don't see the need for anything else. Let me know @scorpion-26 if I am wrong?

I was swayed/convinced in #5625 (comment) but I think the reasoning was wrong since it mandates a backend-specific solution that a third variant exists in GLFW. AFAIK setting ImGuiConfigFlags_NoMouse in your app can always be the solution.