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

refactor!: Simplify over-complicated platform adapter APIs #294

Closed
wants to merge 1 commit into from

Conversation

mwcampbell
Copy link
Contributor

I remembered a wart in the API of the core (non-subclassing) Windows adapter, which I mentioned in this comment on a winit issue. Basically, handle_wm_getobject doesn't really need to return an intermediate struct that is then converted into an LRESULT in a second step. Any window implementations that need to acquire a mutex, mutable borrow, or the like in order to handle the WM_GETOBJECT message can work around the reentrancy issue by storing the adapter in an Rc, obtaining a reference to the adapter, then calling handle_wm_getobject while the mutex/borrow/whatever isn't held. I decided that this workaround is better than complicating the API for everyone.

Then I realized that, by the same logic, we really don't need to have the update methods return a QueuedEvents struct that the caller then has to call raise on. On Windows, that separation could theoretically have allowed the updating and the raising of events to happen on different threads. But then I ended up making it all single-threaded anyway on macOS. So I decided to require that the top-level Adapter struct on Windows be bound to the thread that owns the window, while keeping the ability for UIA to run most of the provider methods on another thread. In any case, I eliminated QueuedEvents, so the API is simpler.

@mwcampbell
Copy link
Contributor Author

Let's put this on hold until I figure out what, if anything, I'm doing about #295. I just realized that this PR and some resolutions to that issue may conflict with each other.

@mwcampbell
Copy link
Contributor Author

I'm abandoning this PR. After thinking about this some more, I believe my earlier instinct was correct; it's a more decoupled design, and it might be necessary for integrating lazy initialization into the core Windows and macOS platform adapters as I'm thinking about doing (see #295). In particular, we shouldn't lightly discard the ability to apply tree updates on a background thread on Windows. If anything, we should figure out a way to do that on macOS as well, since everything else on macOS is bottlenecked on the UI thread already. And the API I proposed in the winit issue comment that I mentioned earlier, that got me started on the path of this PR, probably needs rethinking anyway in light of #295.

@mwcampbell mwcampbell closed this Sep 10, 2023
@mwcampbell mwcampbell deleted the simplify-adapter-apis branch April 25, 2024 17:40
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.

1 participant