Previously, FlutterViewController.engine was declared as a weak readonly property, but we explicitly declared the `FlutterEngine* _engine` ivar as a strong reference in the implementation. This changes the property declaration to be strong and eliminates the now unnecessary ivar.
There is also no semantic change to FlutterViewController itself, since the `_engine` ivar had been manually declared as a strong reference.
There is no semantic change for users of FlutterViewController.engine since whether a user acquires a strong or weak reference to the engine is determined by whether they declare the pointer to which they assign it as strong or weak.
This also eliminates the need for the `engine` getter, which was only present to prevent a warning that the strong ivar didn't match the weak property declaration.
No changes to tests since this introduces 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
Three changes related to string constants:
1. Uses the kIOSurfaceColorSpace constant, which is a CFStringRef pointing to the string "IOSurfaceColorSpace"
2. Uses the kCVPixelFormatType_32BGRA enum value from the CoreVideo headers (which is equal to 'BGRA') in place of hardcoding the string.
From the headers:
```
kCVPixelFormatType_32BGRA = 'BGRA', /* 32 bit BGRA */
```
3. Declares kIOServicePlane as a `constexpr const char*` rather than `static const char*`, this ensures only a single instance is created, rather than one per translation unit into which the header is included. This string is part of IOKit, but see the comment at the top of the header as to why it's apparently needed.
No test changes since there are no semantic changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
FlutterEngineGroup keeps an array of all live FlutterEngine instances it has created such that after the first engine, it can spawn further engines using the first.
Previously we manually managed this array by adding engines to it upon creation, then having [FlutterEngine dealloc] emit a notification such that FlutterEngineGroup can listen for it, and remove instances from the array upon reception of the notification.
Instead, we now use an NSPointerArray of of weak pointers such that pointers are automatically nilled out by ARC after the last strong reference is collected. This eliminates the need for the manual notification during dealloc.
Unfortunately, NSPointerArray is "clever" and assumes that if the array has not been mutated to store a nil pointer since its last compact call, it must not contain a nil pointer and can thus skip compaction. Under ARC, weak pointers are automatically nilled out when the last strong reference is released, so the above heuristic is no longer valid. We work around the issue by storing a nil pointer before calling compact.
See http://www.openradar.me/15396578 for the radar tracking this bug.
I'm not thrilled with the fact that we're replacing one sort of TODO with another, but the code is much simpler and avoids relying on a trip through the notification center, which seems objectively better.
Issue: https://github.com/flutter/flutter/issues/155943
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Spot another clang-tidy linter failure from: https://github.com/flutter/engine/pull/56631
In release mode, if we remove NSAssert, then weakFlutterEngine is not used at all. This should have been an XCTAssert rather than NSAssert in the first place.
```
â Failures for clang-tidy on /Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: error: Value stored to 'weakFlutterEngine' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
239 | weakFlutterEngine = flutterEngine;
| ^ ~~~~~~~~~~~~~
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: note: Value stored to 'weakFlutterEngine' is never read
239 | weakFlutterEngine = flutterEngine;
| ^ ~~~~~~~~~~~~~
Suppressed 9240 warnings (9111 in non-user code, 129 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
```
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/flutter/flutter/issues/157837
*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
SetRenderTargetType is used to configure the backingstore producer on the compositor, but the backingstore types available to any given compositor are limited to the specific graphics backend in use: Software, GL, Metal, or Vulkan.
This moves SetRenderTargetType to EmbedderTestCompositor and its subclasses and adds RenderTargetType validation. A follow-up patch will refactor EmbedderTestBackingStoreProducer into backend-specific subclasses.
For OpenGL backingstore producers, the egl_context_ from EmbedderTestContext (which is actually set in the EmbedderTestContextGL subclass and should live there, but that's a separate cleanup) is required in SetRenderTargetType, so we now inject it into EmbedderTestCompositor in the constructor so it's available when needed.
Issue: https://github.com/flutter/flutter/issues/158998
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Currently, flutter keeps calling MaybeRunInitialVsyncCallback() until 1
OnNextFrameBegin() called from Fuchsia which maybe problem when display
is off.
Tested manually.
Bug: http://fxbug.dev/376079469
## 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.
Threads that add operations to the ReactorGLES assume that those operations will be executed serially.
But prior to this change, the ReactorGLES added all operations into one queue. The reactor would then execute those operations on any thread that can react. This could cause operations that were added to the reactor on the raster thread to be submitted to the GPU on the IO thread (or vice versa).
The reactor does not wait for the GPU to finish execution of those operations. So other operations added on the raster thread could be submitted by a reaction before the GPU has completed the operation that was submitted on the IO thread.
This PR ensures that operations added to the reactor on a given thread will be executed during a reaction on that same thread. If the thread can not currently react, then the operations will be queued until the thread enables reactions.
This also adds a call to CommandBuffer::WaitUntilScheduled to ImageDecoderImpeller. This ensures that the command buffer submitted on the IO thread is flushed before the image is returned.
Fixes https://github.com/flutter/flutter/issues/158535
Fixes https://github.com/flutter/flutter/issues/158388
Fixes https://github.com/flutter/flutter/issues/158390
The Metal context/surface code was only in the `flutter` namespace but should have been in `flutter::testing` for consistency with everything else in the `testing` directory.
Also squashes the declarations in the rest of that directory to match the style guide while I've got the macro still handy.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Impeller is resilient to OpenGL state being trampled upon when accessing the GL context. But the embedder may not necessarily be. Ideally, we'd be using saving the state and restoring it. But that might be too involved. For now, this sets the GL state to a sane "clean" state.
We could, in theory, do this after each render pass but that unnecessarily increases API traffic. For now, I have added it at the transition of the embedder boundary.
Extracts backend-specific code in EmbedderConfigBuilder to separate translation units. In particular, this allows for less conditional header includes, and more specifically, for code relating to the Metal backend to include headers that include Objective-C types -- today we cast these all to void* to avoid declaring them in headers, which requires special handling for ARC.
An alternative approach would have been to extract backend-specific subclasses, but there are test suites such as EmbedderTestMultiBackend that are executed against multiple backends, which currently make that approach impractical, though that should likely be the long-term goal.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Since Metal code frequently uses Objective-C types like id<MTLTexture>
etc. this migrates Metal translation units to Obj-C++. In particular,
this allows transitively including headers that include such types. This
in turn helps with refactoring `EmbedderTest`, `TestMetalContext`,
`TestMetalSurface` to avoid specifying `void*` types in headers and
manually refcounting via ARC bridge casts.
Reformats the source, since Objective-C files are linted to 100 columns
rather than the 80 column limit we impose on C++ files.
This change introduces no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
Years ago, @chinmaygarde created TestMetalSurface and wrote a unit test
for it in 2866095b584bbdc7d9b24076fcb9d07716d673c9.
And some things that should not have been forgotten were lost. History
became legend. Legend became myth. And for four and a half years, the
test passed out of all knowledge. Until, when chance came, it ensnared a
new bearer. The test came to the creature @cbracken, who took it deep
into the tunnels of the GN build files. And there it consumed him. The
test brought to @cbracken unnatural long life. For 500 hours, it
poisoned his mind. And in the gloom of @cbracken's cave, it waited.
Darkness crept back into the forests of the world. Rumour grew of a
shadow in the East - whispers of a nameless fear. And the Test of Unit
perceived its time had now come.
