Implement paintsChild on RenderObjects that skip painting on their children#103768
Implement paintsChild on RenderObjects that skip painting on their children#103768fluttergithubbot merged 5 commits intoflutter:masterfrom
Conversation
flutter-dashboard
bot
added
f: material design
| @override | ||
| void applyPaintTransform(RenderBox child, Matrix4 transform) { | ||
| if (size.isEmpty || child.size.isEmpty) { | ||
| if (!paintsChild(child)) { |
There was a problem hiding this comment.
Will this remove accessibility transform as well? I remember widget like Opacity can still include in semantics when opacity in zero. The semantics transform is calculate using this method
There was a problem hiding this comment.
This is existing behavior for this widget. Yes, this means the a11y transform will be zeroed out. If we want to change that I think it should be done in a separate PR.
There was a problem hiding this comment.
There is this https://api.flutter.dev/flutter/widgets/Opacity/alwaysIncludeSemantics.html
does that mean it was already broken before?
There was a problem hiding this comment.
Oh nvm, this is RenderFittedBox, I thought it was RenderProxyBox
| /// supply a zeroed out matrix if it would not otherwise be able to determine | ||
| /// a valid matrix for the child and thus cannot meaningfully determine where | ||
| /// the child would paint. | ||
| bool paintsChild(covariant RenderObject child) { |
There was a problem hiding this comment.
is there an easy way to check whether the layout will be offscreen? or if there is a parent with clip that clipped the whole renderobject? it may be a good default implementation
There was a problem hiding this comment.
No.
RenderOffstage and RenderOpacity, for example, return the same exact transform/clip whether they are going to paint their children or not.
Offstage does not really mean the same as offscreen - it's not as if you're getting laid out or painted in some coordinates that are outside of the current viewport. It's just that you will never get paint called after being laid out.
| /// one render object into coordinates local to another render object. | ||
| /// | ||
| /// Some RenderObjects will provide a zeroed out matrix in this method, | ||
| /// indicating that even though |
There was a problem hiding this comment.
indicating that even though ....?
There was a problem hiding this comment.
Hah. I got halfway through writing this and thought of what I should say in paintsChild and forgot to come back. Updated.
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
Seems like a reasonable approach
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
I am not super-exited about adding another method to RenderObject, but I also don't see another approach for this, so reluctantly
LGTM.
|
This pull request is not suitable for automatic merging in its current state.
|
|
I only see scuba chagnes that look expected and/or unrelated to this patch. |
5 participants
#103638 was an alternative attempt at this, but doesn't work:
Adds an optimization to InkFeature so that it avoids painting entirely if something between the target and feature stops painting.
Part of the visibility_detector refactor.
@gspencergoog @chunhtai might find this interesting since I'm touching some code you last modified.