Add bulk change guards to successive theme overrides in Editor and GUI #83626
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
YuriSizov
force-pushed
the
control-less-is-more-as-in-less-excessive-notifications-is-more-performance
branch
from
d86fac5
to
215e036
Compare
akien-mga
approved these changes
Oct 19, 2023
|
Thanks! |
YuriSizov
deleted the
control-less-is-more-as-in-less-excessive-notifications-is-more-performance
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that in
TabContainerwe're overriding every theme property of its internalTabBar, which triggers a theme update for every property update. We have guards just for the occasion, so I checked all GUI code, both in editor and in scene, and added those guards.I only touched cases where there are at least three sequential
add_theme_*_overridecalls on the same control. While two sequential updates are also not great, I decided not to bother to avoid making code more verbose than it needs to be. I don't think performance improvements from that would be massive. As an exception, I did update twos in places where I was already making other changes.Another exception is calls before a control is added to the tree. If it's not in the tree, it doesn't matter, so those I ignored regardless of the number of calls. In some cases I modified code to follow this pattern (add overrides before adding as a child), and had to update nearby code for consistency as well.
This should make theme updates a bit snappier, especially in the editor. But I don't think it would be drastic on a decently powerful system. I didn't notice much difference for startup times, but maybe changing the editor theme is a bit better now. Ultimately, it affects individual controls rather than propagates throughout the tree, so it shouldn't be too impactful.