Resolves https://github.com/flutter/flutter/issues/134748.
This was a really fun experiment. I learned a lot from it, and it
genuinely helped me solve some coverage-related problems, but the
reality is it was too little too late -- by the time we had this
capture system, we had already solved most of the problems that would
have benefitted from this.
It's been a few months since I've used or extended the capabilities of
this capture system for something, and I don't have the spare
time/energy to give it the love it needs to realize the vision I had
for it. I still almost exclusively use a combination of native frame
captures and print debugging to solve problems.
RIP in peace.
This reverts commit 104eb98e62. (https://github.com/flutter/engine/pull/52680)
The size of the LTO build of the engine with the dylib compressed is as follows:
```sh
$ du -k libFlutter*
5236 libFlutter.dylib.tar.gz
4324 libFlutterSlimpeller.dylib.tar.gz
```
Sizes are in KiB. This represents a binary size reduction of 17.41% of the compressed artifacts. The compression ratios will likely differ based on the compression scheme.
Uncompressed, the sizes are:
```sh
$ du -k libFlutter*
16920 libFlutter.dylib
14044 libFlutterSlimpeller.dylib
```
This represents a binary size reduction of 16.99% which is in the same ballpark.
The really mucky bit was backing out the raster cache and persistent cache. I want to clean that up in a later patch so that those TUs are part of a separate submodule.
Opting out of Impeller will lead to a fatal log at startup saying the opt-out is disallowed.
Fixes https://github.com/flutter/flutter/issues/126606
This doesn't yet add support for backdrop filters or emulated advanced blends, but will allow them to work if `surface_frame.framebuffer_info().supports_readback` is true.
part of https://github.com/flutter/flutter/issues/142054
Resolves https://github.com/flutter/flutter/issues/134748.
This was a really fun experiment. I learned a lot from it, and it genuinely helped me solve some coverage-related problems, but the reality is it was too little too late -- by the time we had this capture system, we had already solved most of the problems that would have benefitted from this.
It's been a few months since I've used or extended the capabilities of this capture system for something, and I don't have the spare time/energy to give it the love it needs to realize the vision I had for it. I still almost exclusively use a combination of native frame captures and print debugging to solve problems.
RIP in peace.
Canvas implementation that renders directly, requires ENABLE_EXPERIMENTAL_CANVAS to be set to true and only wired up for android/vulkan.
Currently missing:
* Clips
* backdrop filters
* opacity peephole
* More?
This will eventually allow us to delete the aiks layer and cut out most conversion costs. Part of https://github.com/flutter/flutter/issues/142054
Without this, the EmbedderExternalViewEmbedder's call to SurfaceFrame::Submit will render an empty display list that will overwrite the output rendered by the external view embedder's layers.
See https://github.com/flutter/flutter/issues/143387
Fixes https://github.com/flutter/flutter/issues/141351 (speculatively - I have not directly reproduced this in an application, but without this change the added test crashes with a segfault in the submit callback).
If the rasterizer gets torn down, the surface gets released and the submit callback may fire on a collected object. Capturing `this` isn't safe. I'm not quite sure how that could happen from the linked stack trace though, since the draw call and the teardown call should be happening on the raster thread, and if the surface was reset then the draw call should've failed earlier...
The added test causes a segfault without the change.
Reland of https://github.com/flutter/engine/pull/49505
---
part of https://github.com/flutter/flutter/issues/140804
We can't use the existing host buffer abstraction as that requires us to collect all allocations up front. By itself, this isn't sufficient for #140804 , because we'll need a way to mark ranges as dirty and/or flush if we don't have host coherent memory. But by itself this change should be beneficial as we'll create fewer device buffers and should do less allocation in general.
The size of the device buffers is 1024 Kb, somewhat arbitrarily chosen.
Reverts flutter/engine#49505
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
part of https://github.com/flutter/flutter/issues/140804
We can't use the existing host buffer abstraction as that requires us to collect all allocations up front. By itself, this isn't sufficient for #140804 , because we'll need a way to mark ranges as dirty and/or flush if we don't have host coherent memory. But by itself this change should be beneficial as we'll create fewer device buffers and should do less allocation in general.
The size of the device buffers is 1024 Kb, somewhat arbitrarily chosen.
part of https://github.com/flutter/flutter/issues/140804
We can't use the existing host buffer abstraction as that requires us to collect all allocations up front. By itself, this isn't sufficient for #140804 , because we'll need a way to mark ranges as dirty and/or flush if we don't have host coherent memory. But by itself this change should be beneficial as we'll create fewer device buffers and should do less allocation in general.
The size of the device buffers is 1024 Kb, somewhat arbitrarily chosen.
This is more-or-less a revert of https://github.com/flutter/engine/pull/14011
This code never ended up being used outside of tests, and it's not how we handle asset loading at this point anyway.
I was hopeful we could kill off all runtime dependencies on Dart in `FML` when looking at this, but it looks like trace_event.h still wants to import dart_api_tools.h for some Dart enum types. This may or may not matter if we ever want to build FML for web/wasm. /cc @eyebrowsoffire. If we really need to do that, we can refactor the trace event stuff so that it has a web and Dart implementation that's selected at build time.
The new `BUILD.gn` files in the Engine tree can't go under
`build/secondary` because Skia still has its own, and they'd be selected
first. So, this PR puts the new `BUILD.gn` files under `flutter/skia`.
This patch does the following:
- Updates `flutter_tester` to set up an Impeller rendering context and surface if `--enable-impeller` is set to true, using the Vulkan backend with Swiftshader.
- Updates `run_tests.py` to run all tests except the smoke test (that one really has no rendering impact whatsoever) with and without `--enable-impeller`.
- Updates a few tests to work that were trivial:
- A couple tests needed updated goldens for very minor rendering differences. Filed https://github.com/flutter/flutter/issues/135684 to track using Skia gold for this instead.
- Disabled SKP screenshotting if Impeller is enabled, and updated the test checking that to verify an error is thrown if an SKP is requested.
- The Dart GPU based test now asserts that the gpu context is available if Impeller is enabled, and does not deadlock if run in a single threaded mode.
- We were missing some trace events around `Canvas::SaveLayer` for Impeller as compared to Skia.
- A couple other tests had strict checks about exception messages that are slightly different between Skia and Impeller.
- I've filed bugs for other tests that may require a little more work, and skipped them for now. For FragmentProgram on Vulkan I reused an existing bug.
This is part of my attempt to address https://github.com/flutter/flutter/issues/135693, although @chinmaygarde and I had slightly different ideas about how to do this.
The goals here are:
- Run the Dart unit tests we already have with Impeller enabled.
- Enable running more of the framework tests (including gold tests) with Impeller enabled.
- Run all of these tests via public `dart:ui` API rather than mucking around in C++ internals in the engine.
# Description
This PR fixes the `gl_populate_existing_damage` in embedder.cc, which currently returns an empty rectangle when a full repaint is needed. This leads to the `frame_damage` being considered empty in rasterizer.cc, causing incorrect partial repaint within Flutter when `gl_populate_existing_damage` is not provided by the embedder.
# Related Issue
https://github.com/flutter/flutter/issues/119601
# Tests
Add a new test. Also fixes damage calculation related tests in EmbedderTest by
* Use a new Dart embedder fixture entry point `render_gradient_retained` which retains the old layer to make sure layer tree diff happens.
* Add `latch.Wait()` after `SetGLPresentCallback` to make sure assertions have been executed.
* Make `existing_damage_rects` static since it should be valid after `populate_existing_damage` returns.
In https://skia-review.googlesource.com/c/skia/+/742797 Skia refactored
GrBackend* to not require #ifdefs. This changes
callsites in Flutter to use static functions instead of methods that
were conditionally compiled on those classes.
There should be no functional change.
Relands https://github.com/flutter/engine/pull/45131
Fixes https://github.com/flutter/flutter/issues/132416
Differences from last time:
- Some minor merge conflict fixes
- Use the RTree to get the bounds instead of recalculating the bounds
- Make the iOS platform view controller implementation use the impeller-aware slices instead of the display list ones. This has been fixed for Android and the desktop embedding, but I missed iOS. The unit tests weren't actually running before I branched for my PR, @zanderso fixed them up separately and this resulted in catching the failures on post submit last time.
Reverts flutter/engine#45131
This is failing the Impeller variants of the unobstructed platform views tests:
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac%20Production%20Engine%20Drone/132249/overview
```
Failing tests:
-[UnobstructedPlatformViewTests testPlatformViewsMaxOverlays]
-[UnobstructedPlatformViewTests testOneOverlay]
-[UnobstructedPlatformViewTests testOneOverlayPartialIntersection]
-[UnobstructedPlatformViewTests testTwoIntersectingOverlays]
-[UnobstructedPlatformViewTests testOneOverlayAndTwoIntersectingOverlays]
** TEST FAILED **
```
We've retried it a few times so I suspect this isn't a flake.
This is a reland of https://github.com/flutter/engine/pull/44248
Fixes https://github.com/flutter/flutter/issues/132416
Changes from last time:
- The `drawPoints` benchmark was failing to render anything meaningful because of an error in `AiksLayer` that resulted in an infinitely sized bounding rectangle poisoning the bounds with `NaN` (this happens, for example, with a `drawPaint` call, which that benchmark happens to use). Added a test covering this and filed https://github.com/flutter/flutter/issues/132770 to explore ways to avoid this in the future.
- There was a bug in `DlAiksCanvas::SaveLayer` where a `nullptr` `paint` but non-`nullptr` `backdrop` was failing to actually save the layer. This resulted in incorrect rendering.
- There was a bug in `impeller::Canvas::DrawPicture` that resulted in incorrect stencil depth counting. That was fixed separately by @bdero, but was the cause of incorrect rendering in some Wonderous screens.
- I've added a simple implementation for `AiksLayer::IsReplacing`. It does not currently compare as deeply as the `DisplayListLayer` version potentially does, but it is good enough to avoid the regression noted in https://github.com/flutter/flutter/issues/132071. That regression was on a benchmark that greatly benefits from partial repaint. With the new implementation, it still gains partial repaint where it previously did not. There is more work that can be done here, filed https://github.com/flutter/flutter/issues/133361 to track that work.
I merged but did not fully integrate the `DisplayListBuilder`/`CanvasToReceiver` work that @flar has done. I have a local experiment with that, but would prefer to see this land and run through the device lab so we get some better comparison numbers for which one performs better.
Resolves https://github.com/flutter/flutter/issues/130947.
- Allow for setting the text renderer (`TextRenderContext`) when the
`AiksContext` is built. Previously, the text renderer was selected at
build time through linking a `TextRenderContext::Create` symbol.
- Support using Impeller without a text backend. Throw a clear error
when trying to render text if no text backend is present.
- Don't track the Impeller context in `TextRenderContext`. Just let the
renderer supply its context when generating atlases.
- Allow for overriding the `TextRenderContext` for individual Aiks
playground tests/goldens.
Fixes https://github.com/flutter/flutter/issues/130141
The primary goal of this patch is to move dispatching of `dart:ui` `Canvas` commands to the UI thread.
Before this patch, the architecture is something like:
## UI Thread
- `dart:ui` talks to `DisplayListBuilder`, a `DlCanvas` implementation.
- `DisplayListBuilder` does some clip/bounds tracking and creates a `DisplayList` object that is held by `dart:ui`'s `Picture` objects.
- `DisplayList`s are added to `DisplayListLayer`s in `flow`.
## Raster Thread
- `flow` flattens the various operations into a single `DisplayList` via another `DisplayListBuilder`.
- A `DlOpReceiver`implementation converts that `DisplayList` into an `Aiks` `Canvas`/`Picture`.
After this patch, the architecture instead looks like:
## UI Thread
- No change for Skia.
- If Impeller, use a new `DlCanvasImplementation` that talks to `Aiks`'s `Canvas`.
- If Impeller, `dart:ui` Picture's now hold an `Aiks` `Picture`, which get shared into `AiksLayer`s in `flow`.
## Raster thread
- No change for Skia, but some light refactoring for places that assumed a `DisplayListBuilder` where they really just needed a `DlCanvas`.
- The `Aiks` `Picture`s are combined using new API on `DlCanvas` and still backed by `Aiks`.
These changes show significant improvement on raster times on Android and only very small regressions on UI times in local testing, see https://gist.github.com/dnfield/26528090194c9f5abdbac13cdcbf4f79 for old gallery transition perf numbers.
Many of the other changes in this patch are related to the following:
- Making `DlRTree` usable for Impeller.
- It would be nice to have a version of DlRTree that speaks `impeller::Rect`.
- Creating the requisite classes to support `EmbeddedViews` so that Desktop works.
This patch does not remove the `impeller::DlDispatcher`, which now would only be used in tests.
In https://github.com/flutter/engine/pull/41059 the Android context and surface were changed to share one impeller::ContextVK across all instances of AndroidSurfaceVulkanImpeller. However, the ContextVK holds a reference to the current swapchain. If an app uses multiple AndroidSurfaceVulkanImpeller instances (e.g. for platform views), then one surface may overwrite the ContextVK's swapchain while another surface is trying to render.
This patch allows each surface to get its own impeller::Context instance holding a swapchain tied to that surface. The per-surface contexts will delegate most operations to the shared parent ContextVK.
Actually works now, though there is some issue with the default fbo stencil so I've filled https://github.com/flutter/flutter/issues/130048
Other issues:
* ~~Rendering looks wrong~~
* ~~Resizing window hangs~~
* ~~Reactor isn't set up correctly and all blit passes are currently failing.~~
* ~~Needs to handle falling back to sample count of 1 like we do on Android~~.
