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

RFC: Add caches to avoid recomputing MinSize multiple times #4681

Closed
wants to merge 11 commits into from

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Feb 27, 2024

Description:

This is a request for comments about adding an automatic and built-in cache for MinSize values in BaseWidget and containers. This avoids having to recalculate the size by walking the entire tree of objects each time. It does incur a slight change of behaviours for widgets that overwrite the MinSize() method and use the size given from the BaseWidget to add or subtract to. I have updated all widgets to make sure that the renderer defines the needed MinSize and as such all widgets are updated to use the cache.

Checklist:

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

Where applicable:

  • Public APIs match existing style and have Since: line.
  • Any breaking changes have a deprecation path or have been discussed.

@Jacalz Jacalz force-pushed the rfc-basewidget-minsize-cache branch from 7de68d1 to 0fd0cf5 Compare February 27, 2024 17:26
@Jacalz Jacalz marked this pull request as draft February 27, 2024 17:27
@Jacalz Jacalz changed the title RFC for adding a cache for MinSize RFC: Add caches to avoid recomputing MinSize multiple times Feb 27, 2024
@andydotxyz
Copy link
Member

Hmm, it might be a bit out there - but perhaps this does not have to be on the widget at all.
If it is to be reset when Refresh is called then we could probably cache it in a similar way to the texture is cache?
That way it would not need any widgets to change at all...

Looking at the bigger picture the tree walk leans heavily on MinSize but does not know if it is changed - perhaps we could even save the tree walk if we are able to determine that no minimum has changed between walks?

@Jacalz
Copy link
Member Author

Jacalz commented Mar 1, 2024

Interesting thoughts. I will try to get it to work and get all the tests to pass first using this solution and then I can looking into how the other cache works.

FYI: I think using the cache for container might be entirely flawed? We don't want to have to refresh the whole container when a widget changes, right?

@Jacalz
Copy link
Member Author

Jacalz commented May 2, 2024

Continuing this in #4827 instead

@Jacalz Jacalz closed this May 2, 2024
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.

2 participants