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

style_dialog_optimization #19146

Merged

Conversation

RomanPudashkin
Copy link
Contributor

Resolves: #13193
Resolves: #13104
Resolves: #17060

Caused by dca87eb

Copy link
Contributor

@cbjeukendrup cbjeukendrup left a comment

Choose a reason for hiding this comment

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

This solution seems good to me; especially since those QDialog dialogs will be replaced by QML at some point, I think we shouldn't spend too much time on them.

However, I do wonder why it was no problem in MS3. There, nothing was done about disabling accessibility. Do you have any idea about that?
And might there be a way to apply this fix for other dialogs automatically too, like detecting whether an event comes from a dialog that is not opened yet? (Probably not necessary though, because the style dialog is by far the biggest one, so the problem is probably not significant for other dialogs)

@RomanPudashkin
Copy link
Contributor Author

However, I do wonder why it was no problem in MS3. There, nothing was done about disabling accessibility. Do you have any idea about that?

This is most likely a consequence of the fact that we are using our custom accessibility root object (see AccessibilityController::init()) and the default is QApplication (which was used in MS3)

And might there be a way to apply this fix for other dialogs automatically too, like detecting whether an event comes from a dialog that is not opened yet? (Probably not necessary though, because the style dialog is by far the biggest one, so the problem is probably not significant for other dialogs)

I'm leaning toward not applying this fix to other dialogs for now. Implementing automatic detection of whether events need to be ignored would be quite problematic (especially because we have to start ignoring them before calling setupUi)

@Jojo-Schmitz
Copy link
Contributor

I can confirm that this PR fixes the issue for me, similar to the earlier change one you asked be to test, but as far as I can tell without the bad side effects on accessibilty

@MarcSabatella
Copy link
Contributor

If we were to investigate using this elsewhere, there are three places in particular where people have reported issues with lag and it might turn out to be for the same reason:

  1. Preferences dialog - see [MU4 Issue] Long delay to open Format->Style page #13193 (comment)
  2. Palette search - see [MU4 Issue] Palette search box slow to open and close #12753.
  3. Properties panel - see [MU4 Issue] When selecting first note there is an appreciable time lag before it sounds and is highlighted #14383

I also wonder if explicitly disabling accessibility might be a workaround for now (obviously, not for screen reader users). But I only know how to do that on Linux, and I don't experience the issue there.

@zacjansheski zacjansheski self-assigned this Aug 24, 2023
@zacjansheski
Copy link
Contributor

Tested on Mac OS 11.6.5, Ubuntu 20.04.6, Windows 11 (With VoiceOver, Narrator, NVDA, and Orca)

#13193 #13104 #17060. FIXED

@RomanPudashkin RomanPudashkin merged commit 31bbbb5 into musescore:master Aug 24, 2023
11 checks passed
@RomanPudashkin RomanPudashkin deleted the style_dialog_optimization branch August 24, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants