engine icon indicating copy to clipboard operation
engine copied to clipboard

[macOS, multiwindow] Compositor gets FlutterView lazily

Open dkwingsmt opened this issue 2 years ago • 5 comments

This PR changes macos/FlutterCompositor so that instead of getting FlutterViewController* at construction, it gets a callback that provides the FlutterView* lazily.

Part of the multiwindow project (design doc): This is a necessary step for multi-window because FlutterCompositor is constructed at engine initialization, while there can be multiple views or view controllers added after initialization.

Existing tests are adapted. No tests are added.

Pre-launch Checklist

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • [ ] I listed at least one issue that this PR fixes in the description above.
  • [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • [x] I updated/added relevant documentation (doc comments with ///).
  • [x] I signed the CLA.
  • [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

dkwingsmt avatar Sep 23 '22 21:09 dkwingsmt

From PR triage: @dkwingsmt is this ready for another review pass from @stuartmorgan?

zanderso avatar Sep 29 '22 20:09 zanderso

@zanderso @.stuartmorgan said he did not know enough to review this PR. Can you help me find someone to review it?

dkwingsmt avatar Sep 29 '22 23:09 dkwingsmt

I'm trying to understand how this would get used in the multi-window context, can you provide a brief sketch of how you would end up wiring the get view callback when there are multiple views at play?

iskakaushik avatar Sep 30 '22 12:09 iskakaushik

Sure!

How will the compositor get view controllers in the future?

  • The macos/FlutterEngine currently stores a single viewController, but in the future it will store a map of view controllers, which maps from surface IDs to view controllers (every view controller will have a unique ID).
  • The view controller currently uses a fake ID 0 to call the callbacks, but in the future it will receive correct IDs.

Both changes (among others) are described in the design doc. It's still in progress, but contains sufficient information for this PR. https://docs.google.com/document/d/1U_ciMTHJgDrnw8kIo9sGKhiKFOycWRmz1yVVI_osCXg/edit

The core takeaway is that, in a multi-window world,

  • A view controller corresponds to a view, therefore there will be multiple view controllers (and they can be added after the app starts).
  • The Compositor class is a singleton.

Therefore the compositor must be able to handle additional views, instead of just the one from the constructor. It must either have a method of addViewController, or have a callback to get view controllers lazily.

dkwingsmt avatar Sep 30 '22 18:09 dkwingsmt

Gold has detected about 2 new digest(s) on patchset 17. View them at https://flutter-engine-gold.skia.org/cl/github/36392

skia-gold avatar Oct 08 '22 08:10 skia-gold