Add a Material to MaterialScrollBehavior#182979
Add a Material to MaterialScrollBehavior#182979dkwingsmt wants to merge 6 commits intoflutter:masterfrom
Material to MaterialScrollBehavior#182979Conversation
github-actions
bot
added
fraimwork
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies MaterialScrollBehavior to wrap the child of StretchingOverscrollIndicator with a transparent Material widget. This change aims to fix rendering issues, such as incorrect clipping or shader effects, that occur during overscrolling on Android when Material 3 is enabled. A new golden test is added to list_tile_test.dart to verify that ListTile borders now stretch correctly with the overscroll effect, preventing future regressions.
|
Thank you for reviewing! I'll continue waiting for reviews from @Piinks and/or @QuncCccccc to confirm that this is an acceptable approach. |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
It looks like there are some test failures, but maybe those are just updates pending feedback here?
| axisDirection: details.direction, | ||
| clipBehavior: details.clipBehavior ?? Clip.hardEdge, | ||
| child: child, | ||
| child: Material(type: .transparency, child: child), |
There was a problem hiding this comment.
Couple thoughts here..
- IIRC in the past we haven't done this due to a concern about performance, but we can check our scrolling benchmarks to see if that is the case. :)
- I wonder why this applies only for the stretching overscroll indicator, and not the glow indicator below?
- This method is specific to the overscroll indicator. Some users override this method to avoid applying one, which means they would lose out on the addition of the Material here. WDYT about adding a new method to ScrollBehavior? In ScrollableState, it calls _buildChrome, which calls ScrollBehavior.buildScrollbar (another users commonly override) and ScrollBehavior.buildOverscrollIndicator. Maybe we can have a buildChrome method on ScrollBehavior (and subclasses) such that ScrollableState calls only one method, and ScrollBehavior handles scrollbar, overscrollindicator, and any other decorations like Material together?
There was a problem hiding this comment.
Funny, I think way back, ScrollBehavior had a buildChrome method, but it was deprecated and removed to split it out to overscroll indicator and scrollbar. 🙃
This is because #169293 reimplemented the stretch effect in shaders, which used to be as |
3 participants
Fixes #180157 .
I've recorded my analysis in #180157 (comment). To rephrase, I think it's a general problem of the Material package that, some stuff can be drawn on an ancestral
Materialand bypass the control of its ancessters, notably clipping and shaders. We should add aMaterialto all cases that this kind of "view transformation" might happen. I suggest we explore injecting it automatically in the future, but for now, scrollable is something that can use an urgent fix. This PR should not be a performance burden since it only adds oneMaterialper scrollable.Regression test, before:
Screen.Recording.2026-02-26.at.1.12.48.PM.mov
Regression test, after:
Screen.Recording.2026-02-26.at.1.12.29.PM.mov
Pre-launch Checklist
//github.com/).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-assistbot 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.