surge-fx: replace AccSlider with sst/jucegui/Knob.h #7928
Conversation
|
I converted to draft to avoid accidental merge. Have fun! |
There was a problem hiding this comment.
You shouldn't need to implement a stylesheet, just start with one of the built ins and customize
You also shouldn't need a style sheet just for the knobs. Instead give them a custom class if you need differentiated knobs, or just use the knob properties from the parent.
src/surge-fx/KnobSource.h
Outdated
| @@ -36,4 +40,298 @@ struct ConcreteCM : sst::jucegui::data::ContinuousModulatable | |||
| float getModulationValuePM1() const override { return mv; } | |||
| void setModulationValuePM1(const float &f) override { mv = f; } | |||
| bool isModulationBipolar() const override { return isBipolar(); } // sure why not | |||
| }; | |||
|
|
|||
| struct StyleSheetBuiltInImp : public sst::jucegui::style::StyleSheet | |||
There was a problem hiding this comment.
Ooooh we shouldn't need all this code. You should be able to start with the Dark and customize colors from there.
There was a problem hiding this comment.
There was a problem hiding this comment.
example makes sense to me. Will update tmrw.
src/surge-fx/SurgeFXEditor.cpp
Outdated
|
|
||
| setTransform(juce::AffineTransform().scaled(1.0)); | ||
|
|
||
| // @ BP this compiles but causes a segfault on launc |
There was a problem hiding this comment.
@baconpaul this is what I think is supposed to happen but JUCE segfaults on isShowing(). I reverted to L153 instead.
I think once we resolve this issue i can move on to updating the param values.
There was a problem hiding this comment.
I dug into it more. Its segfaulting here https://github.com/surge-synthesizer/sst-jucegui/blob/main/src/sst/jucegui/style/StyleAndSettingsConsumer.cpp#L49
There was a problem hiding this comment.
Well that should be in the if but also should not be null
did you make a non component the consumer?
There was a problem hiding this comment.
I made the editor a consumer, which is a component. My limited understanding is that the component is null when setting the style which doesn't make sense to me either.
If I made a non component the consumer, i think it should have failed to compile?
In my std::cout testing, it failed to even enter the isShowing().
I thought it might have been an issue with https://github.com/juce-framework/JUCE/blob/master/modules/juce_gui_basics/components/juce_Component.cpp#L342 But testing showed that it faulted on the entry of isShowing() from sst-jucegui.
|
There was a problem hiding this comment.
This all looks good.
I had one question about an API point
And the thing I would test also is: when you do external VST automation of a param in reaper, does the knob repaint get called properly?
But with both those things I am very happy to merge this change!
|
did my final pass through and testing. lgtm 🚢 |
This PR switches from
juce::Sliderwithsst-juceguiknobs.it is a follow up on #7924
Extra changes
update sst-jucegui
Testing