Skip to content

Stack and other multi-child layout renderers should not call markNeedsPaint if they don't actually change their layout #5325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
apwilson opened this issue Aug 10, 2016 · 10 comments
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project team-framework Owned by Framework team triaged-framework Triaged by Framework team

Comments

@apwilson
Copy link
Contributor

Example is a Stack with two children, one always changing and the other never changing. The changing child shouldn't cause the non-changing child to repaint.

@abarth
Copy link
Contributor

abarth commented Aug 10, 2016

Currently layout calls markNeedsPaint unconditionally, but we could do something smarter here if we knew which render object actually painted content. It's possible that the layout change actually only affects render objects that live in their own repaint boundaries.

@Hixie
Copy link
Contributor

Hixie commented Aug 10, 2016

I don't really understand this request. Assuming there's no repaint boundary, if we don't repaint the child, how will the child be visible after the parent's layout's changed?

@abarth
Copy link
Contributor

abarth commented Aug 10, 2016

If nothing calls markNeedsPaint in this repaint boundary, we'll just use the recording from last time (which will contain the child). If something does call markNeedsPaint, then we'll paint the child again.

We're currently calling markNeedsPaint when there's nothing actually dirty about the painting in this repaint boundary.

@Hixie
Copy link
Contributor

Hixie commented Aug 10, 2016

If the layout changed, then the paint is dirty, pretty much by definition, no?

@abarth
Copy link
Contributor

abarth commented Aug 10, 2016

That's currently the definition, but there are situations where that is an over-approximation. The case that Andrew was hitting is the following:

Stack
  - unpositioned child1
  - positioned child2, with a repaint boundary

Child2 decides it wants to layout. That causes the stack to layout. The stack decides to put child1 and child2 in exactly the same spots as before.

In our current approach, the stack marks itself as needing paint even though it didn't change anything about itself and it doesn't actually have anything new to paint. That causes child1 to repaint, also spuriously even though layout didn't recurse into child1. (In this scenario, child2 did actually change in some meaningful way, but its repainting is contained by its repaint boundary.)

There's an opportunity to do better here by not calling markNeedsPaint unconditionally from layout.

@Hixie
Copy link
Contributor

Hixie commented Aug 10, 2016

Oh, I see. Sure.

@Hixie Hixie changed the title markNeedsLayout implying markNeedsPaint causes too many paints Stack and other multi-child layout renderers should not call markNeedsPaint if they don't actually change their layout Aug 10, 2016
@Hixie Hixie modified the milestone: Flutter 1.0 Sep 12, 2016
@eseidelGoogle
Copy link
Contributor

It's not blocking @apwilson, removing pink.

@Hixie Hixie added c: performance Relates to speed or footprint issues (see "perf:" labels) framework flutter/packages/flutter repository. See also f: labels. labels Nov 14, 2017
@liyuqian
Copy link
Contributor

liyuqian commented Nov 4, 2019

@Hixie @tvolkert @HansMuller is there any plan or work in progress for this old issue? (It's currently the oldest in terms of the last update date.)

@HansMuller
Copy link
Contributor

I don't know of any active or planned work on this issue.

@Hixie
Copy link
Contributor

Hixie commented Nov 5, 2019

This is definitely something we should fix one day. It's not something anyone has looked at yet.
cc @goderbauer

@Hixie Hixie modified the milestones: Stretch Goals, New Stretch Goals Jan 7, 2020
@kf6gpe kf6gpe added the P3 Issues that are less important to the Flutter project label May 29, 2020
@kf6gpe kf6gpe modified the milestone: Stretch Goals Jun 1, 2020
@Hixie Hixie removed this from the [DEPRECATED] Stretch Goals milestone Jun 16, 2020
@kf6gpe kf6gpe removed this from the [DEPRECATED] Stretch Goals milestone Jul 7, 2020
@kf6gpe kf6gpe modified the milestone: [DEPRECATED] Stretch Goals Jul 22, 2020
@Hixie Hixie removed this from the - milestone Aug 17, 2020
@flutter-triage-bot flutter-triage-bot bot added the team-framework Owned by Framework team label Jul 8, 2023
@flutter-triage-bot flutter-triage-bot bot added the triaged-framework Triaged by Framework team label Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project team-framework Owned by Framework team triaged-framework Triaged by Framework team
Projects
None yet
Development

No branches or pull requests

8 participants