-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
perf: Speed Up Opening of Settings Dialog #4193
Conversation
clang-tidy review says "All clean, LGTM! 👍" |
There is also this call chatterino2/src/widgets/BaseWindow.cpp Line 787 in f2b35fb
triggered a LOT by WM_SHOWWINDOW , at minimum it could be changed to check if nativeScale_ actually changed, but it could be also dropped from here i think, because WM_DPICHANGED handler is already calling updateScale()
another pr should properly fix #1266 without this double scalling fuckery :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just changelog entry rewording, other than that the changes are solid 👍
thank you
I noticed that as well. I changed it to only call |
I noticed that as well.
Probably, though, maybe
True, I think there might be more low-hanging fruits. |
clang-tidy review says "All clean, LGTM! 👍" |
WM_DPICHANGED covers moving between screens and current screen change, so im 80% sure it could be removed from here, but i would leave it, once chatterino upgrades to qt6 it can be revisited with new dpi oriented apis PR looks good to me 👍 |
clang-tidy review says "All clean, LGTM! 👍" |
Pull request checklist:
CHANGELOG.md
was updated, if applicableDescription
I noticed that opening the settings dialog is really slow.
Two things jump out:
updateScale
.setWindowFlags
results in style recalculations (Before, After).I'm not sure if my fix to reduce calls to
updateScale
is correct. It would be nice if people could test this (e.g. with different zoom-levels and on high-dpi). If my fix turns out incorrect, I'd like to add a comment in the code as to why the change-callback must be run when creating the window (the second call toupdateScale
as added in d796517 to fix #1266).