FR+PR: valueToText of AudioProcessorValueTreeState::Parameter accessible via SliderAttachment (DRY)

Hi everybody,
would it be possible to access the valueToTextFunction and textToValueFunction via the SliderAttachment?
What I want to achieve is an AttachableSlider which aggregates the AudioProcessorValueTreeState::SliderAttachment. That way:

  1. It saves copying the ScopedPointerAudioProcessorValueTreeState::SliderAttachment, which is 95% only created at the same time as the slider and also destroyed together
  2. The Slider could use the parameters’ textToValueFunction function as default for virtual double getValueFromText (const String &text) and remove the redundant implementation that you have to do in the Slider

The AttachableSlider could then simply provide a method similar to this outline

class AttachableSlider {
public:
    void attach (AudioProcessorValueTreeState& state, const StringRef paramID) {
        attachment = new AudioProcessorValueTreeState::SliderAttachment (state, paramID, this);
    }
    double getValueFromText (const String &text) override {
        if (attachment)
            return attachment->textToValueFunction (text);
        return Slider::getValueFromText (text);
    }
    String getTextFromValue (double value) override {
        if (attachment)
            return attachment->valueToTextFunction (value);
        return Slider::getTextFromValue (value);
    }
private:
    ScopedPointer<AudioProcessorValueTreeState::SliderAttachment> attachment;
};

That would lead to much cleaner code, adding DRY…

Let me know what you think
Cheers, daniel

1 Like

friendly bumb…

That seems sensible and it’s something we’ll look at, but it’s not particularly high priority so it will take a while to get around to it.

1 Like

Bump that, any chance?

I’ll better bump that now, since the parameters are being worked on…

Note that something like can be used done for any parameter, not specifically AudioProcessorValueTreeState based one. See the AudioProcessorParameterSlider here:

1 Like

Thanks Yair, I am aware of your post, that is a good idea to work around that problem. I have my own workaround with a Slider, that has all common text representations prepared.

But it’s still a workaround, I am not pushing it, but I think everybody would benefit from that addition, since I can’t think of any use case, where you want different settings for the slider textbox and the parameter.

But since the Attachment classes already forward settings from the parameter to the slider, like the range, it would make sense to do the same for the textToValue/valueToText lambdas as well.

BTW. if I would go down that route to subclass the Slider, I would even add the attachment as ScopedPointer, since after creation you never need to access that class anyway…

2 Likes

Wasn’t that hard after all, and didn’t expose too much of API internals.
Hope you like it:

Pinging @jules and @t0m - does that look like an option? Seems to work fine…

1 Like

N.B. I just realise, that the Slider uses doubles and the AudioProcessorParameters use float. I don’t think there would be too much objection to unify that to double on both ends…?

I’ve been looking at similar problems this week and the type difference certainly adds a layer of complexity to things. I’m not sure we want to change AudioProcessorParameters to doubles though - all the plug-in SDKs work with floats, as will the majority of people’s audio processing algorithms…

This doesn’t make this PR invalid though, you can easily cast when the lambda is called to avoid the warning…

Definitely better than having to override the Slider just to add custom text box behaviour, that the user already defined as lambda in the Processor class. It is not only to copy something, you even have to do the same thing in a different manner…

Even if AudioProcessor::supportsDoublePrecisionProcessing () is set? another tile to the complexity…

Here’s a step in the right direction (but solving a different problem):

Previously SliderAttachments ignored the convertFrom0to1, convertTo0to1, and snapToLegalValue lambdas.

I appreciate that you are working on the slider class, however I can’t see how this is related to the fact, that the same thing needs to be implemented differently on the parameters and in the slider, when a concept exists (AudioProcessorValueTreeState::SliderAttachment), that could bring them together…

Anyway, it was a proposal, I can’t force you to like it…

It’s loosely related - we are now passing lambda information from APVTS::Parameters to the Slider class, but as I said, solving a different problem: numerical values rather than strings.

We’re not ignoring your suggestion, we just haven’t evaluated it thoroughly yet!

1 Like
2 Likes

I get a

bad_function_call
#8 0x000000010bbaeb1a in std::__1::__throw_bad_function_call() [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/functional:1396
#9 0x000000010bbaead6 in std::__1::function<juce::String (float)>::operator()(float) const at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/functional:1902
#10 0x000000010bbb9280 in juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)::operator()(double) const at /Builds/MacOSX/…/…/juce/modules/juce_audio_processors/utilities/juce_AudioProcessorValueTreeState.cpp:469
#11 0x000000010bbb9238 in std::__1::__invoke<juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)&, double>(decltype(std::__1::forward<juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)&>(fp)(std::__1::forward(fp0))), juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)&&&, double&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/type_traits:4291
#12 0x000000010bbb9213 in std::__1::__invoke_void_return_wrapperjuce::String::__call<juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)&, double>(juce::String, juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)&&&, double&&) at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__functional_base:328
#13 0x000000010bbb90d0 in std::__1::__function::__func<juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double), std::__1::allocator<juce::AudioProcessorValueTreeState::SliderAttachment::Pimpl::Pimpl(juce::AudioProcessorValueTreeState&, juce::String const&, juce::Slider&)::‘lambda’(double)>, juce::String (double)>::operator()(double&&) at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/functional:1552
#14 0x000000010bf0a3a9 in std::__1::function<juce::String (double)>::operator()(double) const at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/functional:1903
#15 0x000000010bf0a1bf in juce::Slider::getTextFromValue(double) at /Builds/MacOSX/…/…/juce/modules/juce_gui_basics/widgets/juce_Slider.cpp:1543
#16 0x000000010bf08223 in juce::Slider::Pimpl::lookAndFeelChanged(juce::LookAndFeel&) at /Builds/MacOSX/…/…/juce/modules/juce_gui_basics/widgets/juce_Slider.cpp:551
#17 0x000000010bf06f9e in juce::Slider::lookAndFeelChanged() at /Builds/MacOSX/…/…/juce/modules/juce_gui_basics/widgets/juce_Slider.cpp:1475
#18 0x000000010bf07d14 in juce::Slider::Pimpl::setTextBoxStyle(juce::Slider::TextEntryBoxPosition, bool, int, int) at /Builds/MacOSX/…/…/juce/modules/juce_gui_basics/widgets/juce_Slider.cpp:497
#19 0x000000010bf07c2b in juce::Slider::setTextBoxStyle(juce::Slider::TextEntryBoxPosition, bool, int, int) at /Builds/MacOSX/…/…/juce/modules/juce_gui_basics/widgets/juce_Slider.cpp:1447

You have to check that param->textToValueFunction and param->valueToTextFunction are not null in there?

if (auto* param = dynamic_cast<AudioProcessorValueTreeState::Parameter*> (state.getParameter (paramID)))
{
    slider.valueFromTextFunction = [param] (const String& text) { return (double) param->textToValueFunction (text); };
    slider.textFromValueFunction = [param] (double value)       { return param->valueToTextFunction ((float) value); };

    slider.setDoubleClickReturnValue (true, range.convertFrom0to1 (param->getDefaultValue()));
}

Yes. Thanks for reporting!

A fix will appear once it has cleared our CI tests.

also, don’t you have to append the ‘label’ with getTextValueSuffix() at this line?

(because the parameter valueToText method should not include the label (dB, Hz…) as I understand it?)

If the suffix is static, that would be possible. I used the lambdas so far to include the label, since I also use that occasion e.g. to change Hz in kHz, like:

[](float value) { return (value < 1000) ?
                  String (value, 0) + " Hz" :
                  String (value / 1000.0, 2) + " kHz"; },
[](String text) { return text.endsWith(" kHz") ?
                  text.dropLastCharacters (4).getFloatValue() * 1000.0 :
                  text.dropLastCharacters (3).getFloatValue(); },

But I would then probably just leave the label blank, so it wouldn’t interfere.
As long as it is consistent with the way the parameter works…

but then don’t you have the label displayed twice in the host? (in the automations for instance)