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

Menus: slack triangle for submenu opening has better fit #6926

Closed
wants to merge 1 commit into from
Closed

Menus: slack triangle for submenu opening has better fit #6926

wants to merge 1 commit into from

Conversation

zeh
Copy link

@zeh zeh commented Oct 15, 2023

When rolling over menus, sometimes their submenus wouldn't open correctly, particularly on the far right side of the menu item (or far left for menus opening to the left).

As reported in #6671:

257549900-267a8420-e6a6-49ac-91a0-dca49a6e00cc

Problem context

The code for handling that behavior implements a solution like this post, where it uses an imaginary triangle to define an area that the cursor can still roll into (outside of the menu item space even) before the current submenu closes and a different one opens.

However, it seems to be the current code is attempting to move the corners of this imaginary triangle unnecessarily, making it more likely it wouldn't detect the mouse had left a valid area when it did. This change was made in 86bad3c9e1 as fixes to #5614.

Fix

My changes remove those moves, making it follow the original triangle corners more correctly, while still keeping some of its limits in place. It seems to make it feel more responsive, without making it behave incorrectly.

Capture showing how rolling over the end of the menu works, while still respecting the "slack" area that goes over other menus (without opening them):

imguimenu

(I do the "slack" rollover very quickly on the above capture, but it works fine in normal speeds as well)

After my changes, I wasn't able to reproduce the problem shown in #5614 either, which makes me believe it's addressed by the rest of the code already.

This was tested on Windows, example_win32_directx9 example, VS Community 2022 (17.7.4), against current master (v1.90 WIP), using a high-dpi monitor. However, the code seems to make it self-contained enough, so I don't believe we'd have problems in other platforms, or in different DPI monitors. If anything, it removes a calculation that seemed to be missing DPI scaling. Therefore, I believe it's a low-risk change.

Fixes #6671.

Don't move the corners of the triangle horizontally anymore. This gives more accurate angles for quick mouse moves
@zeh
Copy link
Author

zeh commented Oct 15, 2023

cc @OneForgottenMan if you have any additional opinions.

@ocornut ocornut added the menus menu bars, menu items label Oct 15, 2023
@ocornut
Copy link
Owner

ocornut commented Nov 29, 2023

Hello,

Unfortunately without backing debug visualization this seems like a blind attempt at improving the situation.
I don't think the change to tb.x and tc.x have anything to do with the fix, in fact it's clearly going backward undoing part of the fix for #5614 (which happens when crossing over multi-viewports), while not actually fixing #6671 at all (can still repro).

I believe we should boldly extend the geometry (into e.g. a quad covering more of the height of the parent menu item) + incorporate element of mouse stillness/timer into the heuristic.

ocornut added a commit that referenced this pull request Nov 29, 2023
…where a slow vertical movements toward another parent BeginMenu() can keep the wrong child menu open. (#6671, #6926)
@ocornut
Copy link
Owner

ocornut commented Nov 29, 2023

Should be solved by 2ee40d3, see #6671 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
menus menu bars, menu items
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hovering over menu entry not detected reliably
2 participants