[macOS] Multiview compositor #52253
[macOS] Multiview compositor #52253
Conversation
|
Also cc @loic-sharma |
shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
Outdated
Show resolved
Hide resolved
| @@ -851,10 +860,6 @@ - (FlutterViewController*)viewController { | |||
| } | |||
|
|
|||
| - (FlutterCompositor*)createFlutterCompositor { | |||
There was a problem hiding this comment.
Is this still the right name for the method? "Create" usually implies allocation of some sort. Here what we're doing is more like initialisation the existing field and returning a pointer to it.
There was a problem hiding this comment.
I think it's still a reasonable name. For whoever uses FlutterEngine, this method creates a embedderAPI/FlutterCompositor object and returns it. How the object is memory-managed is implementation detail.
| FlutterCompositor(id<FlutterViewProvider> view_provider, | ||
| FlutterTimeConverter* time_converter, | ||
| FlutterPlatformViewController* platform_views_controller); | ||
| FlutterPlatformViewController* platform_views_controller, | ||
| dispatch_queue_t main_queue); |
There was a problem hiding this comment.
Is there ever a case where this isn't the main queue returned by dispatch_get_main_queue()? If not, I think we should just test dispatch_get_current_queue() == dispatch_get_main_queue() where necessary.
There was a problem hiding this comment.
I'm mostly thinking about testability, although there really aren't many tests for FlutterCompositor. However, the ThreadSynchronizer accepts the queue as a replaceable parameter (which was added when I wrote tests for it). I think it's a clearer design if the same queue is passed in for both classes in production code.
There was a problem hiding this comment.
Ok, ThreadSynchronizer has two initializers, one of which uses the main queue implicitly, which is used in production code. Still, its internal implementation allows a replaceable queue. I'm ok with any suggestion.
This PR makes the macOS
FlutterCompositorable to handle multiple views by adding methodsAddViewandRemoveView.These two methods must take place on the main queue (platform thread), which is indeed the case, but extra effort is made to assert so. This is important because, due to the constraint of the macOS system that requires rendering to take place on the platform thread (hence
FlutterThreadSynchronizer), no locks are needed to avoid race condition betweenAdd/RemoveViewand presenting.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.