-
Notifications
You must be signed in to change notification settings - Fork 6k
WIP: Smooth window resizing on macOS #21252
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
iskakaushik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this maks the existing FlutterView class more complex.
I recommend creating new classes in their own files that take over some of the responsibilities from FlutterView.
Maybe we can do something like this:
FlutterIOSurfaceManager
- Manages the lifecycle of IOSurfaces: Creation, deletion, swap-buffers etc.
- This will be created in
initWithFrameof FlutterView and can also be called when replace existing usages ofrecreateSurfaceWithScaledSizeto recreate surfaces.
FlutterResizer
- Manages the synchronous resizing. This will hold the only reference to
FlutterIOSurfaceManager. - Keep track of the front and back-buffer.
The way I see it, FlutterView will hold FlutterResizer, which will then hold FlutterIOSurfaceManager. FrameBuffer related requests in FlutterView will be delegated to the FlutterReizer which will hold context about the current resize lifecycle and respond to requests using FlutterIOSurfaceManager.
This will let us unit-test each of these classes better.
| * Listener for view resizing. | ||
| */ | ||
| @protocol FlutterViewReshapeListener <NSObject> | ||
| @protocol FlutterViewDelegate <NSObject> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call this FlutterSurfaceDelegate and move present and getFrameBufferIdForWidth to this?
| */ | ||
| - (void)viewDidReshape:(nonnull NSView*)view; | ||
|
|
||
| - (void)scheduleOnRasterTread:(nonnull dispatch_block_t)block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "RasterThread". Also, given that the embedder method is called FlutterEnginePostRenderThreadTask, maybe we can call this: scheduleOnRenderThread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this. Apart from FlutterEnginePostRenderThreadTask, the thread is referred to as raster thread in most places in the code.
|
Replaced by #21525 |
Description
Superseds #21110
Make window resizing on macOS smooth, responsive and synchronous with windows size.
Related design doc: https://flutter.dev/go/desktop-resize-macos
Notable change: It seems that double buffered IOSurface backed NSOpenGLContext is not all that double buffered after all and I was able to create a test case which resulted in partial drawn frame visible. This version uses single buffered NSOpenGLContext with two IOSurfaces (front and back) which eliminates the problem.
Depends on: #21108
Related Issues
flutter/flutter#44136
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.