This reverts commit d5d8b5de90f40644f23d309c0f68d03e299334eb.
The flutter/tests/skp_generator test (running in flutter_tester on
Linux) was failing, along with some customer tests.
Places the binding data in a vector, since they key was only meaningful on metal but not used anywhere. I don't think that we need to specificially handle the case where our own contents bind the same contents multiple times, but interested to discuss if folks disagree.
If all previous elements were skipped due to clear color optimization,
then the clear color must be rendered as an input to the advanced blend
if framebuffer fetch is not available.
In https://g-issues.skia.org/issues/305780908 Skia is removing the
default SkFontMgr. Previous work consolidated all references to
txt/platform.h and this replaces those last references. I attempted to
mirror the existing functionality, which still responds to GN flags and
the target platform.
After this PR, Flutter should not be depending on the default fontmgr
(and the defines in flutter_defines.gni) will maintain that behavior
until the legacy functions/methods are deleted from Skia. There were a
few tests that I missed on an earlier PR which relied on the default
font (helper added in #47493). These tests were failing because they
were making some assertions related to TextBlobs, which didn't work if
the (now-empty) Typeface they loaded had no glyphs. Thus, I added a few
extra asserts to make sure these textblobs *had* glyphs which make the
failing tests less mysterious, should this issue crop up again.
I cleaned up Flutter's BUILD.gn file for Skia a bit, deleting unused
targets related to the font managers. This involved fixing an implicit
dependency from //third_party/glfw/ to `Gdi32.lib` on Windows.
## 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.
- [ ] 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
From local testing across both flutter galleries, wonderous, some other test apps, the only time this code successfully culls is during the stretch overscroll (and we cull 1 or so entries). The cost of this culling is approximately 20% of entity rendering time, and about 5% of overall raster time.
Support framebuffer fetch on devices that have the extension VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS which gives us a fairly easy way to add subpass self dependencies.
Part of https://github.com/flutter/flutter/issues/120223
Placing the vertex buffer on the binding object meant that we were actually paying 2x the size cost for it (one for vertex bindings, one for fragment bindings). By moving this object onto command itself, we reduce the size and avoid spliting up the command state in a weird way.
Also updates most of the contents to prefer moving the VertexBuffer.
A tiny tweak that should be better math since the sigma will scale based on the actual scale amount considering we have to scale down to discrete boundaries for the downsampled texture. I also turned on linear sampling while I was at it to try to calm down the jitters. I think it looks better at small sigmas.
test: This is covered by existing golden tests (when we turn this on).
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes https://github.com/flutter/flutter/issues/138819
Emoji glyphs normally ignore their text color (except for alpha channel). Text shadows are created by rendering the font with the shadow color as the text color and then applying a blur. If this is done to an emoji, we need to apply the text color so that the shadow has the correct color.
And make it illegal to do so in the future. Now, making such a mistake will lead to a linker error complaining about duplicate definitions of `kYouInstantiatedEntityTestMultipleTimes`.
Fixes https://github.com/flutter/flutter/issues/139090
From looking at profiles, we're always copying paths at least once when recording commands. By deleting the copy constructor, I cna ensure that we're always either moving or explicitly cloning the Path object.
Or, now that I fixed all the moves I could add the copy constructor back.
Towards https://github.com/flutter/flutter/issues/133417#issuecomment-1839273553
I'll request a test exemption for this as it's a semantics-only refactor.
There are still a bunch of other forward declares in Impeller that make things confusing for tooling, but some of those will require less trivial changes to interfaces.
Similar work as done for filled circles in https://github.com/flutter/engine/pull/48103, stroked circles can also be directly tessellated using the same internal data structures used for filled circles.
Fixes: https://github.com/flutter/flutter/issues/138877
The existing golden tests were already showing this bug, but we didn't see it. With this fix the size of the squares should be the same as the circle for the round caps in the DrawPoints playground goldens.
Test exemption request: This fix is covered by existing golden tests.
Note: The GPU tesselation for drawPoints is disabled in this test PR, but will be re-enabled when I can run some benchmarks to see where a good cutoff exists.
Tessellation for circular or quarter circular paths is now handled by a single tessellator class that provides services for DrawCircle, DrawPoints (round caps), and DrawLines (round caps). Creating a general path is avoided along with the associated overhead of recognizing and using a generic tessellation step.
This test worked by 1) releasing an item to be destructed on a background thead 2) adding a second item to this queue with a waitable event.
The idea being you could wait for the event and that would tell you when 1) was complete. Unfortunately these items are released by being placed into a std::vector, which destroyed in _reverse_ order. Somtimes when the test state was queried the work was done and sometimes it wasn't
To fix this, add a second waitable event that guarantees that both the first event and the original event have finished - because we do not add these event until the first is destructed and we do not add items to the destroy list while it is in progress.
The Impeller blur is scaling down about twice as fast as the Skia Blur. Instead adopt their approximate constant values, which results in less shaking as the blur gets bigger.
FYI @gaaclarke
Like command pools, descriptor pools can be reset on a background thread and reused to improve CPU efficiency. Unlike command pools, we create one per render pass, so the existing management of the lifecycle is handled via the Tracked objects and we need only adapt the easier parts of the command pool reset.
To make the caching more effective, we change descriptor pool allocation to round up to NPOT. this is so if we have a frame with varying numbers of draws : 33, 42, 16, 45, we still end up recycling and reusing the descriptor pool (its just a bit of extra memory).
Fixes https://github.com/flutter/flutter/issues/134968
This benchmark includes things like allocating geometry/contents, path conversions, et cetera. It doesn't include dispatching or GPU work, but should make it easier to see improvements/regressions in the efficiency of this code.
This extension only implies decal support for OES textures. Remove it from the check for generic decal support.
In practice i think it would be unlikely that a driver supports this and not regular decal, but that would only make the bugs this may cause even harder to track down.
First triangle, in the framework! 🎉
Adds shader libraries, pipelines, command buffers, render passes, etc.
* Light pipelines/shader objects. No optimization yet, pipeline warming
to come.
* "Dynamic" command style. Don't re-send bindings if you don't need to.
Essentially: https://github.com/flutter/flutter/issues/133179
* No need to explicitly encode passes.
* Minimal descriptor usage.
* Nothing is async, except for the optional command buffer completion
callback.
It took a bunch of experimenting to get here, but I think things are
starting to look pretty neat. :)
Todo:
* Land the shader bundle format/remove the testing hacks & fixtures that
piggyback off of the runtime effect system.
* Add remaining calls for blend config, clearing bindings, etc.
* Inconsistent error handling patterns that need cleanup.
* Maybe: Surface exceptions for validation errors.
* Handle the texture usage bitmask more elegantly.
Fixes https://github.com/flutter/flutter/issues/138373
When constructing overlay layers when there are platform views, make sure the same pixel format and color space as the main view is used.
Add validation to impeller HAL about blitting different pixel formats.
Commands are massive 500 byte objects, re-allocating this vector while recording them can actually add a substantial amount of overhead to applications with lots of drawing commands. Sizing to npot so that underestimating by a few commands doesn't force us to immediately copy all command objects.
This is still a herustic driven approach. An alternative exact approach would have entities/contents describe how many commands they would add. This may be more important for stencil then cover (depending on how we do it) since some contents would need to create two commands.
Some discussion here:
https://discord.com/channels/608014603317936148/1175215129135153202.
In short, `*<std::optional>` is [undefined
behavior](https://en.cppreference.com/w/cpp/utility/optional/operator*).
After talking to @bdero we are considering that this should never
happen.
I thought of a few different approaches here, none of them are great.
However given that this class is used quite minimally, this seems to
jive with similar patterns we're using in scene/scene_context.
---
Feel free to push back or suggest alternatives, mostly proposing this to
get brain juices flowing.
