You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This sounds related to #194314 which was reverted previously for similar problems. @andywolff can you take a look please?
Gemini's evaluation:
• Over the last ~200 builds, every action_sheet_test.dart failure (press-drag and pressed) is on or after f5cd205, the
commit right after 730d1b2 (first failure 2026-10-21 23:52Z). The ~10 days before that are clean.
• All 28 failures in that window were on M1 bots running Mac-26.6.2. Most builds run on that config, though, so the bot
correlation is weak evidence.
• The original version of this PR (#191096#191096) caused the same press-drag flake.
justinmc bisected it to that PR in #192992#192992 and it was reverted. Andy suspected
RenderTargetCache reuse between tests in the same process; I'm not sure that was ever proven.
• The action sheet is exactly the pattern this PR changes: ClipRSuperellipse → BackdropFilter(blur) → translucent children
(dialog.dart:1309-1326).
Looks like yes, my square corners fix is the cause for this. I'll revert.
So, I relanded that PR after landing #193842 to clear RenderTargetCache between tests. It seems that did not fix the whole problem. I'll reassess whether we should revert it or keep it.
The current issue appears to be a test order dependency related to opacity behavior. My PR was focused on the clip behavior, but SaveLayer / Restore also handles opacity, so it makes sense that it could have changed behavior there. I suspect this is a true defect in my fix, I'll address it before trying to re-land
Upon further investigation, I think I found what was wrong with the square corners fix which caused the alpha problem.
In packages/flutter/test/cupertino/action_sheet_test.dart (cupertinoActionSheet.pressed.png and cupertinoActionSheet.press-drag.png), the expected alpha is 1.0 on the opaque button labels and ~0.78 on the translucent action sheet background. In the flakes, RGB stayed unchanged, while the alpha of affected pixels was scaled by one of a few discrete quarters (0.75x, 0.50x, or 0.25x of its expected value). In some flakes the entire sheet was scaled almost uniformly (as in the first failing image below at 0.50x alpha), while in others different regions of the sheet had different quarter multiples matching shapes rendered by earlier tests in the shard (as in the second failing image below, where the top title section is at 0.25x alpha and the lower buttons section is at 0.50x alpha). These discrete alpha values were a big hint pointing towards a problem with 4x MSAA.
Passing image
Example failing images
Digging into this, I found that when 'Action buttons shows pressed color as soon as the pointer is down' (and 'Taps on a button can be slided to other buttons') calls matchesGoldenFile, it captures a RepaintBoundary around the CupertinoActionSheet and Impeller performs 4x MSAA on it. Specifically, DisplayListToTexture creates an offscreen 4x MSAA snapshot RenderTarget (CreateOffscreenMSAA -> render_target_). Using 4x MSAA here is expected because Impeller relies on hardware 4x MSAA to antialias paths and rounded superellipse clips (ClipRSuperellipse).
The test flakes because there are uncleared depth buffers during the final 4x MSAA composite onto render_target_. Whenever one or more of a pixel's 4 MSAA subsamples contains any stale depth value greater than kDepthEpsilon, that subsample fails the depth test and remains fully transparent. The more subsamples fail and resolve as transparent, the more transparent the pixel becomes, scaling its alpha by exact quarters (0.75x, 0.50x, or 0.25x) as we saw in the CI flakes. It would be 0x if all four subsamples fail, though we didn't see this happen in any of the flakes.
The original square corners fix caused these stale depth values because it added a && is_onscreen_ condition in Canvas::SaveLayer.
I added this to make unit testing easier. CreateTestCanvas in canvas_unittests.cc defaulted to is_onscreen = false with a render target texture that did not always enable kShaderRead, so gating should_use_onscreen on is_onscreen_ let unit tests opt into or out of the root-target flip path explicitly. However, I don't need to do that, because I can instead write tests which call DisplayListToTexture directly to cover both root-pass flips where should_use_onscreen == true and nested SaveLayer subpass flips where should_use_onscreen == false.
The problem with this new conditional is that we actually want should_use_onscreen to be true even when is_onscreen_ is false. I was tripped up by misleading naming in Canvas. "onscreen" in should_use_onscreen and BlitToOnscreen actually refers to render_target_, the root destination RenderTarget passed to Canvas. Our snapshots are "offscreen" because Picture.toImage / OffsetLayer.toImage rasterizes into a standalone GPU texture in memory rather than an OS window surface, so DisplayListToTexture sets is_onscreen = false. This tells EndReplay() not to call Context::SubmitOnscreen to present a window drawable. For CupertinoActionSheet snapshots within DisplayListToTexture, we still want should_use_onscreen to be true so that FlipBackdrop renders directly into render_target_ instead of rendering into a temporary texture and copying to render_target_ afterward. It might make sense in a follow-up to rename should_use_onscreen and BlitToOnscreen to refer to the root target rather than "onscreen".
Here is how should_use_onscreen changes the route DisplayListToTexture takes. Because CupertinoActionSheet has a root BackdropFilter, Canvas starts with requires_readback_ = true and initially records into a temporary RenderTargetCache pass in Canvas::SetupRenderPass. When Canvas::SaveLayer reaches that BackdropFilter, it calls Canvas::FlipBackdrop, which branches on should_use_onscreen:
if (should_use_onscreen) { // Previously evaluated to true// ...
requires_readback_ = false; // Setting this to false allows Canvas::EndReplay to behave properly later
} else {
// requires_readback_ stays true, which gives us undesired behavior in Canvas::EndReplay -> BlitToOnscreen// ...
}
RenderPass& current_render_pass =
*render_passes_.back().GetInlinePassContext()->GetRenderPass();
if (SupportsBlitToOnscreen()) {
// Only true on Metal
} else {
// Creates a new renderPass but doesn't override depth attachment's `kDefaultStencilConfig`, which has `LoadAction::kDontCare`auto render_pass = command_buffer->CreateRenderPass(render_target_);
// ...
}
Creating a new render pass with a depth attachment of kDefaultStencilConfig, which has LoadAction::kDontCare, explains why the depth buffer isn't getting cleared by the load action.
But why didn't it get cleared when we purged RenderTargetCache between tests in PR #193497? Because DisplayListToTexture's render_target_ attachments aren't allocated from RenderTargetCache—they are local to DisplayListToTexture / Canvas and allocated directly through AllocatorVK (vmaCreateImage). Even before #193497, those transient 4x MSAA depth textures were already being destroyed at the end of each snapshot (vmaDestroyImage once the command buffer completed). However, in flutter_tester, a single ContextVK and VmaAllocator live for the entire process across all tests in the file. VMA retains its underlying VkDeviceMemory blocks and allocates the same memory ranges to subsequent tests without ever zeroing freed memory. In Vulkan, clearing attachment memory is solely the job of LoadAction::kClear.
This means that #193497 probably didn't actually do anything relevant. I'll revert it.
In order to fix the square corners issue without introducing this missing depth buffer clear, I just need to keep the good part of the change (improvement to clip stack popping behavior in EntityPassClipStack::RecordRestore) and test it differently without adding the is_onscreen_ condition. I'll work on that in #195716
Activity
jtmcdole commentedon Oct 27, 2026
@gaaclarke - can you find someone to investigate?
gaaclarke commentedon Oct 27, 2026
It almost seems like an alpha issue since everything seems like it might be blended with the white background?
gaaclarke commentedon Oct 27, 2026
This sounds related to #194314 which was reverted previously for similar problems. @andywolff can you take a look please?
Gemini's evaluation:
andywolff commentedon Oct 27, 2026
Looks like yes, my square corners fix is the cause for this. I'll revert.
So, I relanded that PR after landing #193842 to clear RenderTargetCache between tests. It seems that did not fix the whole problem. I'll reassess whether we should revert it or keep it.
The current issue appears to be a test order dependency related to opacity behavior. My PR was focused on the clip behavior, but SaveLayer / Restore also handles opacity, so it makes sense that it could have changed behavior there. I suspect this is a true defect in my fix, I'll address it before trying to re-land
Revert: Reland square corners fix (flutter#195436)
Revert: Reland square corners fix (flutter#195436)
andywolff commentedon Oct 30, 2026
Upon further investigation, I think I found what was wrong with the square corners fix which caused the alpha problem.
In
packages/flutter/test/cupertino/action_sheet_test.dart(cupertinoActionSheet.pressed.pngandcupertinoActionSheet.press-drag.png), the expected alpha is1.0on the opaque button labels and~0.78on the translucent action sheet background. In the flakes, RGB stayed unchanged, while the alpha of affected pixels was scaled by one of a few discrete quarters (0.75x,0.50x, or0.25xof its expected value). In some flakes the entire sheet was scaled almost uniformly (as in the first failing image below at0.50xalpha), while in others different regions of the sheet had different quarter multiples matching shapes rendered by earlier tests in the shard (as in the second failing image below, where the top title section is at0.25xalpha and the lower buttons section is at0.50xalpha). These discrete alpha values were a big hint pointing towards a problem with 4x MSAA.Passing image
Example failing images
Digging into this, I found that when
'Action buttons shows pressed color as soon as the pointer is down'(and'Taps on a button can be slided to other buttons') callsmatchesGoldenFile, it captures aRepaintBoundaryaround theCupertinoActionSheetand Impeller performs 4x MSAA on it. Specifically,DisplayListToTexturecreates an offscreen 4x MSAA snapshotRenderTarget(CreateOffscreenMSAA->render_target_). Using 4x MSAA here is expected because Impeller relies on hardware 4x MSAA to antialias paths and rounded superellipse clips (ClipRSuperellipse).The test flakes because there are uncleared depth buffers during the final 4x MSAA composite onto
render_target_. Whenever one or more of a pixel's 4 MSAA subsamples contains any stale depth value greater thankDepthEpsilon, that subsample fails the depth test and remains fully transparent. The more subsamples fail and resolve as transparent, the more transparent the pixel becomes, scaling its alpha by exact quarters (0.75x,0.50x, or0.25x) as we saw in the CI flakes. It would be0xif all four subsamples fail, though we didn't see this happen in any of the flakes.The original square corners fix caused these stale depth values because it added a
&& is_onscreen_condition inCanvas::SaveLayer.flutter/engine/src/flutter/impeller/display_list/canvas.cc
Lines 1941 to 1943 in 5544f00
const bool should_use_onscreen = renderer_.GetDeviceCapabilities().SupportsFramebufferFetch() && - backdrop_count_ == 0 && render_passes_.size() == 1u; + backdrop_count_ == 0 && render_passes_.size() == 1u && is_onscreen_;I added this to make unit testing easier.
CreateTestCanvasincanvas_unittests.ccdefaulted tois_onscreen = falsewith a render target texture that did not always enablekShaderRead, so gatingshould_use_onscreenonis_onscreen_let unit tests opt into or out of the root-target flip path explicitly. However, I don't need to do that, because I can instead write tests which callDisplayListToTexturedirectly to cover both root-pass flips whereshould_use_onscreen == trueand nestedSaveLayersubpass flips whereshould_use_onscreen == false.The problem with this new conditional is that we actually want
should_use_onscreento betrueeven whenis_onscreen_isfalse. I was tripped up by misleading naming inCanvas. "onscreen" inshould_use_onscreenandBlitToOnscreenactually refers torender_target_, the root destinationRenderTargetpassed toCanvas. Our snapshots are "offscreen" becausePicture.toImage/OffsetLayer.toImagerasterizes into a standalone GPU texture in memory rather than an OS window surface, soDisplayListToTexturesetsis_onscreen = false. This tellsEndReplay()not to callContext::SubmitOnscreento present a window drawable. ForCupertinoActionSheetsnapshots withinDisplayListToTexture, we still wantshould_use_onscreento betrueso thatFlipBackdroprenders directly intorender_target_instead of rendering into a temporary texture and copying torender_target_afterward. It might make sense in a follow-up to renameshould_use_onscreenandBlitToOnscreento refer to the root target rather than "onscreen".Here is how
should_use_onscreenchanges the routeDisplayListToTexturetakes. BecauseCupertinoActionSheethas a rootBackdropFilter,Canvasstarts withrequires_readback_ = trueand initially records into a temporaryRenderTargetCachepass inCanvas::SetupRenderPass. WhenCanvas::SaveLayerreaches thatBackdropFilter, it callsCanvas::FlipBackdrop, which branches onshould_use_onscreen:flutter/engine/src/flutter/impeller/display_list/canvas.cc
Lines 2729 to 2741 in 5544f00
Canvas::BlitToOnscreenifrequires_readback_istrueCreating a new render pass with a depth attachment of
kDefaultStencilConfig, which hasLoadAction::kDontCare, explains why the depth buffer isn't getting cleared by the load action.But why didn't it get cleared when we purged
RenderTargetCachebetween tests in PR #193497? BecauseDisplayListToTexture'srender_target_attachments aren't allocated fromRenderTargetCache—they are local toDisplayListToTexture/Canvasand allocated directly throughAllocatorVK(vmaCreateImage). Even before #193497, those transient 4x MSAA depth textures were already being destroyed at the end of each snapshot (vmaDestroyImageonce the command buffer completed). However, influtter_tester, a singleContextVKandVmaAllocatorlive for the entire process across all tests in the file. VMA retains its underlyingVkDeviceMemoryblocks and allocates the same memory ranges to subsequent tests without ever zeroing freed memory. In Vulkan, clearing attachment memory is solely the job ofLoadAction::kClear.This means that #193497 probably didn't actually do anything relevant. I'll revert it.
In order to fix the square corners issue without introducing this missing depth buffer clear, I just need to keep the good part of the change (improvement to clip stack popping behavior in
EntityPassClipStack::RecordRestore) and test it differently without adding theis_onscreen_condition. I'll work on that in #195716gaaclarke commentedon Nov 2, 2026
reverted in #195436
[Impeller] Reland clipped BackdropFilter square corners fix (flutter#…