Ensures that the onscreen command buffer is blocked via the render ready
semaphore, instead of just the layout transition. I can confirm this
fixes the rendering artifacts on the Samsung a50 - which I suspect are
the same as the issues these other devices are having. Essentially,
without the semaphore protecting the onscreen pass we can end up writing
to the color attachment while it is still being read.
* https://github.com/flutter/flutter/issues/160522
* https://github.com/flutter/flutter/issues/160370
Closes https://github.com/flutter/flutter/issues/160933.
The timing of this callback gives our users (and plugin authors) a
chance to stop using the `Surface` before it becomes invalid, allowing
us to fix https://github.com/flutter/flutter/issues/156488 - we no
longer need to do shenanigans on storing and restoring state because
`ExoPlayer` can now handle it out of the box; see
https://github.com/flutter/flutter/issues/160933#issuecomment-2564092567.
It's unfortunate we have to go through a bit of churn on the callback
API, but realistically this _is_ the feedback we were looking for when
originally creating it - it just took longer than expected due to the
long release cycle.
/cc @hasali19, @xxoo, @camsim99
Fixes https://github.com/flutter/flutter/issues/160480
Both The Impeller and Skia variant of the OES texture rendering use the
same shared code path, so the Impeller code must match the weird 1x1
texture behavior of Skia. In addition, we have to add back the
TiledTextureContents support, since we need to render a texture with a
transform. I had previously tested this but neglected to force the
SurfaceTexture path, so I only tested the ImageReader path which does
not use a transform.
Adds files from flutter/flaux which contain modifications for the engine
structure. The history for engine/ has been edited. Please see
flutter/engine for the original PRs.
The RGB565 format documentation in embedder.h incorrectly stated that the red component uses the least significant bits. Unit tests in embedder/testdefs/embedder_unittests.cc demonstrate this is incorrect, showing:
- Red test (0xF800): Uses bits [15:11]
- Green test (0x07E0): Uses bits [10:5]
- Blue test (0x001F): Uses bits [4:0]
This commit fixes the documentation to correctly reflect the actual bit layout:
- Red uses 5 MSBs [15:11]
- Green uses 6 middle bits [10:5]
- Blue uses 5 LSBs [4:0]
Also fixes the example bit extraction code to use correct masks, matching the test expectations.
Impact: This change helps prevent potential developer confusion about RGB565 bit ordering and ensures the documentation matches the actual implementation as verified by the test suite.
The old path doesn't exist.
`dart format` stumbles over this non-existent include.
`analysis_options.yaml` files that just imported something non-existent were deleted.
I am surprised that this never caused any other issues. Is this all dead code that isn't actually analyzed?
This PR limits the search depth, because we don't want to enable this workaround for AdMob banner, which has a WKWebView in the depth of 7. See the previous PR for more context: https://github.com/flutter/engine/pull/57168
I was able to confirm that this returns YES for the 3P plugin, and NO for AdMob.
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/flutter/flutter/issues/158961
*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
Continuing the migration of engine code to the new geometry classes. Only DlRTree and DlRegion are converted in this pass, plus a small amount of associated code.
The original workaround ([PR](https://github.com/flutter/engine/pull/56804)) works for the official web view plugin, but it doesn't work for a third party plugin `flutter_inappwebview` ([issue](https://github.com/pichillilorenzo/flutter_inappwebview)). Upon discussion with the author of that plugin, it turns out that their platform view is not a WKWebView, but rather a wrapper of WKWebView.
This PR performs a DFS search of the view hierarchy, and enable the workaround as long as there's a WKWebView inside.
TODO: pending sample project:
I am quite positive that it should work, but **I haven't tried it since I don't have a sample project yet**. I have requested a sample project with them so I can verify the solution.
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/pichillilorenzo/flutter_inappwebview/issues/2415
*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
Eliminates some cases where `FlutterViewController` was relying on `FlutterEngine` internals:
* `[FlutterEngine shell]`
* `[FlutterEngine platformView]`
* `[FlutterEngine iosPlatformView]`
Instead, `FlutterEngine` now exposes:
* `installFirstFrameCallback:`
* `enableSemantics:withFlags:`
* `notifyViewCreated`
* `notifyViewDestroyed`
* `waitForFirstFrameSync:callback:`
Also fixes a couple cases where we were relying on transitive header includes:
* `FlutterAppController` relied on `FlutterViewController_Internal.h` for `sendDeepLinkToFramework:completionHandler:`
This is a refactoring followup to https://github.com/flutter/engine/pull/57099 that introduces no semantic changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
`GetLastError` returns an unsigned 32 bit integer that was being
implicitly cast to an int for the std::variant<..., int>. This was
causing my build to fail with:
```
../../flutter/shell/platform/windows/platform_handler.cc(178,12): error: no viable conversion from returned value of type 'DWORD' (aka 'unsigned long') to function return type 'std::variant<std::wstring, int>' (aka 'variant<basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t>>, int>')
178 | return ::GetLastError();
| ^~~~~~~~~~~~~~~~
../../../../../../../Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.42.34433/include\variant(923,7): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'DWORD' (aka 'unsigned long') to 'const variant<basic_string<wchar_t>, int> &' for 1st argument
923 | class variant : private _SMF_control<_Variant_destroy_layer<_Types...>, _Types...> { // discriminated union
| ^~~~~~~
```
Commands:
```
./flutter/tools/gn --runtime-mode release --no-rbe
ninja -C .\out\host_release windows gen_snapshot flutter/build/archives:windows_flutter
```
Explicitly casting `::GetLastError` to an int fixes this issue.
I'm running on Windows 11 (Version 10.0.26100 Build 26100) with VS 2022
Community Edition.
@loic-sharma
Co-authored-by: Eric Seidel <eric@shorebird.dev>
Collection of changes to DlVertices::Builder and the stopwatch visualizer.
At a high level:
* improve performance of the stopwatch visualizer by pre-allocating storage (and sharing it across both visualizers), lookup up font once, and cache the debug frame rate used. Updates to use Dl types instead of SkTypes.
* Change DlVerticesBuilder to allow storing the bounds and use that in the visualizer, since we already know them. Make FML_CHECKS into dchecks, as the dart:ui vertices will already bounds check correctly - so these should only be necessary for debugging engine changes.
`FlutterEngine` at the `_shell` unique_ptr ivar it owns have different lifetimes. `_shell` is initialised transitively from `runWithEntrypoint`, and reset in `[FlutterEngine destroyContext]`, which is called transitively from `[FlutterviewController dealloc]` via `[FlutterEngine notifyViewControllerDeallocated]`.
As such, all uses of `_shell` should be checked either via an assertion, in cases we know the shell should be non-null, or via a runtime null check in cases where it's expected that it may be null.
Specifically, this guards against a crash that can occur if we get a CoreAnimation transaction commit callback for an inflight frame just as we're shutting down the app (or removing the FlutterView in an add-to-app scenario).
Example stack trace:
```
0 Flutter 0x11b28 -[FlutterEngine platformView] + 53 (weak_ptr.h:53)
1 Flutter 0x11994 -[FlutterEngine updateViewportMetrics:] + 186 (ref_ptr.h:186)
2 Flutter 0x1f854 -[FlutterViewController updateViewportMetricsIfNeeded] + 427 (vector:427)
3 Flutter 0x1f9b8 -[FlutterViewController viewDidLayoutSubviews] + 1411 (FlutterViewController.mm:1411)
4 UIKitCore 0x8c864 -[UIView(CALayerDelegate) layoutSublayersOfLayer:] + 2376
5 QuartzCore 0x1fa0c CA::Layer::layout_if_needed(CA::Transaction*) + 516
6 QuartzCore 0x1ae84c CA::Context::commit_transaction(CA::Transaction*, double, double*) + 516
7 QuartzCore 0x2888 CA::Transaction::commit() + 648
```
Issue: https://github.com/flutter/flutter/issues/98735
Issue: https://github.com/flutter/flutter/issues/159639
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Migrate the use of fuchsia.io open functions to the new open3 replacements (which also requires transitioning from OpenFlags -> Flags). Likewise, we update all uses of the SDK VFS library to use the new set of flags.
This work is being done as part of the ongoing io2 migration in https://fxbug.dev/378924259
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Remove pending keyboard event ID and use the object directly.
Use references to make sure FlKeyboardManagerData is freed.
Refactor marking events as handled
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer.
The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup.
This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer.
Here are the steps of my research for future reference:
1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped.
2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer.
3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state.
4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state).
5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state.
6. I tried recreating a new delaying recognizer when `blockGesture` is called:
```
- blockGesture {
delayingRecognizer.state = .ended
dispatch_async {
// force re-create the delaying recognizer
}
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer.
7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works).
8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers.
9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state.
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/158961
*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
In the case where `CreateImpellerContext` encounters an error while creating an `impeller::ContextMTL`, we logged an error, returned nullptr, then immediately dereferenced the null pointer. Now, rather than crash due to a segfault, we now intentionally abort with an appropriate error message.
This adds checks in the `FlutterDarwinContextMetalImpeller` initialiser that aborts with an appropriate error message if impeller context creation fails, Metal device creation fails, or texture cache creation fails.
Rather than bailing out and returning nil from the initialiser to pass the buck to the caller, we terminate since without a graphics context, the app won't be able to render anything to begin with.
Issue: https://github.com/flutter/flutter/issues/157489
Issue: [b/378790930](http://b/378790930)
No test changes since this just changes an accidental crash to an intentional crash.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Split the channel messaging out of the system channels to make them
simpler to understand and refactor.
The new channel classes could be automatically generated in a second
phase, e.g. using Pigeon to reduce code usage.
The new classes don't have tests as they will already be covered by the
existing code.
The previous mock required knowing the specific functions used in the
binary messenger, this method instead allows test code to provide
complete platform channel implementation for testing and make simulated
platform channel calls into embedder code.
Incorrect reference counting of GTask objects meant platform channel
method calls would leave tasks alive that would leak memory and leave
unclosed references to the binary messenger.
The same problem with NVIDIA drivers which causes issue [152099](https://github.com/flutter/flutter/issues/152099) occurs with Vivante Corporation drivers.
Quick fix for issue on Vivante drivers:
https://github.com/flutter/flutter/issues/152099
- [ 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.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Applies non-null by default annotations to
FlutterPlatformViewsController with opt-outs where necessary. Updates unit tests to create graphics contexts where requried.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This is a minor refactoring that moves a global bool to a boolean ivar in FlutterPlatformViewsController. The purpose of this variable is simply to avoid the overhead of trying to create backdrop filters if we've ever failed to create one in the past.
Given that this class will only ever have the one instance created and held by per engine with the same duration, and that most apps only ever have one engine, the performance win will be identical for most apps. For the few add-to-app cases with multiple engines either at once or over the course of an app's lifetime, the costs associated with firing up an engine are already a far bigger hit than those being saved by this bool.
Also migrates from C++ style namespace { ... } to Obj-C style static functions. These are entirely equivalent as both restrict symbols to the current translation unit.
This is a reland of https://github.com/flutter/engine/pull/56805, which was reverted as part of https://github.com/flutter/engine/pull/56817.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
