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

Updating demo with ImGuiWindowFlags_ResizeFromAnySide (was: Adding mouse cursor support in backends) #1495

Closed
wants to merge 3 commits into from

Conversation

amc522
Copy link
Contributor

@amc522 amc522 commented Dec 11, 2017

Several of the platform examples have been updated with cursor support. Some have not been updated because I do not have the build dependencies or proper computer to test on.

…dded cursor support to several of the platform examples.
@ocornut
Copy link
Owner

ocornut commented Dec 11, 2017

Hello @amc522,
Thanks for the PR.

I think there are a few issues:

A) I haven't run your Win32 use case but having tried this approach myself, I'm pretty sure it is causing conflicts with Windows owns handling of cursors. Try to hover around the OS window borders and you'll notice that the resize cursors get overridden by ours.

We are normally supposed to answer to the WM_SETCURSOR message however that approach has another problem where it only sends the message when moving or clicking, so on the frame where we "land" on an imgui location that changes the cursor it won't be updated until the mouse moves again.
I was looking into the GLFW approach of mixing both and testing the client rectangle for the first part which I think may be the right (but a little ugly) solution. Sorry I should have mentioned I had done a bit of research on this in #822 thread earlier.

If you have a suggestion on how to tackle this another way I'd be interesting.

B) For the 2 missing diagonal cursors see glfw/glfw#427, if you have some time to help the author of GLFW with adding those (depending on what she decided as the discussion was still dangling) I'd gladly appreciated it.

C) The code is too long. How about using ImGuiMouseCursor_Count_ sized arrays instead? Don't worry about it too much, I can refactor, solving the problem in A) is the most important part here.

@amc522
Copy link
Contributor Author

amc522 commented Dec 11, 2017

A) Definitely see the issue and this is a way trickier problem than I thought it would be. I'll play around with some code and see what conclusions (and maybe solutions) I come to.
B) Can look into the glfw library to help out.
C) Really easy to move this into arrays.

@amc522
Copy link
Contributor Author

amc522 commented Dec 12, 2017

Updated imgui_impl_dx11.cpp with some working code that handles WM_SETCURSOR and plays well with ImGui. It's not pretty, but seems to work well to me. Let me know what you think, and if you can find any cases that don't work.

…ch of defines for this value all beginning with HT.
@ocornut ocornut changed the title Updating imgui demo to show off ImGuiWindowFlags_ResizeFromAnySide Adding mouse cursor support in backends / Updating demo with ImGuiWindowFlags_ResizeFromAnySide Dec 20, 2017
@ocornut ocornut added this to the v1.53 milestone Dec 24, 2017
@ocornut
Copy link
Owner

ocornut commented Jan 10, 2018

Hello @amc522, I am putting on hold because I'll be reworking the main bindings structure to support multiple windows, cross-window drag and drop and other advanced features. (See #1542), and some of that is having an effect on mouse cursors. I'll merge isolated bits or integrate equivalent-feature code over the course of the month probably.

ocornut added a commit that referenced this pull request Feb 20, 2018
…mGui::GetMouseCursor() value and WM_SETCURSOR message handling). (#1495)
ocornut added a commit that referenced this pull request Feb 20, 2018
…r() value and WM_SETCURSOR message handling). (#1495)
@ocornut
Copy link
Owner

ocornut commented Feb 20, 2018

@amc522 Same feature was implemented today on the 3 DirectX, 3 GLFW based examples and 2 SDL based examples. I took the logic and code from my other branches (less code, a little more tested and following imgui coding style more closely).
GLFW is still lacking the diagonal resize cursors otherwise it works well.

I am keeping this PR open and renaming it because I would like to add the "ResizeFromAllSide" feature in the demo. However considering going via a route that we have flags to specify what the backend can do, and this feature is automatically enabled if the backend claims it supports mouse cursor.

@ocornut ocornut changed the title Adding mouse cursor support in backends / Updating demo with ImGuiWindowFlags_ResizeFromAnySide Updating demo with ImGuiWindowFlags_ResizeFromAnySide (was: Adding mouse cursor support in backends) Feb 20, 2018
@amc522
Copy link
Contributor Author

amc522 commented Feb 20, 2018

This looks great. Your code definitely looks cleaner and more sane than mine. I think enabling resize on all borders and corners by default if the back end says it supports mouse cursors is a great idea.

One minor note regarding the corner resize grips. Why is there still a special case for the lower right grip to always be visible, even when there are multiple resize grips (imgui.cpp:5515)? I think it would be nicer if either all the grips were always visible, or, if there's more than one grip, not showing any of the grips until they are interacted with. So the code would be:

Grips invisible until interacted with

if (held || hovered)
    resize_grip_col[resize_grip_n] = GetColorU32(held ? ImGuiCol_ResizeGripActive : hovered ? ImGuiCol_ResizeGripHovered : ImGuiCol_ResizeGrip);

Grips always visible

resize_grip_col[resize_grip_n] = GetColorU32(held ? ImGuiCol_ResizeGripActive : hovered ? ImGuiCol_ResizeGripHovered : ImGuiCol_ResizeGrip);

Thoughts?

@ocornut
Copy link
Owner

ocornut commented Feb 20, 2018

I think it would be nicer if either all the grips were always visible, or, if there's more than one grip, not showing any of the grips until they are interacted with. So the code would be:

All grips always visible would be visually too noisy with the look they have.

The right direction seem to be that the grip should be invisible before you hover/active it (which you can currently get by locally setting the ImGuiCol_ResizeGrip color to 0). However users frequently have combination of resizable and auto-resized windows in their app, and it could make a little confusing to remove the visual cue. Perhaps clearing the ResizeGrip color in default theme would be appropriate.

ocornut added a commit that referenced this pull request Mar 20, 2018
…_HasMouseCursors, ImGuiBackendFlags_HasSetMousePos. (#787, #1495, #1202)
@ocornut
Copy link
Owner

ocornut commented Mar 20, 2018

There's no a standard way for back-end to communicate the features they support,
io.BackendFlags |= ImGuiBackendFlags_HasMouseCursors

What it means is that maybe we can retire the ImGuiWindowFlags_ResizeFromAnySide flag and make it the default behavior if the back-end claim to support mouse cursors.

@amc522
Copy link
Contributor Author

amc522 commented Mar 26, 2018

I think ImGuiBackendFlags_HasMouseCursors is a great addition. That would work for my use cases. As long as its well documented about what being assumed that you implement when enabling that backend flag.

ocornut added a commit that referenced this pull request Apr 6, 2018
…CursorChange. Followup to 75c3793 two weeks ago. (#787, #1495, #1202) + comments
@ocornut ocornut modified the milestones: v1.60, v1.61 Apr 7, 2018
@ocornut ocornut modified the milestones: v1.61, v1.62 May 14, 2018
ocornut added a commit that referenced this pull request Jul 6, 2018
…vor `io.OptResizeWindowsFromEdges=true` to enable the feature globally. (#1495) The feature is not currently enabled by default because it is not satisfying enough.
@ocornut
Copy link
Owner

ocornut commented Jul 6, 2018

@amc522 I have pushed a change to remove the ImGuiWindowFlags_ResizeFromAnySide flag and replaced it with a global setting io.OptResizeWindowsFromEdges = true.

// [BETA] Enable resizing of windows from their edges and from the lower-left corner. 
// This requires (io.BackendFlags & ImGuiBackendFlags_HasMouseCursors) because it needs mouse cursor feedback. 
// (This used to be the ImGuiWindowFlags_ResizeFromAnySide flag)
bool          OptResizeWindowsFromEdges;              

I'm currently not making it the default because the behavior is not satisfying enough (because of how we don't detect it when hovering slightly outside of window rectangle + probably we should have a stateful system where once the resize cursor is offered, we should allow the mouse to wander a little more without losing that possibility).

Whenever the behavior is improved we'll make it the default.

For now I think we can close this topic!

@ocornut ocornut closed this Jul 6, 2018
@amc522
Copy link
Contributor Author

amc522 commented Jul 6, 2018

Thanks for the update on the change. I dont have a lot of time right now, but maybe in a month or two i can start investigating how to use the mouse slightly outside the window.

ocornut added a commit that referenced this pull request Aug 1, 2018
… io.OptCursorBlink to io.ConfigCursorBlink, io.OptMacOSXBehaviors to ConfigMacOSXBehaviors for consistency. (#1427, #1495, #822, #473, #650)

Demo: Exposed flags in Demo.
ocornut added a commit that referenced this pull request Oct 2, 2018
…a flag) extends the hit region of root floating windows outside the window, making it easier to resize windows. Resize grips are also extended accordingly so there are no discontinuity when hovering between borders and corners. (#1495, #822, #2110)
ocornut added a commit that referenced this pull request Jan 23, 2019
…low the rounded corners. Border rendering moved to RenderOuterBorders so it can be called in a different order for docking. (#1495, #822)
ocornut added a commit that referenced this pull request Jan 23, 2019
…low the rounded corners. Border rendering moved to RenderOuterBorders so it can be called in a different order for docking. (#1495, #822)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants