Improve dialog window association and _InputBox$ support on Windows #245
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Functionally it doesn't matter what kind of images we store for the tests, as _LOADIMAGE() will open them exactly the same. png however has the advantages of being substantially smaller, and also viewable directly on GitHub in PRs, making them easier to review.
Currently we have two different ways of determining what Window handle to tie our dialogs too - we either use GetForegroundWindow(), or create a completely new and hidden handle. The associated window determines what process names shows up on notifications, and also which window can't be interacted with while a dialog is open. Both of those approaches aren't really good. In the case of GetForegroundWindow(), it just returns whatever window the user has in focus, which might be a completely different process. With the hidden window, it means the dialog and notification aren't really tied to the QB64-PE program, so you can still interact with the window even when a dialog is open, and the notification doesn't show an exectuable name. To solve this we're now using EnumWindows() to enumerate over all the Windows on the system and find one associated with our ProcessId. We then check if it's the top-level window and return it if it is. If that process fails to find a window (such as if this is a console-only program, or $SCREENHIDE is used) then we check if GetConsoleWindow() gives us a handle and use that. If neither approach works, then we fall back to creating a hidden window so that the dialogs can still work.
This applies various dialog settings so that the dialog is always on top, and also so that Tab works as expected to move between the controls. The Edit control is moved first so that it's focused when the window appears.
It seems we weren't including stdint.h on Windows by mistake. Likely we were getting it anywy from some other header, but it sounds like that got changed in newer versions of MinGW and either way we shouldn't rely on that. Being that we include stdint.h on all platforms, I also changed os.h to always use these types when defining the `int32` and friends sized types. Ideally we get rid of those defines in the future but we can at least use the stdint.h types going forward (as we already have).
Thank you for doing this! Honestly, I think that all new stuff we are writing should use the |
a740g
approved these changes
Nov 13, 2022
SteveMcNeill
approved these changes
Nov 13, 2022
I agree, I've pretty much been doing that already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The commit messages have more detailed information, but a summary of the changes:
GetForegroundWindow(), which is pretty bad since it just gives the focused Window which might not even be ours. We now use three approaches (in this order) to get a window to associate with dialogs and notifications:a. Use
EnumWindows()and find a top-level window associated with our Process ID.b. Try using
GetConsoleWindow()and see if it gives us back a valid handle.c. Create a hidden window.
pnginstead ofbmp. For the tests it doesn't matter which we use, butpngs are smaller and also show up natively on GitHub.stdint.hon Windows (which we were already using but not including directly).