A new source code/header structure was introduced when the DlColorSource and DlImageFilter objects were migrated to Impeller geometry classes. Even though the DlColorFilter objects did not depend on Skia geometry objects, they need to be updated to the new source layout for consistency.
This is a combination of 3 reverts, required to get back to the revert that caused `ios_platform_view_tests` to start failing in the framework repo. In reverse chronological order, this reverts two trivial commits plus the non-trivial commit that likely caused the breakage:
* Revert "iOS: Eliminate global in platformviews controller (#56805)" This reverts commit cea4600caa7098fa7ec109d18b869db46cda726a.
* Revert "iOS: Delete FlutterPlatformViewsController.layerPoolSize (#56806)" This reverts commit 80fa8a590876e0d29055b9ddbfa8670c1f83759e.
* Revert "iOS: Migrate PlatformViewsController to Objective-C (#56790)" This reverts commit afd05afc406deb79fbe9c16684aeeeb19322b288.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Objective-C files should be named to match their header, in this case, //flutter/shell/platform/darwin/ios/framework/Headers/FlutterPlatformViews.h.
When private API is required, we often create a header named FlutterFoo_Internal.h, but the implementation file is always just FlutterFoo.mm. This brings FlutterPlatformViews back into line with convention.
No test changes since this is just a file rename.
[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.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This migrates PlatformViewController from C++ to Objective-C. Generally, we try to keep the embedder interfaces and components written in Objective-C except for the few places where C++ interfaces are requried to interface with engine APIs such as Shell and PlatformView (e.g. the PlatformViewIOS subclass). Now that the implementation is Objective-C, the class and file are renamed to match Objective-C naming conventions.
This allows us to take advantage of ARC and weak references, which eliminates the need for std::shared_ptr, fml::WeakPtr etc. Further, this eliminates some particularly unintuitive behaviour wherein this class was owned via a std::shared_ptr held by FlutterEngine, and injected into many other classes (e.g. AccessibilityBridge) via a std::shared_ptr& reference -- such that only one instance of the std::shared_ptr actually ever existed, presumably to avoid std::shared_ptr refcounting overhead. Given that this overhead was only incurred a single time at engine initialisation, this seems like overkill. One might ask why it wasn't therefore held in a `std::unique_ptr` and a `std::unique_ptr&` reference passed around. Likely, this was because we wanted to take a `fml::WeakPtr` reference on it.
Regardless, none of this is necessary any longer now that we can inject `__weak FlutterPlatformViewsController*` instances to classes that use it.
To be clear, this patch makes no attempt whatsoever to simplify or clean up the interface or implementation of this class. This class ties together far too many concepts and is injected into far too many places, and we should break it up and simplify it. However, the goal of this patch was simply to port to an Objective-C interface that plays nicely with the rest of the iOS embedder. This does include a couple minor cleanups in `#include`/`#import` order and usage to match our style guide.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts: flutter/engine#56606
Initiated by: LongCatIsLooong
Reason for reverting: https://github.com/flutter/flutter/issues/159456
Original PR Author: LongCatIsLooong
Reviewed By: {chunhtai, cbracken}
This change reverts the following previous change:
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible.
Previous PR for context: https://github.com/flutter/engine/pull/55964https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac
### Why the UIScrollView subclass in the focus hierarchy
The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.
### Things that currently may not work
1. Nested scroll views (have not tried to verify)
The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).
2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list
Video demo (as you can see the scrolling is really finicky):
https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978
I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
For https://github.com/flutter/flutter/pull/158881
The new Android m3 page transition animates a saveLayer w/ opacity + slide in animation. See example video from linked PR:
https://github.com/user-attachments/assets/1382374a-4e0c-4a0e-9d70-948ce12e6298
On each frame, we intersect the coverage rect of this saveLayer contents with the screen rect, and shrink it to a partial rectangle. But this changes each frame, which forces us to re-allocate a large texture each frame, causing performance issues.
Why not ignore the cull rect entirely? We sometimes ignore the cull rect for the image filter layer for similar reasons, but from observation, the sizes of these saveLayer can be slightly larger than the screen for flutter gallery. Instead, I attempted to use the cull rect size to shrink the saveLayer by shifting the origin before intersecting.
I think this should be safe to do, as I believe it could only leave the coverage as larger than it would have been and not smaller.
In flutter/engine#35501, handling was added to log a debug message to the console in the case where a platform view with a non-zero origin was identified.
Unfortunately:
* In unopt builds, the first thing we do in that block is to call FML_DCHECK asserting that the origin is zero, so we never actually emit the log statement.
* In opt builds, FML_DCHECK is a no-op, so users are unlikely to actually ever notice the crash.
The proper fix is to eliminate this restriction, but in the meantime, this eliminates this block entirely and leaves the TODO. We've had only two comments on that bug in the 2.5 years since it was added.
Issue: https://github.com/flutter/flutter/issues/109700
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible.
Previous PR for context: https://github.com/flutter/engine/pull/55964https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac
### Why the UIScrollView subclass in the focus hierarchy
The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.
### Things that currently may not work
1. Nested scroll views (have not tried to verify)
The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).
2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list
Video demo (as you can see the scrolling is really finicky):
https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978
I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Bumps the version of AGP used in the IDE-support `build.gradle`, as well as the robolectric version in both the IDE-support `build.gradle` and test-runner-`build.gradle`.
This is the current latest robolectric: https://github.com/robolectric/robolectric/releases/tag/robolectric-4.14.1.
Also
1. configures robolectric to use API 35, and
2. removes the use of a deprecated class which (from what I could tell) looked like it was just used for setup, and the test still passes without it.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Internal bug: b/380745221
The use of `malloc` in L70:
d48ceb1ba2/third_party/tonic/filesystem/filesystem/file.cc (L70)
Results in the following error, after an update to the internal toolchain. For some reason, this error only appears when targeting Linux.
```
tonic/filesystem/filesystem/file.cc:70:28: error: use of undeclared identifier 'malloc'
70 | uint8_t* ptr = (uint8_t*)malloc(file_size);
| ^
1 error generated.
```
This PR fixes it by including the header file.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts: flutter/engine#56738
Initiated by: jonahwilliams
Reason for reverting: speculative revert for framework failures.
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons}
This change reverts the following previous change:
Changes the following shell callbacks to flush the dart event loop:
* OnPlatformViewSetViewportMetrics
* OnPlatformViewDispatchPointerDataPacket
* OnPlatformViewDispatchPlatformMessage
* OnPlatformViewSetSemanticsEnabled
* OnPlatformViewSetAccessibilityFeatures
Using a new TaskRunner API RunNowAndFlushMessages. If the task runner can run tasks on the current thread, this will immediately invoke a callback and then post an empty task to the event loop to ensure dart listeners fire.
This also updates the vsync waiter to use RunNowOrPostTask, so that we start vsync events as early as possible.
The Skia snapshot controller will activate the delegate's surface render context on the current thread. If the delegate has no surface, then it will use the snapshot surface producer to create a temporary surface.
The Impeller snapshot controller needs to do the same in order to support OpenGL/GLES scenarios where the thread does not currently have an EGL context.
The DlColorSource code uses Skia geometry classes for its internal computations. This PR switches those implementations to use the Impeller geometry classes for consistency and 3rd party header file independence.
Creating and attaching textures/render buffers to a FBO is an expensive operation. Similar to how we cache vulkan framebuffers/render passes, we can cache the FBO object on the color0 texture to avoid extra state invalidation.
Part of https://github.com/flutter/flutter/issues/159177
Changes the following shell callbacks to flush the dart event loop:
* OnPlatformViewSetViewportMetrics
* OnPlatformViewDispatchPointerDataPacket
* OnPlatformViewDispatchPlatformMessage
* OnPlatformViewSetSemanticsEnabled
* OnPlatformViewSetAccessibilityFeatures
Using a new TaskRunner API RunNowAndFlushMessages. If the task runner can run tasks on the current thread, this will immediately invoke a callback and then post an empty task to the event loop to ensure dart listeners fire.
This also updates the vsync waiter to use RunNowOrPostTask, so that we start vsync events as early as possible.
Moves code specific to each graphics backend into the (existing) translation unit associated with that backend.
Previously, we could not include any Objective-C types in `shell_test_platform_view_metal.h`, since that file was included into `shell_test_platform_view.cc`, which is pure C++. To work around this, we had encapsulated Objective-C Metal types in a `DarwinContextMetal` struct hidden in the implementation file with a pointer to it forward-declared in the header.
We now use Metal types directly in the header, without the workarounds.
Issue: https://github.com/flutter/flutter/issues/158998
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
`kDefaultBackendType` is intended to make life easier for authors of tests, but in any switch statement where it's used (currently just a single location), we rely on ordering it first and `#ifdef`ing out all backends that aren't available.
Instead, we define a static function that returns the default that callers can invoke instead. This avoids relinace on case ordering and fallthrough.
In https://github.com/flutter/engine/pull/56722 we split backends out into separate translation units, and ideally should remove the `#ifdef`s, which means we can't rely on this trick anymore.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
