-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Add custom cursors to scrollbars, buttons, sliders, drag widgets, etc. #6442
base: master
Are you sure you want to change the base?
Conversation
Hello, Thanks for the PR! This is a good reference. Some feedback:
Design wise:
I'm not really sure what to do about this, writing my thoughts down for down. |
Hello, Thanks for the feedback. I will comment on and fix the immediate technical issues and concerns within a couple of hours. As for the design plan: Hand cursor on buttonsI do agree that the behaviour of having the hand button on every button is non-standard, even on the web, buttons were not designed to have this type of cursor. For one reason or another, it has now become the unofficial standard to use hand buttons on the web. There are many arguments for and against it, which I will not get into. I think that having this feature being optional, because of its unofficial nature, is the best way to handle it. However, it may be better, as you said, to have a type of setting that, for example, enables this only on The same thing can be done for sliders, etc. The "I don't want this cursor" issueIf we take the feature from above, why not make the user able to select the type of cursor for these widgets too Third party widgets' implementationI'm not sure how to handle this. I looked at the internals of the most popular libraries to calculate some type of rough impact:
Well... this is not ideal. There is basically no non-intervention way to fix this. I will add these libraries to a test project to make a more detailed impact report soon. |
Applied, signature returned to old state and called
Now we don't call
Switched to checking with
Switched to recommended pattern |
By the way, if (held || (hovered && g.HoveredIdPreviousFrame == id && g.HoveredIdTimer >= hover_visibility_delay))
SetMouseCursor(axis == ImGuiAxis_Y ? ImGuiMouseCursor_ResizeNS : ImGuiMouseCursor_ResizeEW); Should this also be applied to the other widgets? |
This is designed to reduce visual noise when moving mouse around, especially since they are thin borders that are frequently crossed over with no intent of staying over. |
May I suggest to occasionally rebase over master + force push instead of merging master + push, this will create less commit cruft in the history. (But it's not too important as normally if we adopt the commit, we can squash and rebase). |
I'm sorry, I was iterating over my private forks and this seems to have passed through. I will discard the commit right away |
… outside functions + remove calls to ImGui::IsItemHovered() internally
Thinking about this, what is your opinion on making this into a style-var-like API? The benefits of this, in my eyes, are these:
|
I am sorry can't validate or come up with a design when designing this is the main chunk of the work. |
Hello, firstly, thanks for maintaining such a great GUI library.
How I found this problem
Since dear imgui has support for setting and using different cursors, I decided it would be a good idea to use the hand cursor for buttons. But after scrolling through the settings and the source code, I found that no such feature was present.
This is what cursors look like normally:
Now with the patch:
I am aware of the fact that I can use
ImGui::SetMouseCursor(ImGuiMouseCursor_...)
like how it is shown in the demo window, however, this would bloat too much of the code for such a small aesthetic feature and would require me to rewrite 1000s of lines of perfectly functioning dear imgui code.I decided that the best course of action would be to integrate it into the backend.
Why make this PR
I think this would be beneficial to the dear imgui community, because by adding this, developers can provide better UX for their end users. Additionally, taking an example from the most used UI framework, the web, buttons and links on almost all sites use the Hand button, for example.
Furthermore, I have created a new IO variable
ConfigUseDefaultMouseCursors
, which can be used to enable/disable this feature with it beingfalse
by default, as to not break any existing behaviour. This way, we can still preserve the current imgui style, while allowing these mouse cursors to be turned on, for those who need them.Contents of the PR
This PR adds the following:
ConfigUseDefaultMouseCursors
, which can be used to enable/disable this feature, which is set tofalse
by default, as to not break any existing behaviourButtonBehaviour
, except forInvisibleButton
, since that widget is used by many non-button interfaces, and it's not that common, so it would be better to use the regular cursorShowcase
Horizontal slider:
Vertical slider:
Drag and drop:
Horizontal scrollbar:
Testing
I tested this patch with the latest dear imgui version and using the following examples:
null
glfw_opengl3
glfw_opengl2
glfw_vulkan
sdl2_opengl3
sdl2_opengl2
sdl2_sdlrenderer
sdl2_vulkan
glut_opengl2
The rest are not tested for the following reasons:
allegro
- Allegro package is broken on my Linux distributionandroid
- Android emulators are broken on my systemapple
- Don't have an apple systemglfw_metal
- Don't have an apple systemsdl2_directx
- Don't have a Windows systemsdl2_metal
- Don't have an apple systemsdl3
- SDL3 is in the testing phase but still not included in my Linux distribution as of nowwin32
- Don't have a Windows systemwgpu
- WGPU is still not supported on Chrome or Firefox on Linux