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

VST3 rescans param names on params::request_rescan #289

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

baconpaul
Copy link
Collaborator

request_rescan with CLAP_PARAM_RESCAN_INFO may invalidate the name, module, or several other flags. The wrapper up until now just invalidated the value and re-ran the value to text.

Shortcircuit allows users to rename macros easily and the clap correctly calls params_rescan which results in bitwig re-displaying the clap name. The VST3 and AUv2 don't do this.

This commit fixes the VST3 with a partial correction and a comment. The partical correction is to re-acquire the parameter name in a RESCAN_INFO case. The comment explains that we do not re-acquire the module and build the entire tree.

Would appreciate careful review here. It fixes the shortcircuit bug but this is pretty fundamental code.

request_rescan with CLAP_PARAM_RESCAN_INFO may invalidate
the name, module, or several other flags. The wrapper up
until now just invalidated the value and re-ran the value
to text.

Shortcircuit allows users to rename macros easily and the clap
correctly calls params_rescan which results in bitwig re-displaying
the clap name. The VST3 and AUv2 don't do this.

This commit fixes the VST3 with a partial correction and a comment.
The partical correction is to re-acquire the parameter name
in a RESCAN_INFO case. The comment explains that we do not
re-acquire the module and build the entire tree.

Would appreciate careful review here. It fixes the shortcircuit bug
but this is pretty fundamental code.
if (flags & CLAP_PARAM_RESCAN_INFO)
{
// In this case, the name and module can also change.
// For now, don't rebuild the unit tree with modules but
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about checking for a change in the module name and rebuild in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about that but do we have the old module name around? Also can you rebuild unit trees in VST3?

Happy to look into both though!

Copy link
Collaborator

Choose a reason for hiding this comment

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

not yet, we could add it, though. but you can not request that, only a full reload of the component.

You can also remap parameters but only during project loading (kParamIDMappingChanged).. I remember that I decided back then that CLAPs shouldn't do so much weird things 😺

this does not really match the vst3 semantics. It is something that should probably avoided when you write a CLAP to be used with the wrapper.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah my view was 'modules changing is wierd names is common' hence the fix here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So @defiantnerd we already send those param change flags at line 906. This diff is really just rebuilding the internal cache of the params we have when they change.

I propose the following

  1. lets merge this. The common case of param names change is then taken care of and also we can easily test the unicode thing
  2. lets open an issue for module changes. That issue would contains
    • param has to cache the module name
    • param diffs the module name here
    • if that module name has diffed we issue a full reload, which is a bummer, and add that to the documentation

wdyt?

@defiantnerd defiantnerd merged commit 7bf6177 into free-audio:next Sep 25, 2024
19 checks passed
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.

2 participants