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

Don't instantiate ColorPicker in EditorPropertyColor constructor #101570

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

larspet
Copy link
Contributor

@larspet larspet commented Jan 15, 2025

The EditorPropertyColor constructor was using ColorPickerButton::get_popup() and ColorPickerButton::get_picker(), which was instantiating the ColorPicker without it being opened first. This led to 40+ pickers being instantiated in some cases.

This PR avoids calling those getters by utilizing the ColorPickerButton::toggled signal instead.

On a debug build I get these results (quickly measured by eye & timer):

  • Opening Editor Settings > Text Editor > Theme:
    • Master: 2 sec
    • This PR: instant
  • Opening the MRP inspector:
    • Master: 1.5 sec
    • This PR: 0.2 sec (must be other Node properties making it not instant)

Switching away to another setting or inspector is faster now too, since there's no need to delete all the pickers.

However, now that it's using a different signal there may be a regression somewhere. The pickers work using both mouse or keyboard though.

@Chaosus Chaosus added this to the 4.4 milestone Jan 15, 2025
@AThousandShips AThousandShips requested a review from a team January 15, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inspector loads slowly when there's many Color properties
2 participants