[Impeller] Enable depth buffer clipping & Stencil-then-Cover path rendering.

[bdero]

Turn the page, wash your hands.

Addresses the following issues:

• [Impeller] ☂️ Path drawing performance needs significant improvement. flutter#143077
• [Impeller] Depth/Stencil changes for improved rendering performance of complex geometry. flutter#137714
• [Impeller] Perform clipping on the depth buffer. flutter#138460
• [Impeller] Why did Impeller opt for "triangulation then draw" over "stencil then cover"? flutter#123671
• [Impeller] performance issue on specific lottie files. flutter#141961
• [Impeller] Tessellation overhead causes regression on some animations. flutter#134432

[bdero]

This test was fragile so I tried to encode its intent better rather than adding more branches.

[jonahwilliams]

SGTM

[bdero]

I had to rework this StC logic a bit because it wasn't handling Positions+UVs right. I was inadvertently stenciling with position+UV geometry (bad) and then covering using a position-only rectangle (also bad).

This is fixed now. When we need to use StC, we only generate positions for the stencil prep draw and then draw the cover rectangle with or without UVs as necessary.

[chinmaygarde]

I have a preference towards separating out this rework and the flag-flip of the stencil-then-cover into separate PRs.

[bdero]

Agreed, I pulled out these changes into this PR: #50900

[chinmaygarde]

Can you separate just the flag flip into another PR? Otherwise lgtm.

[chinmaygarde]

lgtm after #50900 lands. So a lot of this go away with just the flag flip remaining. Thanks!

[jonahwilliams]

LGTM (but not to CI)

[jonahwilliams]

The failing test: "Canvas.drawParagraph " IIRC is using Picture.toImage and the Vulkan swiftshader backend, fwiw

[bdero]

There's something funky going on here. On linux_host_engine, Canvas.drawParagraph renders tab as space instead of tofu passed on debug but not profile or release: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20linux_host_engine/27617/overview

[bdero]

Okay, so it only ever fails when flutter_tester is run with --enable-impeller and --force-multithreading. The --enable-impeller invocation without --force-multithreading doesn't fail.

[bdero]

In total I've seen these tests failing on and off so far:

• toImage and toImageSync have identical contents
• Canvas.drawParagraph renders tab as space instead of tofu
• Canvas preserves perspective data in Matrix4

All of these are doing toImage + toByteData and verifying pixel data.

On macos, I can only repro the toImage and toImageSync have identical contents failure. I ran canvas_test.dart > 100 times without --force-multithreading and it never failed. But when I enable --force-multithreading, it fails around half of the time.

[bdero]

I started dumping the result images before performing the failing ByteData comparison. Formatting the byteData as a PNG drastically reduced the error rate and it took more than 200 runs to collect three failure samples, all of which are identical (MD5 96f4201864c564a731a252100ef6c764):

toImageSync:

toImage:

Close-up in photoshop of the failed toImage result:

[bdero]

If I add sleep(const Duration(milliseconds: 300)) just before return (await resultImage.toByteData())! in toImage and toImageSync have identical contents, then Canvas.drawParagraph renders tab as space instead of tofu starts failing every time.

Peculiar! Maybe an underlying allocation is being prematurely freed or reused.

[bdero]

If I disable the DeviceBuffer VMA pool by commenting out these lines the buffers stop getting corrupted:

engine/impeller/renderer/backend/vulkan/allocator_vk.cc

Lines 459 to 461 in 1d7d5e6

	if (created_buffer_pool_ && desc.storage_mode == StorageMode::kHostVisible) {
	allocation_info.pool = staging_buffer_pool_.get().pool;
	}

[jonahwilliams]

I'm not sure how the multi threaded test env. Works, but it's possible that it is breaking some contracts about thread safety of the contexts. You might try backing out Aaron's change to reuse the context objects in the test environment

[jonahwilliams]

Oh wait, but then that wouldn't explain tester changes.

[jonahwilliams]

Well if it makes it work, id say delete the VMAPool. But I'm a bit suspicious that something in the flutter tester integration is busted

[jonahwilliams]

I'm sort of wondering if there is a problem with the readback implementation in image_encoding_impeller.cc -> ConvertBufferToSkImage.

We blit the render target texture into a device buffer and then readback that device buffer. Potentially the device buffer is getting released too early, or we're missing a memory barrier for the readback. I'm not sure if the fence waiter firing guarantees that the buffer is fully populated.

[jonahwilliams]

I opened two PRs with adjustments to buffer usage/readback to see if those pass/fail

[jonahwilliams]

I think the "do everything plus barrier" pr is working but is hitting a GLES exception

[jonahwilliams]

You gotta stop force pushing!

[jonahwilliams]

LGTM, please land a non-force pushed commit so we can verify gold.

[flutter-dashboard]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #50856 at sha daef206

[bdero]

please land a non-force pushed commit so we can verify gold.

My understanding is that this bug is limited to the flutter-engine-gold.skia.org interface. The flutter-gold test won't succeed if there are golden differences.

[jonahwilliams]

these goldens look unexpected. Did you need the change from #51101 ?

[bdero]

Maybe. Let's try just the picture.cc attachment change.

[bdero]

Yup that seems to work. Want to land your patch @jonahwilliams? The brand new blur style is broken with StC so now I'm trying to chase that down now.

[flutter-dashboard]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #50856 at sha 1fb0d41

[bdero]

I think the behavior of Inner and Outer is reversed at ToT and enabling StC/switching to the new clip system is just fixing it:

[jonahwilliams]

LGTM