all: nuke WindowGroup, overhaul Application, WindowBase, top-level an…#59
Merged
all: nuke WindowGroup, overhaul Application, WindowBase, top-level an…#59
Conversation
nickkhyl
approved these changes
Apr 9, 2024
Member
nickkhyl
left a comment
nickkhyl
left a comment
There was a problem hiding this comment.
LGTM with the following changes!
…d modal message loops WindowGroup was a bad abstraction that supported creating windows across multiple threads in the same application. This is completely opposite to the Win32 application model and should never have been supported. This PR completely removes WindowGroup, moving any of its code that was actually useful over to more appropriate locations. Instead, we add a top-level message pump via Application. I also did not particularly like the way that modal message pumps were attached to Forms, so I reimplemented the modal message loop as part of Application and then modified Form and Dialog to use the new stuff. This also allows us to fix issue 42: we don't need events to track how many of themselves are on the call stack; we guarantee that there are no events pending by virtual of the modal loop calling OnPostDispatch. To make all of this stuff happen, I rewrote a bunch of things in Application. I removed lazy initialization of Application; this is too fragile when we have important things that should ideally be initialized before we start creating windows and such. Programs must now explicitly call walk.InitApp() to initialize the framework, and call walk.App().Run() to execute the top-level message pump. We also add an allocator for WM_APP messages and perform setup of a rudimentary handler of (de)cloaking events. This PR also changes a bunch of things in WindowBase. Window initialization is broken down into smaller pieces such that they can be integrated into WM_NCCREATE and WM_CREATE handlers. We also move most of the implementation of WindowBase.Dispose into the WM_DESTROY handler. We add an allocator for WM_USER messages. We also remove Max and Min in favour of the Go's new built-in implementations. Some additional minor cleanup is done here and there. It's not directly related to the issues at hand, but since this PR already requires so many changes to example code (and eventually our own app), I felt that it was easier to do these things all at once. All sample programs in this repo have been updated accordingly. A follow-up PR is ready for our GUI but will not be merged until after the next release. Fixes #33 Fixes #42 Fixes #53 Fixes #58 Signed-off-by: Aaron Klotz <aaron@tailscale.com>
dblohm7
force-pushed
the
aaron/nuke_windowgroup
branch
from
6a81deb to
14f5a03
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
…d modal message loops
WindowGroup was a bad abstraction that supported creating windows across multiple threads in the same application. This is completely opposite to the Win32 application model and should never have been supported. This PR completely removes WindowGroup, moving any of its code that was actually useful over to more appropriate locations.
Instead, we add a top-level message pump via Application. I also did not particularly like the way that modal message pumps were attached to Forms, so I reimplemented the modal message loop as part of Application and then modified Form and Dialog to use the new stuff. This also allows us to fix issue 42: we don't need events to track how many of themselves are on the call stack; we guarantee that there are no events pending by virtual of the modal loop calling OnPostDispatch.
To make all of this stuff happen, I rewrote a bunch of things in Application. I removed lazy initialization of Application; this is too fragile when we have important things that should ideally be initialized before we start creating windows and such. Programs must now explicitly call walk.InitApp() to initialize the framework, and call walk.App().Run() to execute the top-level message pump. We also add an allocator for WM_APP messages and perform setup of a rudimentary handler of (de)cloaking events.
This PR also changes a bunch of things in WindowBase. Window initialization is broken down into smaller pieces such that they can be integrated into WM_NCCREATE and WM_CREATE handlers. We also move most of the implementation of WindowBase.Dispose into the WM_DESTROY handler. We add an allocator for WM_USER messages.
We also remove Max and Min in favour of the Go's new built-in implementations. Some additional minor cleanup is done here and there. It's not directly related to the issues at hand, but since this PR already requires so many changes to example code (and eventually our own app), I felt that it was easier to do these things all at once.
All sample programs in this repo have been updated accordingly. A follow-up PR is ready for our GUI but will not be merged until after the next release.
Fixes #33
Fixes #42
Fixes #53
Fixes #58