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

When the Parameter combo box in the Parameters dialog gets focus, rebuild the parameter list. #707

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

Conversation

jcsteh
Copy link
Owner

@jcsteh jcsteh commented Mar 7, 2022

This deals with cases where changing a parameter value also changes parameter names; e.g. changing the bad type in ReaEq.
Fixes #706.

This is a fairly naive solution in that it doesn't bother to keep track of whether a parameter value has actually changed. Thus, it rebuilds the parameter list more often than it strictly needs to. That said, unless this actually causes performance problems for plugins with large numbers of parameters, I'd prefer not to complicate the code here. It should be possible if this does cause problems, though.

Either way, this needs testing on plugins with large numbers of parameters.

@AppVeyorBot
Copy link

Build failed! Build osara pr707-875,3738ff48 failed (commit 3738ff482f by @jcsteh)

@jcsteh
Copy link
Owner Author

jcsteh commented Mar 7, 2022

Oh look. It failed on Mac. What a Zarqing surprise. No idea what to do about that. SWELL doesn't support WM_SETFOCUS either, so I can't use that either.

jcsteh referenced this pull request Mar 19, 2022
…uild the parameter list.

This deals with cases where changing a parameter value also changes parameter names; e.g. changing the bad type in ReaEq.
Fixes #706.
@LeonarddeR
Copy link
Collaborator

... SWELL doesn't support WM_SETFOCUS either

Looks like that assumption is no longer correct. See justinfrankel/WDL@20057d4
It will of course require a bump of swell/wdl.

@AppVeyorBot
Copy link

Build failed! Build osara pr707-962,e27b97f7 failed (commit e27b97f703 by @jcsteh)

@jcsteh
Copy link
Owner Author

jcsteh commented May 7, 2022

Oh. I forgot I need to change this to use WM_SETFOCUS instead of CBN_SETFOCUS. That's going to require implementing a WindowProc for the combo box, which is annoying, but should be possible.

…uild the parameter list.

This deals with cases where changing a parameter value also changes parameter names; e.g. changing the bad type in ReaEq.
Fixes #706.
@jcsteh
Copy link
Owner Author

jcsteh commented Aug 24, 2022

Would someone be able to test this on Mac? See #706 (comment) for testing instructions.

@jcsteh
Copy link
Owner Author

jcsteh commented Aug 24, 2022

Oh, and it still needs testing with plugins with a large number of parameters, either on Windows or Mac.

@pitermach
Copy link

pitermach commented Aug 24, 2022 via email

@jcsteh
Copy link
Owner Author

jcsteh commented Aug 24, 2022

On Windows, the names change correctly but there's now some lag when the combo box gets focused. With NVDA and ZDSR it's about a second, with Narrator it's a bit less but still noticeable.

Yeah, that's what I was afraid of. That's OSARA trying to fetch all the parameter names again.

On Mac, there is no lag when you tab into the popup, but the names in the list don't update.

Ug. Terrific. That means the code isn't actually working at all, probably because the focus notification doesn't work properly on Mac even though it now should.

Would this be any easier to do if #534 were implemented?

No; I suspect the lag and focus failure is not related to the type of control.

@jcsteh
Copy link
Owner Author

jcsteh commented Aug 24, 2022

Thanks for your testing regardless.

@jcsteh
Copy link
Owner Author

jcsteh commented Aug 24, 2022

Actually, I guess it's possible the lag is accessibility events being fired. Hmm.

@ptorpey
Copy link

ptorpey commented Aug 24, 2022 via email

@ptorpey
Copy link

ptorpey commented Oct 11, 2022 via email

@jcsteh
Copy link
Owner Author

jcsteh commented Oct 11, 2022

@ptorpey, that wouldn't be addressed by this fix, even assuming it worked satisfactorily. That is due to the plugin not supporting reporting of friendly values for any value other than the current one, so OSARA can't get the friendly value until after the value has been changed in the plugin. That also prevents OSARA from snapping to the next discrete value if the parameter is not entirely continuous.

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.

Parameters list not refreshed when parameter changes affect other parameters
5 participants