Reverts: flutter/engine#55006
Initiated by: jonahwilliams
Reason for reverting: still failling mac module test
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons}
This change reverts the following previous change:
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
It turns out that when there are multiple paths to clip, they are unioned together, rather than intersected. But clipping paths need to be intersected.
There isn't any good way to intersect arbitrary paths. However, it is easy to intersect rect paths, which is the most common use case. Then we simply fallback to software rendering if we have to intersect non-rect paths. That is:
**Case 1** Only 1 clipping path (either rect path or arbitrary path):
Hardware rendering. This should be the most common use case
**Case 2** Multiple rect clipping path:
Hardware rendering. This is also common, and it's the linked issue that we are fixing.
**Case 3** Other complex case (multiple non-rect clipping path)
Fallback to software rendering. This should be rare.
After https://github.com/flutter/engine/pull/53826, we don't have a working benchmark that measures the main thread anymore. However, this PR shouldn't impact our ad benchmark, since it only has 1 clipping path. I will verify manually by checking Instruments and make sure no software rendering is happening. But we really should make the benchmark working again, not just for performance improvement, but also for monitoring regression.
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/153904
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Use default mouse event handling behaviour on macOS 13.3.1 onwards. This has two positive effects:
* Avoids the workaround on newer macOS versions where it's unnecessary, thereby giving us confidence that the underlying AppKit issue is fixed and the whole method can later be removed.
* Will be caught by tooling when we drop support for versions of macOS prior to the fixed version.
Issue: https://github.com/flutter/flutter/issues/154063
Issue: https://github.com/flutter/flutter/issues/115015
No tests modified since there is no semantic change, either on versions of macOS where the issue is fixed (and thus the default event handler is correct) or on versions where it is not (and we still use the workaround).
Re-tested manually with the reduced transparency setting on macOS 14.6.1.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes text input edit rotor accessibility missing edit actions.
It also fixes a few edit items that is displayed where it's not supposed to. (e.g. displaying select item while nothing to select).
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/151029
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Still needs to be tested in metalless environment. This should probably print out some kind of error message that tells folks that this is WAI, but I'm not sure if that would be too disruptive for g3.
Fixes https://github.com/flutter/flutter/issues/153883
Fixes https://github.com/flutter/flutter/issues/153335
When platform views are present, we always repaint entire frame. We must pass the buffer damage all the way to `GPUSurfaceMetalSkia/GPUSurfaceMetalImpeller` so that they know that the other two surfaces are lagging behind front surface and need to be repainted.
This is currently not being done with platform view present, that's why we miss full frame repaint when removing platform view.
With this merged in, it should be safe to reland https://github.com/flutter/engine/pull/54537 without any changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts flutter/engine#54453
Not quite ready to remove Impeller Scene yet because I'm still porting
the animation functionality. Keeping it around allows me to switch back
and forth to compare without having to recompile the engine.
Over a month ago I said something like "we can revert this a couple of
weeks from now" in one of the Impeller meetings. But for better or worse
(better, I think), I ended up spending a ton of time trying to make the
PBR good first (which doesn't exist in this C++ version).
Splits FlutterPlatformViews.mm into the primarily C++ class FlutterPlatformViewsController/Pool/Overlay and all of the touch interceptor / gesture stuff.
Renames FlutterPlatformViewsController to PlatformViewsController, because, you know.
Renames FlutterPlatformViewLayer and FlutterPlatformViewLayerPool to OverlayLayer and OverlayLayerPool
Remove the need to merge raster and platform thread in the presence of platform views by defering UIView mutation and presentation of flutter views into separate platform thread task. Fixes priority inversion problem cause by platform thread blocking on drawable aquisition.
Open questions:
* Fixed ~What is a better interface for handling the partial submit with impeller. (TBD)~
* Fixed ~Update: We Don't | How do we fix this for Skia~ Fixed
* Fixed ~Update: Done, we post a task to the platform thread. Is there a shorter term solution for creating overlay layers on the raster thread.~ Fixed
* Fixed ~Update: seems to. Does this perform well enough (independent of platform/ui thread merge and w/ thread merge).~ Fixed
Fixes https://github.com/flutter/flutter/issues/142841
part of https://github.com/flutter/flutter/issues/150525
Developers can control the backend in the following ways:
* **Do nothing**: Impeller with Vulkan is used where Vulkan is available with a fallback to Skia with OpenGL.
* **In `AndroidManifest.xml`, specify `io.flutter.embedding.android.EnableImpeller` as `false`**: Skia with OpenGL is used.
* **On the command line, specify `--no-enable-impeller`**: Skia with OpenGL is used.
Manifest options will take priority command line options when there is a conflict. This matches iOS behavior per https://github.com/flutter/flutter/issues/124049 (closed as WAI).
Fixes https://github.com/flutter/flutter/issues/149360
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
For this to work, we need to provide our own capture scope otherwise the default scope won't capture our commands.
This is required as part of the work to switch to unmerged threads for PVs (https://github.com/flutter/engine/pull/53826), as I can confirm @knopp 's observations that the performance is much worse with the default CAMetalLayer.
Fixes https://github.com/flutter/flutter/issues/140901
This removes support for "unobstructed platform views" on iOS - instead prefering to use the Android strategy of minimizing overlay layers, as this is generally more performant.
`FlutterPlatformViewLayerPool` is a pool of iOS layers used for rendering platform views on iOS. When layers are no longer needed, we currently mark them available for re-use but we never actually flush them and thus recover the memory associated with these layers once we know that the layers are no longer needed.
We now flush the layer pool on `SubmitFrame` if a previously-used layer is no longer used in the current frame. In theory, this could cause a performance regression in the case where an additional layer is needed every second or third frame, but we should keep it simple on the first pass and only complicate things later if warranted.
Fixes: https://github.com/flutter/flutter/issues/152053
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR makes the macOS `FlutterCompositor` able to handle multiple views by adding methods `AddView` and `RemoveView`.
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 between `Add/RemoveView` and presenting.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Experimentally support merging UI and platform thread on Android/iOS. This works by changing the shell holder to set the UI thread to the platform thread. Several shell APIs that post messages from the platform to ui thread were changed to use RunNowOrPostTask to immediately call the UI task if the threads are the same.
Experimentally, this seems to work reasonably well if there are no platform views. On Android with TLHC it works fine either way, while iOS currently takes a big performance hit.
This can be opted into via a Plist:
```
<key>FLTEnableMergedPlatformUIThread</key>
<true/>
```
Or via AndroidManifest.xml:
```
<meta-data
android:name="io.flutter.embedding.android.EnableMergedPlatformUIThread"
android:value="true" />
```
https://github.com/flutter/flutter/issues/150525
Fixes https://github.com/flutter/flutter/issues/151274
This is only an issue on iOS (so far) because of the platform view rendering strategy that involves submitting multiple impeller frames per compositor frame.
FYI @bdero
When `FlutterUndoManagerPlugin` deallocated, it is supposed to deregister itself from the `NSUndoManager` stack.
However, during dealloc `_undoManagerDelegate.undoManager` is nil (`_undoManagerDelegate` = the engine, and `undoManager` was its view controller's `undoManager`, which was already gone).
Since `_undoManagerDelegate.undoManager` is nil, it doesn't actually call `-[NSUndoManager removeAllActionsWithTarget]`. In the add-to-app scenario, after the view controller pops back to the native view, and "undo" is invoked, the undo action for the `FlutterUndoManagerPlugin` is handled, but it already dealloced so crash.
Fixes https://github.com/flutter/flutter/issues/150408
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
fix cursor hit testing in platform view in #52159
We found that the mechanism of `[NSView hitTest: view] `will handle the
flip coordinate automatically. In the previous PR, because the superview
of FlutterView was not flipped, we should not convert the point using
`[self convertPoint:event.locationInWindow fromView:nil]` .The
FlutterView hit test should run under the window coordinate system;
otherwise, FlutterMutatorView will hit a wrong point.
Before the fix:
Actual mouse move: `event.locationInWindow (x = 47.45703125, y =
416.86328125)`
FlutterView HitTest: `(NSPoint) point = (x = 47.45703125, y =
203.13671875)`
FlutterMutatorView HitTest: `(NSPoint) point = (x = 47.45703125, y =
416.86328125)`
After the fix:
Actual mouse move: `event.locationInWindow (x = 47.45703125, y =
416.86328125)`
FlutterView HitTest: `(NSPoint) point = (x = 47.45703125, y =
416.86328125)`
FlutterMutatorView HitTest: `(NSPoint) point = (x = 47.45703125, y =
203.13671875)`
## 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].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or 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].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
In https://github.com/flutter/buildroot/pull/860, disabling `NSAssert` in release build was added, and https://github.com/flutter/engine/pull/53005 tries to roll the change into the engine. However, the change is blocked due to several compilation errors in the engine.
This is due to the fact that Google's `DCHECK`(in flutter engine, `FML_DCHECK`) does not make unused variable warning even in release build, while traditional assertion macro like `assert` or `NSAssert` does. For example, the following code will be expanded like this:
code:
```c++
FML_DCHECK(status == kCVReturnSuccess && pxbuffer != nullptr) <<
"Failed to create pixel buffer";
```
expanded code:
```c++
true || (status == kCVReturnSuccess && pxbuffer != nullptr)
? (void)0 ::
: fml::LogMessageVoidify() &
::fml::LogMessage(::fml::kLogFatal, 0, 0, nullptr).stream()
```
However, equivalent code with `NSAssert` will make unused variable warning, since the expanded code doesn't have any reference to the variable:
code:
```c++
NSAssert(status == kCVReturnSuccess && pxbuffer != NULL, @"Failed to create pixel buffer.");
```
expanded code:
```c++
do {
} while (0)
```
To unblock the build, this CL replaces several `NSAssert` with `FML_DCHECK` in the engine codebase. This CL isn't aimed to replace all `NSAssert` with `FML_DCHECK` since discussions in https://github.com/flutter/buildroot/pull/860 are not finalized whether to replace all `NSAssert` with `FML_DCHECK` or not.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1. Migrate TextInputSemanticsObject to ARC
2. Rename the Objective-C-only `accessibility_text_entry.*` files to `TextInputSemanticsObject.*`
3. Move `FlutterInactiveTextInput` interface from the header file into the implementation, the only place it's used.
Blocked on `SemanticsObject` ARC migration https://github.com/flutter/engine/pull/52824
Part of https://github.com/flutter/flutter/issues/137801.
