Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Check coverage after screen clip #41129

Merged
auto-submit merged 3 commits into from
Apr 12, 2023
Merged

[Impeller] Check coverage after screen clip #41129

auto-submit merged 3 commits into from
Apr 12, 2023

Conversation

@bdero
Copy link
Member

@bdero bdero commented Apr 12, 2023

Resolves flutter/flutter#124658.

Need to add error messages around all failed scenarios in EntityPass since we don't have a recovery and the whole frame fails to render. Will CP this.

Before: Failed rendering.

After:

image

@bdero bdero requested review from dnfield and jonahwilliams April 12, 2023 20:09
@bdero bdero self-assigned this Apr 12, 2023
@flutter-dashboard

This comment was marked as outdated.

Sign in to view
jonahwilliams
jonahwilliams reviewed Apr 12, 2023
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need to CP this we'll need a test case

@bdero
Copy link
Member Author

bdero commented Apr 12, 2023

Working on a playground repro for this

@bdero bdero force-pushed the bdero/screen-clip-check branch from ac219dc to f0602bb Compare April 12, 2023 20:40
@bdero
Copy link
Member Author

bdero commented Apr 12, 2023

Added a playground that hits this first frame:

Before:

[ RUN      ] Play/AiksTest.CanRenderTinyOverlappingSubpasses/Metal
2023-04-12 13:51:54.729 impeller_unittests[21430:3091893] Metal API Validation Enabled
2023-04-12 13:51:54.729 impeller_unittests[21430:3091893] Metal GPU Validation Enabled
[FATAL:flutter/impeller/base/validation.cc(47)] Break on 'ImpellerValidationBreak' to inspect point of failure: Could not render into the surface.
[ERROR:flutter/fml/backtrace.cc(108)] Caught signal SIGABRT during program execution.
Frame 0: 0x7ff80889db45 abort
Frame 1: 0x103063192 fml::KillProcess()
Frame 2: 0x10306312d fml::LogMessage::~LogMessage()
Frame 3: 0x1030631c3 fml::LogMessage::~LogMessage()
Frame 4: 0x103042196 impeller::ImpellerValidationBreak()
Frame 5: 0x103041fa5 impeller::ValidationLog::~ValidationLog()
Frame 6: 0x103042333 impeller::ValidationLog::~ValidationLog()
Frame 7: 0x102e3ba07 impeller::Playground::OpenPlaygroundHere()
Frame 8: 0x105b38f3c impeller::AiksPlayground::OpenPlaygroundHere()
Frame 9: 0x105b38e38 impeller::AiksPlayground::OpenPlaygroundHere()
Frame 10: 0x102c4dfcd impeller::testing::AiksTest_CanRenderTinyOverlappingSubpasses_Test::TestBody()
Frame 11: 0x105cf1279 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
Frame 12: 0x105cc7945 testing::internal::HandleExceptionsInMethodIfSupported<>()
Frame 13: 0x105cc7861 testing::Test::Run()
Frame 14: 0x105cc85f2 testing::TestInfo::Run()
Frame 15: 0x105cc9a2b testing::TestSuite::Run()
Frame 16: 0x105cd670d testing::internal::UnitTestImpl::RunAllTests()
Frame 17: 0x105cf7c59 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
Frame 18: 0x105cd62a5 testing::internal::HandleExceptionsInMethodIfSupported<>()
Frame 19: 0x105cd613a testing::UnitTest::Run()
Frame 20: 0x1036203f3 RUN_ALL_TESTS()
Frame 21: 0x1036201d5 main
Frame 22: 0x210f9641f start

[1]    21430 abort      ../out/host_debug_unopt/impeller_unittests --timeout=0  --enable_playground

After:
image

jonahwilliams
jonahwilliams approved these changes Apr 12, 2023
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@flutter-dashboard
Copy link

flutter-dashboard bot commented Apr 12, 2023

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #41129 at sha 3ac7075

@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023
@auto-submit auto-submit bot merged commit 9b72a5d into flutter:main Apr 12, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
9b72a5ddf [Impeller] Check coverage after screen clip (flutter/engine…
86a2e1e 
…#41129)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
9b72a5ddf [Impeller] Check coverage after screen clip (flutter/engine…
f212c01 
…#41129)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
9b72a5ddf [Impeller] Check coverage after screen clip (flutter/engine…
7eef877 
…#41129)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
9b72a5ddf [Impeller] Check coverage after screen clip (flutter/engine…
566a93c 
…#41129)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
9b72a5ddf [Impeller] Check coverage after screen clip (flutter/engine…
f3415b1 
…#41129)
bdero added a commit to bdero/flutter-engine that referenced this pull request Apr 13, 2023
@bdero
[Impeller] Check coverage after screen clip (flutter#41129)
d99bf1a 
[Impeller] Check coverage after screen clip

(cherry picked from commit 9b72a5d)
This was referenced Apr 13, 2023
Merged
Merged
bdero added a commit that referenced this pull request Apr 14, 2023
@bdero
[Impeller] Print a log on all non-recoverable failure paths in Entity…
00bef00 
…Pass (#41178)

This eliminates a lot of surface area when debugging non-recoverable
frame failures such as #41129.
Every failure case should be covered with at least one log, but without
recursive log spam.
zhongwuzw pushed a commit to zhongwuzw/engine that referenced this pull request Apr 14, 2023
@bdero @zhongwuzw
[Impeller] Check coverage after screen clip (flutter#41129)
b0e11c2 
[Impeller] Check coverage after screen clip
zhongwuzw pushed a commit to zhongwuzw/engine that referenced this pull request Apr 14, 2023
@bdero @zhongwuzw
[Impeller] Print a log on all non-recoverable failure paths in Entity…
9c3b23b 
…Pass (flutter#41178)

This eliminates a lot of surface area when debugging non-recoverable
frame failures such as flutter#41129.
Every failure case should be covered with at least one log, but without
recursive log spam.
CaseyHillers pushed a commit that referenced this pull request Apr 14, 2023
@bdero
[CP][Impeller] Check coverage after screen clip (#41129) (#41167)
38a80ad 
Beta CP for #41129.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dnfield dnfield dnfield approved these changes

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bdero bdero

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller will affect goldens

Projects

No open projects
Impeller
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Impeller] CupertinoDatePicker inside CupertinoModalPopup not working

3 participants

@bdero @dnfield @jonahwilliams