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

Convert all tool Popups to floating windows #1658

Merged
merged 23 commits into from
May 10, 2024

Conversation

SparkyTD
Copy link
Contributor

@SparkyTD SparkyTD commented May 8, 2024

Problem description

In previous versions of ImHex, all tool windows were implemented as static popups fixed in the upper left position of the hex view. This PR refactors all tool popups to use floating windows that can be dragged around by the user, or closed with a dedicated close button on the title bar. These popup also support a stylable transparency when the user is not hovering their mouse over the window.

Implementation description

I rewrote the logic in ViewHexEditor::drawPopup() to use a custom ImGuiExt::BeginHoveringPopup function for rendering the popup windows. This new function is an almost exact replica of the built-in ImGui::BeginPopupModal, except it does also displays the default window title bar with a close button.

A second custom function, ImGuiExt::PopupTitleBarButton was also added for rendering small icon-based buttons into the title bar of the parent popup window. This new function was used to implement an optional "Pinning" feature that individual popup implementations can specify. If a window is pinned, it won't close automatically when its main action is executed. For example, the "Select" button on the Select dialog will close the popup by default, unless the window is pinned.

Screenshots

Popup dialogs before:
image

Popup dialogs after:

Recording.2024-05-08.233435.mp4

Additional things

  • When the user stops hovering their mouse over a popup window, it becomes semi-transparent, making it easier to see the content behind it
  • This PR also introduces the styles.imhex.popup-alpha style, making the transparency effect configurable, including the ability to disable the effect completely by setting popup-alpha to 1.0.
  • Fixed a bug that caused some popup windows to ignore the Enter and the KeypadEnter keys. With this PR, all tool windows will execute their main action when the user presses either one of the two Enter keys, and will also close automatically unless the window is pinned.

Possible changes and improvements

  • Should the transparency effect be disabled if a window is pinned?
  • Should the transparency factor be modifiable on the Settings/Interface page?
  • A keyboard shortcut could be added for quickly pinning / unpinning the current window.
  • Can the pin icon stay on the left, or should it be moved next to the close button, with a similar circular background?

SparkyTD added 12 commits May 8, 2024 05:37
…opups

Additional changes:
- Added a 'Pin' button to the floating popup's toolbar. When a tool is pinned, the popup will not close automatically after the action is executed.
- Added a virtual function to the `ViewHexEditor::Popup` class, allowing implementations to specify a tool window title.
Before this change, the modifications to the tool popup UIs did not respect the user's scaling factor settings.
…window when the user isn't hovering over it

In the previous approach, I was using ImGuiWindowFlags_NoBackground combined with an explicitly drawn background rectangle to correctly set the window transparency within ImGuiExt::Begin/EndHoveringPopup(). However with this approach, there was a small but noticeable gap between the window's external borders and the background rectangle, due to the configured default padding.

With this new approach, the decision to apply the transparency will always be delayed by one frame, but theis way the whole window can be set to being transparent before ImGuiExt::BeginHoveringPopup().
@SparkyTD
Copy link
Contributor Author

SparkyTD commented May 9, 2024

I found a bug that makes popup windows unusable after they've been dragged outside of the root window.

Repro:

  1. Open the Select popup (or any other popup)
  2. Drag the newly spawned window outside of the root window's bounds
  3. Click anywhere else, e.g. on the hex editor to close the popup
  4. Open the popup again (e.g. Ctrl+E)
  5. After reopening, any mouse click within the popup's background or title bar will immediately close it. Clicking on a widget inside the popup will not close it.

EDIT: The commit below (72701a6) fixes this issue, but more rigorous testing might be useful.

…isappear on click

This if branch handles the case when the user clicks outside of the currently visible popup, and it loses focus. But if the popup loses focus while `ImGui::IsWindowHovered()` still returns true, it means that the focus loss was not caused by the user clicking away, so it should not be closed yet. Since the focus loss was only registered when clicking on the background / title bar of the popup, and not any of its child widgets, this could be an ImGui bug.
@WerWolv WerWolv merged commit 973af46 into WerWolv:master May 10, 2024
15 checks passed
WerWolv pushed a commit that referenced this pull request May 10, 2024
### Problem description
Merging my previous PRs, #1660 and #1658 has resulted in a conflict
causing an application crash due to a missing `ImGui::BeginDisabled();`
in the `PopupSelect` class. Specifically, none of the code related to
offset validation made it into the final merge. This PR fixes the crash
by reintroducing the deleted lines.

### Additional things
The nightly release build seems to be unaffected by the crash, most
likely because ImGui's `DisabledStack` assertions are only enforced in
Debug mode. The offset validation was still missing before this fix.
@SparkyTD SparkyTD deleted the dev_floating_popups branch May 13, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants