The pattern is similar to the recently added WASM (and previously NACL)
toolchains.
The major change is the engines use of functions that became constexpr
in C++26 (such as
[sqrt](https://en.cppreference.com/w/cpp/numeric/math/sqrt)) as if they
were already constexpr. I have attempted to keep the linkage the same
and am investigating how to enable this form of checking with our own
toolchain.
This includes a fix for a race seen in
EmbedderTest.PlatformThreadIsolatesWithCustomPlatformTaskRunner
The implementaion of MergedPlatformUIThread::kMergeAfterLaunch required
changing the interface of the TaskObserverAdd/TaskObserverRemove
callbacks so that TaskObserverAdd returned the TaskQueueId where the
observer was added. That TaskQueueId would later be given to
TaskObserverRemove.
The original implementation of this PR updated the embedder library's
implementation of TaskObserverAdd to return TaskQueueId::kInvalid to
signal that the observer was not added. However, this conflicted with
the embedder's EmbedderTaskRunner, whose implementation of
GetTaskQueueId returns TaskQueueId::kInvalid as a placeholder.
This PR reverts the embedder's TaskObserverAdd/TaskObserverRemove to the
original implementation which adds the observer to the current thread's
message loop and does not call GetTaskQueueId.
See https://github.com/flutter/flutter/issues/167418
If settings.merged_platform_ui_thread is set to kMergeAfterLaunch, then
the engine will be started on the UI thread. After engine setup
completes and the Dart isolate is loaded, the UI task runner will be
merged into the platform thread and all future Dart execution will run
on the platform thread.
This makes it possible for other work to run on the platform thread
while the engine starts.
See https://github.com/flutter/flutter/issues/163064
Changes required for this roll:
* addition of LLVM libc (which is now a dependency of libcxx)
* updates the to license script
* patching the verify_exported check to allow some symbols added by
libcxx
* annotations for clang-tidy warnings
Asking if the logs should be emitted and killing the process (say on
unreachable statements) wasn't constexpr. However we managed to use
these in constexpr contexts. As I understand, this was because of
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2448r2.html
which is available in C++23. This technique makes the methods constexpr
safe in C++17.
This was likely a holdover from a time when trace events were routed to
the timeline directly.
This was causing standalone QNX builds to also pull in Dart.
No change in functionality and hopefully faster builds for smaller
targets.
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.
