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.
Unlike https://github.com/flutter/engine/pull/56738, does not touch the vsync API - which looks like it depends on scheduling behavior today, at least for iOS.
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.
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.
Enables the `-fobjc-arc` compiler flag for Objective-C and Objective-C++
translation units.
Eliminates the flutter_cflags_objc[c]_arc settings, since they're now
redundant.
All Obj-C/Obj-C++ code in our codebase has now been migrated to ARC.
Issue: https://github.com/flutter/flutter/issues/137801
`fml::CFRef` implements the bulk of the operations implemented in the one-off `Scoped` class except with better safety guarantees such as `[[nodiscard]]` on the `Release` method. It doesn't implement the `handle()` method that allows direct writing into the internal storage of the wrapper, but that method is effectively an escape hatch for all the safety guarantees provided by the wrapper, so it seems safer to avoid adding it.
No changes to tests since this includes no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Document where CFRef takes over or hands back ownership of the underlying CoreFoundation object memory.
Migrates manual CoreFoundation object management to CFRef in:
* impeller/golden_tests/metal_screenshot.mm
* shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.mm
* shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm
* shell/platform/darwin/ios/framework/Source/FlutterView.mm
* shell/platform/darwin/macos/framework/Source/FlutterSurface.mm
Adds a `Retain()` method to take shared ownership of the underlying object, as opposed to Reset, where ownership is transferred to the CFRef wrapper.
Adds a `Get()` method to make dealing with bridged Objective-C casts more convenient:
```objc
fml::CFRef<CFStringRef> cfString(...);
NSString* aString = (__bridge NSString*)cfString.Get();
```
as opposed to:
```objc
fml::CFRef<CFStringRef> cfString(...);
NSString* aString = (__bridge NSString*)static_cast<CFStringRef>(cfString);
```
I considered making use of `fml::scoped_policy::OwnershipPolicy` to add a second parameter to the ctor and `Reset` but, but I think documentation and addition of a `Retain()` method makes things a little clearer at the call site. It's also more consistent with `sk_cfp`, which we use in some Skia bits of the codebase.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Eliminates the two remaining uses of ScopedBlock and with it, ScopedTypeRef which was only used by ScopedBlock. ARC automatically generates the necessary block copy/retain/release calls, including moving blocks to the heap during a retain so manual _Block_copy/_Block_release calls are no longer required.
No test changes since no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Ensure that all Objective-C code in the codebase is being built with the standard set of Flutter Objective-C compiler flags with ARC enabled.
Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent.
Migrates The following targets to ARC, which had been missed in previous passes since they didn't declare the standard Flutter Obj-C[++] cflags:
* `//flutter/fml:fml_unittests`
* `//flutter/impeller/golden_tests:metal_screenshot`
* `//flutter/impeller/playground:playground`
* `//flutter/impeller/backend/metal:metal`
* `//flutter/impeller/backend/metal:metal_unittests`
* `//shell/gpu:gpu_surface_metal_unittests`
* `//flutter/shell/platform/embedder:embedder_unittests`
This patch includes no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
fml::scoped_nsobject and fml::scoped_nsprotocol classes are no longer
necessary now that the entire codebase has been migrated to ARC.
This also eliminates the fml_arc_unittests target which tests that code.
All references to that target in documentation, run_tests.py have been
removed. This target was used in various `et` build/query unit tests and
has been replaced with the fml_unittests target in those cases.
Issue: https://github.com/flutter/flutter/issues/137801
This issue was found with memory sanitizer.
Commit 988c4ffb83398bf8511122d73f0f85010e0edeea introduced a change that leads to use-after-free condition.
In function MessageLoopTaskQueues::GetNextTaskToRun:
1) Call is made to PeekNextTaskUnlocked(queue_id);. Returned value contains a reference to to an object of const DelayedTask& taken from an std::queue container as returned by primary_task_queue_.top().
2) Variable TaskSource::TopTask top now contains a reference to this object.
3) Function queue_entries_.at(top.task_queue_id)->task_source->PopTask(...) which in turn calls pop() method on std::queue.
4) Object of type DelayedTask on top of the queue gets deleted.
5) top.task.GetTaskSourceGrade() is called later with top.task refering to an already deleted object.
*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
Some android devices have only a single fast core. We set the threading affinity for UI/Raster to the fast core, which can lead to the UI/Raster being serialized on this thread. Instead, we should weaken /invert the affinity to "Not slow cores".
FIxes https://github.com/flutter/flutter/issues/153690
Customer money will see some benchmark regressions but they can deal.
We used to require this only on iOS because the standard library till iOS 9 didn't have support for this. We have moved past that version. No change on other platforms.
Fixes Impeller engine build with `clang-16` and GCC 14 C++Stdlib does not implicitly include `<algorithm>`.
Fixes issue: #150515
*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
Reverts flutter/engine#53136
b/345642546 shows some massive performance regressions on this patch. i'll rework it so the conditions are only loosened on phones with one fast core.
For devices with 1 fast core, the current affinity set is less idea because it forces UI and raster on the same thread. Widen this to exclude slow cores instead of including only the fast core.
Previously, because the suppression was local, every user of the the header had to manually add the flag. Now the flag will be propagated to the targets automatically. This linked issue still needs to be fixed but the fix can now be more isolated.
This wires up Android Hardware Buffer backed swapchains on platform that support
it (Android >= 29) with a KHR swapchain fallback (which can be removed later to
save on some binary size if needed).
Some salient features of these swapchains and their differences with the KHR
variant:
* All swapchain images are guaranteed to R8G8B8A8. This could potentially allow
for earlier creation of the context and the PSO libraries.
* All swapchain allocations are lazy. This should greatly reduce the time it
takes to create and resize a swapchain. However, swapchain image acquisition
may take longer for the first few frame where there is no pre-pooled image
available. Resizes should be similarly faster since the swapchain images for
the intermediate sizes are never created.
* Swapchain transients allocations (the attachments for the root render target)
are also lazily allocated.
* Swapchain images are pool and reused. The size of the pool is user specified
(currently 2). If an image in the pool ages past a user supplied value
(currently 1 second), it is collected as well. Applications that don't render
frames for a long period of time should see less memory use because of
swapchain image allocations.
* The present mode of AHB swapchains behave similar to KHR swapchains but with
VK_PRESENT_MODE_MAILBOX_KHR. In cases where there is no application managed
frame pipelining, this might cause images to never be presented if a newer
image is available. This wasted work can only be avoided by application
provided pipelining.
* There are no client side waits during image presentation. Instead, a new type
of fence is wired up that exports its state as a sync file descriptor. The
fence signal operation is enqueued on the client side and the buffer is set on
the surface control. The presentation engine then performs the wait.
* On Qualcomm devices, Chromium seems to be setting vendor specified flags for
opting the hardware buffers into using UBWC. AFAICT, this is similar to AFBC
(and NOT AFRC) on ARM Mali. This has not been wired up since I don't have a
Qualcomm device at the moment and cant verify bandwidth use using GPU
counters. I would also like to verify that UBWC is safe to use to images that
can be used as input attachments.
This reverts commit 037aa6b4caa6a3a726e67e519324b2c0a1ec274a.
The original cause of the revert was a flake introduced because the unit-test could request a frame from the Choreographer if it ran long enough. The availability checks in the choreographer were inaccurate after we intentionally backed out of using the Callback32 variant on 32 bit platforms.
The new tests didn't catch it because of an unrelated issue. In the first version of the patch for review, the proc table was only supposed to run on API levels 29 and above. When @dnfield requested we also get rid of the NDK helpers, the choreographer and additional utilities were added. But the API level gate in the new test harness wasn't removed. This made the tests be skipped. That gate has been removed entirely now. The error that cause the revert because of flakiness will now be a reliable failure.
Only available on Android device API levels >= 29. Proc table is setup has versioning checks. All handles are type safe. Collection of handles takes into account cleanup tasks (like reparenting surface controls). The proc table contains code duplicated in ndk_helpers and I will remove that in favor of this in a subsequent patch.
Part of https://github.com/flutter/engine/pull/51213 being chopped up.
### Motivation of the change:
Both dart and flutter are using fairly outdated gn-sdk without properly maintained. Currently @hjfreyer is working on version'ed IDK / SDK libs which requires changes in gn-sdk to use the right version of the libs in fuchsia/sdk/obj/{arch}-api-{level} rather than the one in the fuchsia/sdk/arch. But current implementation does not support choosing the right version.
### Blocking issue:
The new gn-sdk (in flutter/tools/fuchsia/gn-sdk) generates multiple BUILD.gn files rather than a large BUILD.gn the previous version created. So most of the build rules need to switch from the old `fidl:{api}` build rule to `fidl/{api}` rule. The same change will happen in the dart/sdk, i.e. http://go/dart-reviews/356924. But since the two repos cannot have one single atomic change, changing either side first will cause flutter to break. E.g. the linkage error caused by duplicated symbols will happen if we change the dart/sdk first, since in flutter, it will still refer to the old build rules in the middle.
### Solutions:
Ideally we can create redirect rules in the current `build/fuchsia` buildroot tree to redirect the old rules into the new one, so we can make the change in the flutter first then dart/sdk. But creating the rules is not trivial and will only be used once.
So an alternative solution is
- pause the dart/sdk -> flutter roll
- submit dart/sdk change (http://go/dart-reviews/356924)
- update this change to manually bring the dart/sdk change, namely the `dart_revision` in the DEPS file and signatures in the ci/licences.
- resume the dart/sdk -> flutter roll.
But it requires this change itself to be reviewed first, and I'd like to know your opinion before moving forward.
See corresponding dart/sdk change at http://go/dart-reviews/356924.
### //build/fuchsia/ from buildroot should be removed after this change.
Bug: [b/40935282](https://issues.chromium.org/issues/40935282?pli=1&authuser=0)
FYI: @hjfreyer
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
ImpellerC tests that use a temporary directory will append the current process ID to the directory name to avoid collisions.
The temporary directory will also be deleted after each test case completes.
See https://github.com/flutter/flutter/issues/143379
This PR changes the format check on CI to use the command added in
https://github.com/flutter/engine/pull/50747.
Additionally, while making this change, I noticed that the CI check was
not checking the formatting of all files, and that as a result, files
were present in the repo with incorrect formatting. I have fixed the
formatting and fixed the check to always check all files.
This is a fix forward alternative to the revert here: https://github.com/flutter/engine/pull/50581
If the revert lands first I'll rebase into this. I'm working on verifying this locally against the devicelab tests.
Adds more dynamic method lookups in service of https://github.com/flutter/flutter/issues/143105
Moves the TU out to FML so that Impeller can more easily use it.
Adds checking on `AHardwareBuffer_getId` so that it checks the return value before returning what is potentially garbage.
Adds some smoke tests to make sure these things actually work/look up meaningful symbols. Test is in the shell because we have testing infra for this kind of thing there.
Fixes https://github.com/flutter/flutter/issues/142488
- Only logs on iOS if Skia is used instead of Impeller.
- Logs on other platforms if Impeller is used instead of Skia.
- Uses "IMPORTANT" rather than "ERROR" for these logs. This will show up by default since flutter_tools sets ERROR and above as logs to show.
- Adds some tests.
- Makes INFO log print file paths the same as other verbosities.
The number of arguments are not used.
And also, even if we need it in the future, they can be derived at compile time:
```cpp
template <typename T>
struct function_traits;
template <typename Ret, typename... Args>
struct function_traits<Ret(Args...)>
{
using params = std::tuple<Args...>;
};
template <typename T>
constexpr std::size_t get_parameter_count() {
return std::tuple_size<typename function_traits<T>::params>::value;
}
template <typename T>
struct member_function_traits;
template <typename C, typename Ret, typename... Args>
struct member_function_traits<Ret(C::*)(Args...)>
{
using params = std::tuple<Args...>;
};
template <typename T>
constexpr std::size_t get_member_function_parameter_count() {
return std::tuple_size<typename member_function_traits<T>::params>::value;
}
```
(I got the code above with ChatGPT but I verified that they work)
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Flutter implements the UI isolates message via posting to a UI task queue. That task queue has a primary and a secondary queue. The
* primary is used for running tasks resulated to framework (e.g. draw frame)
* secondary is used to run Dart event loop messages (e.g. received data on a socket)
The Dart semantics requires running microtasks before processing the next event loop message.
The way flutter implements by attaching an observer to the secondary queue. Every time a task from the queue is run, all observers are run and one of them is going to run pending microtasks.
In some situations the engine pauses the dart event loop. The terminology used in the code is "microtask" when in reality what it means is "event loop".
=> This PR changes this terminology to reflect what actually happens.
This change migrates off of the old fuchsia logging apis to use the
structured logging apis. The initial FIDL connection is made during
global initialization (before main()) and the initial minimum log level
is queried from the system. Later on, once the main loop is initialized,
we setup an async task to listen for additional log interest changes
from the system. The advantage of doing this on the main loop is that we
avoid spawning an additional background thread in the process (the
legacy logging apis use the background thread approach).
One added benefit of this change is it reduces the size of the
dart/flutter runner far packages by about 250kb in release mode, because
libsyslog.so and libbackend_fuchsia_globals.so are no longer needed.
flutter/flutter#141924
## 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
The thread_local storage class was not available pre-C++11. Even when C++11 was available, the C++ runtime in versions of iOS <= 9.0 was not capable of supporting this storage class. So we ended up using pthread directly in that case. The unique pointer support was added later. Now that the storage class has been supported on all Flutter platforms for a while, we can remove the fallback and remove a bunch of code. The ThreadLocalUniquePtr can be removed too but that can be attempted in a separate migration.
