Optimize SliverMainAxisGroup/SliverCrossAxisGroup paint function #129310
Conversation
github-actions
bot
added
f: scrolling
thkim1011
changed the title
| // If renderHeader._lastStartedScrollDirection is not ScrollDirection.forward, then we shouldn't see the header at all. | ||
| expect((renderHeader.parentData! as SliverPhysicalParentData).paintOffset.dy, equals(0.0)); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Both the cross axis group and the main axis group tests here pass without the change to the paint method in the group widgets. They don't validate the change.
What they are checking is that the SliverToBoxAdapters are painting/not painting as expected, but not the sliver groups.
There was a problem hiding this comment.
How would I solve this then? I'd need to verify that SliverCrossAxisGroup and SliverMainAxisGroup draws exactly two boxes, but I'm not sure how to do this.
There was a problem hiding this comment.
There are a couple different ways to do this. You can look up the Sliver*AxisGroup in the same way you are looking up the SliverToBoxAdapter.
You could mock out the class and increment a counter every time it paints.
There are probably a lot of different examples in existing tests if that helps. :)
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
|
||
| // Test sliver which always attempts to paint itself whether it is visible or not. | ||
| // Use for checking if slivers which take sliver children paints optimally. | ||
| class RenderMockSliverToBoxAdapter extends RenderSliverToBoxAdapter { |
There was a problem hiding this comment.
We were chatting about this yesterday, why not put this in the rendering directory under a file name like sliver_utils.dart? We typically try to keep utilities like this organized in that way. Importing the rendering library here also kind of indicates this should be in rendering.
There was a problem hiding this comment.
Ah I discovered the test_widgets.dart file after chatting but yeah it makes sense that this should go in rendering. I'll fix this.
…into tae/optimize-painting
flutter/flutter@d7ed5dc...9def8f6 2023-07-25 [email protected] Proposal to add barrier configs for showDatePicker, showTimePicker and showAboutDialog. (flutter/flutter#130484) 2023-07-25 [email protected] Roll Flutter Engine from a7a842ee9ccd to 036c58f79307 (1 revision) (flutter/flutter#131244) 2023-07-25 [email protected] Roll Flutter Engine from 3baca2fe55c8 to a7a842ee9ccd (1 revision) (flutter/flutter#131243) 2023-07-25 [email protected] Roll Flutter Engine from 9a0192d965e0 to 3baca2fe55c8 (1 revision) (flutter/flutter#131241) 2023-07-25 [email protected] Roll Flutter Engine from ceb2674e82b4 to 9a0192d965e0 (3 revisions) (flutter/flutter#131230) 2023-07-25 [email protected] Roll Flutter Engine from 4fded78e5a01 to ceb2674e82b4 (2 revisions) (flutter/flutter#131229) 2023-07-25 [email protected] Roll Flutter Engine from ff02fa72acce to 4fded78e5a01 (2 revisions) (flutter/flutter#131225) 2023-07-24 [email protected] Roll Flutter Engine from a489c7496268 to ff02fa72acce (1 revision) (flutter/flutter#131221) 2023-07-24 [email protected] Roll Flutter Engine from 815b97157dc7 to a489c7496268 (3 revisions) (flutter/flutter#131218) 2023-07-24 [email protected] Roll Flutter Engine from 2b8d83fa20e3 to 815b97157dc7 (5 revisions) (flutter/flutter#131214) 2023-07-24 [email protected] Use toStringAsFixed in DecorationImage.toString (flutter/flutter#131026) 2023-07-24 [email protected] Roll Flutter Engine from aa876f6bec69 to 2b8d83fa20e3 (3 revisions) (flutter/flutter#131207) 2023-07-24 [email protected] Fix M3 TimePicker dial background uses incorrect color (flutter/flutter#131045) 2023-07-24 [email protected] Fix floating SnackBar throws when FAB is on the top (flutter/flutter#129274) 2023-07-24 [email protected] Update link to unbounded constraints error (flutter/flutter#131205) 2023-07-24 [email protected] Optimize SliverMainAxisGroup/SliverCrossAxisGroup paint function (flutter/flutter#129310) 2023-07-24 [email protected] [DropdownMenu] Close menu after editing is complete (flutter/flutter#130710) 2023-07-24 [email protected] Reduce usage of testUsingContext (flutter/flutter#131078) 2023-07-24 [email protected] Roll Packages from 2266a76 to 8028caf (13 revisions) (flutter/flutter#131196) 2023-07-24 [email protected] Fix material date picker behavior when changing year (flutter/flutter#130486) 2023-07-24 [email protected] Update Gallery demo app themes for Material3 compatibility (flutter/flutter#131093) 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 [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…tter#129310) This PR changes the paint functions for SliverMainAxisGroup and SliverCrossAxisGroup so that only visible slivers are painted. Fixes flutter#129214.
…tter#129310) This PR changes the paint functions for SliverMainAxisGroup and SliverCrossAxisGroup so that only visible slivers are painted. Fixes flutter#129214.
2 participants
This PR changes the paint functions for SliverMainAxisGroup and SliverCrossAxisGroup so that only visible slivers are painted.
Fixes #129214.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.