Skip to content

Conversation

harryterkelsen
Copy link
Contributor

Adds a test that checks that pictures are not composited if they are clipped out (or not in the frame) in the final scene. This is important to test since we have benchmarks that depend on this behavior being implemented properly.

Part of the web renderer unification. See #172311

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@harryterkelsen harryterkelsen requested a review from mdebbar August 26, 2025 17:22
@harryterkelsen harryterkelsen force-pushed the web-fix-picture-culling branch from 1406a0b to f5a8f4b Compare August 26, 2025 17:28
@github-actions github-actions bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Aug 26, 2025
@harryterkelsen harryterkelsen added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Aug 26, 2025
@fluttergithubbot
Copy link
Contributor

An existing Git SHA, f5a8f4ba77167a80e05ab0428157e05de5c60370, was detected, and no actions were taken.

To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with --force) that already was pushed before, push a blank commit (git commit --allow-empty -m "Trigger Build") or rebase to continue.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new test file to verify that pictures are not rasterized when they are clipped out of the scene. This is achieved by adding a @visibleForTesting getter to expose the EmbedderFrameContext for inspection in tests. The tests are well-structured and cover various culling scenarios. My feedback focuses on improving the maintainability of one of the tests by refactoring duplicated code.

@harryterkelsen harryterkelsen added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025
@harryterkelsen harryterkelsen added this pull request to the merge queue Aug 26, 2025
Merged via the queue into flutter:master with commit 93d5ace Aug 26, 2025
183 of 184 checks passed
@harryterkelsen harryterkelsen deleted the web-fix-picture-culling branch August 26, 2025 20:36
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 27, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 28, 2025
flutter/flutter@c65f01d...da5523a

2025-08-27 dacoharkes@google.com [native assets] Roll dependencies (flutter/flutter#174522)
2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from 8cf2faada2b5 to 7e6da45059c5 (5 revisions) (flutter/flutter#174523)
2025-08-27 43054281+camsim99@users.noreply.github.com [Android] Restrict AOT shared library engine flag to trusted paths (flutter/flutter#173359)
2025-08-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from L5zGzsIWIS8N36AFQ... to bHYRvLv2Dg56RWZF2... (flutter/flutter#174518)
2025-08-27 engine-flutter-autoroll@skia.org Roll Packages from 1ef712e to 86fbeec (7 revisions) (flutter/flutter#174521)
2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from 448a0d0e57e3 to 8cf2faada2b5 (1 revision) (flutter/flutter#174510)
2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from 4703976a4dae to 448a0d0e57e3 (3 revisions) (flutter/flutter#174494)
2025-08-27 sokolovskyi.konstantin@gmail.com Fix SliverMainAxisGroup and SliverCrossAxisGroup gestures' local positions. (flutter/flutter#174265)
2025-08-27 robert.ancell@canonical.com Fix lock up when window resized with merged UI and platform threads. (flutter/flutter#172893)
2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from dc2872de506f to 4703976a4dae (1 revision) (flutter/flutter#174489)
2025-08-27 engine-flutter-autoroll@skia.org Roll Dart SDK from db45c0ce46f9 to 89023922f96d (2 revisions) (flutter/flutter#174481)
2025-08-27 32538273+ValentinVignal@users.noreply.github.com Migrate examples and defaults to `WidgetState` (flutter/flutter#174421)
2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from 8e48b9e6d67b to dc2872de506f (7 revisions) (flutter/flutter#174479)
2025-08-27 36861262+QuncCccccc@users.noreply.github.com SnackBar with action no longer auto-dismiss (flutter/flutter#173084)
2025-08-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 9054cd8af73c to db45c0ce46f9 (1 revision) (flutter/flutter#174438)
2025-08-26 engine-flutter-autoroll@skia.org Roll Skia from f941bfab7c09 to 8e48b9e6d67b (4 revisions) (flutter/flutter#174468)
2025-08-26 matanlurey@users.noreply.github.com Fix bug in test_golden_comparator, add an e2e test. (flutter/flutter#174459)
2025-08-26 matt.boetger@gmail.com fix typo in test_profile/README.md (flutter/flutter#174384)
2025-08-26 matanlurey@users.noreply.github.com Remove CP labels on not-merged PRs, and explain why (flutter/flutter#174448)
2025-08-26 1063596+reidbaker@users.noreply.github.com Increase testing coverage and maintainability of android manifest parsing logic (flutter/flutter#174070)
2025-08-26 1961493+harryterkelsen@users.noreply.github.com [web] Add test that pictures are not rasterized when clipped out (flutter/flutter#174452)
2025-08-26 engine-flutter-autoroll@skia.org Roll Skia from 538260c45b4a to f941bfab7c09 (3 revisions) (flutter/flutter#174450)
2025-08-26 30870216+gaaclarke@users.noreply.github.com fixes the vulkan image layout transitions for mipmap generation (flutter/flutter#173884)
2025-08-26 15619084+vashworth@users.noreply.github.com Move flakey iOS tests to bringup (flutter/flutter#174446)
2025-08-26 30870216+gaaclarke@users.noreply.github.com [Impeller] Make sure inline passes always do a clear action. (flutter/flutter#174083)
2025-08-26 engine-flutter-autoroll@skia.org Roll Skia from 21214d63fc40 to 538260c45b4a (2 revisions) (flutter/flutter#174441)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine flutter/engine related. See also e: labels. platform-web Web applications specifically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants