Skip to content
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

Only call updateLayout once on the parent. #3864

Merged
merged 2 commits into from
May 6, 2023

Conversation

Bluebugs
Copy link
Contributor

@Bluebugs Bluebugs commented May 4, 2023

Description:

updateLayout was called for each children that has a min size change instead of one time once all the children have been updated. This lead to some large performance impact on Tree specifically, but all nested container with large number of object would have been impacted.

Checklist:

  • Lint and formatter run with no errors.
  • Tests all pass.

@Bluebugs Bluebugs requested a review from andydotxyz May 4, 2023 16:46
@coveralls
Copy link

coveralls commented May 4, 2023

Coverage Status

Coverage: 61.994% (+0.01%) from 61.984% when pulling 7ff9d3b on Bluebugs:bugs/speedup-tree into bc2b858 on fyne-io:develop.

Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

Nicely done. Just one comment in-line on the struct field perhaps not being necessary?

internal/driver/common/canvas.go Outdated Show resolved Hide resolved
@Jacalz
Copy link
Member

Jacalz commented May 6, 2023

Also, please remember to check the checkmarks in the PR template.

@andydotxyz
Copy link
Member

Also, please remember to check the checkmarks in the PR template.

And don't remove the "tests included" its presence is not optional. If there are no tests, leave it there unchecked so we are fully aware that there are no new tests for this PR.

@Bluebugs Bluebugs merged commit 56cab00 into fyne-io:develop May 6, 2023
11 checks passed
@Bluebugs Bluebugs deleted the bugs/speedup-tree branch May 6, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants