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

Flutter Windows API v2 prototype for review [do not submit] #18112

Closed
wants to merge 13 commits into from

Conversation

clarkezone
Copy link
Contributor

@clarkezone clarkezone commented May 3, 2020

[NOTE: This PR is to solicit feedback on the shape of the API and changes to Client Wrapper, it will likely be abandoned in favor of a staged set of changes.]

Prototype of an update to the Windows API and shell implementation to remove the dependency on user32.dll and friends in order to enable alternative Windows host environments and runners to be supported.

At a high level this change:

  • Moves all input and windowing code out of engine into the client_wrapper.
  • Extend Windows Shell API surface to expose implementation neutral rendering and input calls that can support either a user32 or a potential future UWP runner (note this change doesn't incorporate UWP support merely enables it)

Remaining work needed to land:

  • Remove dependency on WindowsApp.lib
  • Task runner implementation still has a win32 dependency. Fixing that will require an API change
  • Fix copy paste
  • Make sure API boundary is exception safe
  • Fix Windows 7 compatibility
  • Verify that it's possible to write a UWP plugin
  • Update windows flutter create template with changes needed by runner
    Update
  • Update Flutter tooling as needed
  • Incorporate design feedback
  • Figure out best way of staging

@auto-assign auto-assign bot requested a review from GaryQian May 3, 2020 22:13
@clarkezone clarkezone added the Work in progress (WIP) Not ready (yet) for review! label May 5, 2020
@stuartmorgan
Copy link
Contributor

Seeing this in concrete terms, I'm pretty concerned by the amount of code that's moving from the embedding to the runner (we we lose all control over it). E.g., the unicode input handling fixes I made recently would be impossible here, because those bugs would have been baked into everyone's existing runners.

I think we'll need to seriously consider having multiple libraries (one for Win32, one for CoreWindow). I need to think about that some more though, as it has a lot of potential implications.

@clarkezone
Copy link
Contributor Author

clarkezone commented May 11, 2020

@stuartmorgan based on the above feedback I've

  1. prepared a more scoped change to remove the win32ness from the API: https://github.com/clarkezone/engine/tree/winapiv2b_proto

  2. what it would look like for testbed to consume that:
    https://github.com/clarkezone/flutter-desktop-embedding/tree/winapiv2b_proto

  3. what the engine refactoring would look like to build a separate library for win32 and uwp: https://github.com/clarkezone/engine/tree/uwp-prototype-7 (this only captures the uwp half, the win32 half would add win32_flutter_window_pub.h from this change back into engine and call into flutter_windows_view in the same way that uwp_flutter_window does to send input.

Although this approach has different tradeoffs, I agree it feels more encapsulated and cleaner.

@clarkezone
Copy link
Contributor Author

Based on feedback, this approach is being abandoned in favor of Refactor Win32FlutterWindow in preparation for UWP windowing implementation #18878 hence closing

@clarkezone clarkezone closed this Jun 7, 2020
@clarkezone clarkezone deleted the winapiv2_proto branch September 17, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Work in progress (WIP) Not ready (yet) for review!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants