From 2fafad9519019dde6d644b5aaa25e26e9c341797 Mon Sep 17 00:00:00 2001 From: Paul Walker Date: Sat, 21 Nov 2020 11:01:07 -0500 Subject: [PATCH] Fix a set of Automation problems 1. Automation would fill the refresh_parameter if many messages came between idle, which previously would drop messages, but now would over-refresh. Two updates on a param between idles is the same as one 2. Rounding and integer handling in the idle loop was incorrect for fm and filterblock activation 3. In the event of overflow, the wrong codepath ran mis-reactivating some sliders. So consolidate the all-changed codepath and the some-changed codepath in idle. Closes #3096 --- src/common/Parameter.h | 7 + src/common/SurgeSynthesizer.cpp | 2 +- src/common/dsp/effect/DualDelayEffect.cpp | 2 +- src/common/gui/SurgeGUIEditor.cpp | 258 ++++++++++------------ 4 files changed, 131 insertions(+), 138 deletions(-) diff --git a/src/common/Parameter.h b/src/common/Parameter.h index cfa4177bba8..6a1b5e37f9c 100644 --- a/src/common/Parameter.h +++ b/src/common/Parameter.h @@ -419,6 +419,13 @@ class Parameter ** if( storage && storage->isStandardTuning ) { } */ SurgeStorage *storage = nullptr; + + static inline float intScaledToFloat( int v, int vmax, int vmin = 0 ) { + return 0.005 + 0.99 * ((float)(v - vmin)) / ((float)(vmax - vmin)); + } + static inline int intUnscaledFromFloat( float f, int vmax, int vmin = 0 ) { + return (int)((1 / 0.99) * (f - 0.005) * (float)(vmax - vmin) + 0.5) + vmin; + } }; // I don't make this a member since param needs to be copyable with memcpy. diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp index 3bad6c4ff8d..d3028faf928 100644 --- a/src/common/SurgeSynthesizer.cpp +++ b/src/common/SurgeSynthesizer.cpp @@ -1698,7 +1698,7 @@ bool SurgeSynthesizer::setParameter01(long index, float value, bool external, bo bool got = false; for (int i = 0; i < 8; i++) { - if (refresh_parameter_queue[i] < 0) + if (refresh_parameter_queue[i] < 0 || refresh_parameter_queue[i] == index ) { refresh_parameter_queue[i] = index; got = true; diff --git a/src/common/dsp/effect/DualDelayEffect.cpp b/src/common/dsp/effect/DualDelayEffect.cpp index ce4db798d17..9634cbfb33c 100644 --- a/src/common/dsp/effect/DualDelayEffect.cpp +++ b/src/common/dsp/effect/DualDelayEffect.cpp @@ -22,7 +22,7 @@ void DualDelayEffect::init() lfophase = 0.0; ringout_time = 100000; envf = 0.f; - LFOval = 0,f; + LFOval = 0.f; LFOdirection = true; lp.suspend(); hp.suspend(); diff --git a/src/common/gui/SurgeGUIEditor.cpp b/src/common/gui/SurgeGUIEditor.cpp index 109586173e7..98082e12b01 100644 --- a/src/common/gui/SurgeGUIEditor.cpp +++ b/src/common/gui/SurgeGUIEditor.cpp @@ -704,181 +704,167 @@ void SurgeGUIEditor::idle() } } + std::vector refreshIndices; if( synth->refresh_overflow ) { - // Basicall yreset everything and repaint. - synth->refresh_overflow = false; + refreshIndices.resize( n_total_params ); + std::iota (std::begin(refreshIndices), std::end(refreshIndices), 0); + frame->invalid(); + } + else + { for( int i=0; i<8; ++i ) - synth->refresh_parameter_queue[i] = -1; - for( int i=0; irefresh_parameter_queue[i] >= 0 ) + refreshIndices.push_back(synth->refresh_parameter_queue[i]); + } + + synth->refresh_overflow = false; + for( int i=0; i<8; ++i ) + synth->refresh_parameter_queue[i] = -1; + + + for( auto j : refreshIndices ) + { + if ((j < n_total_params) && param[j]) { - auto p = param[i]; - if( ! p ) p = nonmod_param[i]; - if( p ) + SurgeSynthesizer::ID jid; + if( synth->fromSynthSideId(j, jid )) + param[j]->setValue(synth->getParameter01(jid)); + frame->invalidRect(param[j]->getViewSize()); + + if( oscdisplay ) { - SurgeSynthesizer::ID jid; - if( synth->fromSynthSideId(i, jid )) - if( synth->getParameter01(jid) != p->getValue() ) - p->setValue(synth->getParameter01(jid)); + ((COscillatorDisplay*)oscdisplay)->invalidateIfIdIsInRange(j); + } + + if( lfodisplay ) + { + ((CLFOGui*)lfodisplay)->invalidateIfIdIsInRange(j); } } - for( int i=0; i= metaparam_offset) && (j < (metaparam_offset + n_customcontrollers))) { - gui_modsrc[ms_ctrl1 + i]->setValue( - ((ControllerModulationSource*)synth->storage.getPatch() - .scene[current_scene].modsources[ms_ctrl1 + i])->get_target01()); - + int cc = j - metaparam_offset; + gui_modsrc[ms_ctrl1 + cc]->setValue( + ((ControllerModulationSource*)synth->storage.getPatch() + .scene[current_scene].modsources[ms_ctrl1 + cc])->get_target01()); } - frame->invalid(); - } - else for (int i = 0; i < 8; i++) - { - if (synth->refresh_parameter_queue[i] >= 0) + else if((j>=0) && (j < n_total_params) && nonmod_param[j]) { - int j = synth->refresh_parameter_queue[i]; - synth->refresh_parameter_queue[i] = -1; - if ((j < n_total_params) && param[j]) + /* + ** What the heck is this NONMOD_PARAM thing? + ** + ** There are a set of params - like discrete things like + ** octave and filter type - which are not LFO modulatable + ** and aren't in the params[] array. But they are exposed + ** properties, so you can control them from a DAW. The + ** DAW control works - everything up to this path (as described + ** in #160) works fine and sets the value but since there's + ** no CControl in param the above bails out. But ading + ** all these controls to param[] would have the unintended + ** side effect of giving them all the other param[] behaviours. + ** So have a second array and drop select items in here so we + ** can actually get them redrawing when an external param set occurs. + */ + CControl *cc = nonmod_param[ j ]; + + /* + ** Some state changes enable and disable sliders. If this is one of those state changes and a value has changed, then + ** we need to invalidate them. See #2056. + */ + auto tag = cc->getTag(); + SurgeSynthesizer::ID jid; + + auto sv = 0.f; + if( synth->fromSynthSideId(j, jid )) + sv = synth->getParameter01(jid); + auto cv = cc->getValue(); + + if ((sv != cv) && ((tag == fmconfig_tag || tag == filterblock_tag))) { - SurgeSynthesizer::ID jid; - if( synth->fromSynthSideId(j, jid )) - param[j]->setValue(synth->getParameter01(jid)); - frame->invalidRect(param[j]->getViewSize()); + std::unordered_map resetMap; - if( oscdisplay ) + if (tag == fmconfig_tag) { - ((COscillatorDisplay*)oscdisplay)->invalidateIfIdIsInRange(j); + auto targetTag = synth->storage.getPatch().scene[current_scene].fm_depth.id + start_paramtags; + auto targetState = (Parameter::intUnscaledFromFloat(sv , n_fm_configuration - 1) == fm_off); + resetMap[targetTag] = targetState; } - if( lfodisplay ) + if (tag == filterblock_tag) { - ((CLFOGui*)lfodisplay)->invalidateIfIdIsInRange(j); - } - - } - else if ((j >= metaparam_offset) && (j < (metaparam_offset + n_customcontrollers))) - { - int cc = j - metaparam_offset; - gui_modsrc[ms_ctrl1 + cc]->setValue( - ((ControllerModulationSource*)synth->storage.getPatch() - .scene[current_scene].modsources[ms_ctrl1 + i])->get_target01()); - } - else if((j>=0) && (j < n_total_params) && nonmod_param[j]) - { - /* - ** What the heck is this NONMOD_PARAM thing? - ** - ** There are a set of params - like discrete things like - ** octave and filter type - which are not LFO modulatable - ** and aren't in the params[] array. But they are exposed - ** properties, so you can control them from a DAW. The - ** DAW control works - everything up to this path (as described - ** in #160) works fine and sets the value but since there's - ** no CControl in param the above bails out. But ading - ** all these controls to param[] would have the unintended - ** side effect of giving them all the other param[] behaviours. - ** So have a second array and drop select items in here so we - ** can actually get them redrawing when an external param set occurs. - */ - CControl *cc = nonmod_param[ j ]; + auto pval = Parameter::intUnscaledFromFloat(sv, n_fb_configuration - 1 ); - /* - ** Some state changes enable and disable sliders. If this is one of those state changes and a value has changed, then - ** we need to invalidate them. See #2056. - */ - auto tag = cc->getTag(); - SurgeSynthesizer::ID jid; + auto targetTag = synth->storage.getPatch().scene[current_scene].feedback.id + start_paramtags; + auto targetState = (pval == fb_serial); + resetMap[targetTag] = targetState; - auto sv = 0.f; - if( synth->fromSynthSideId(j, jid )) - sv = synth->getParameter01(jid); - auto cv = cc->getValue(); + targetTag = synth->storage.getPatch().scene[current_scene].width.id + start_paramtags; + targetState = (pval != fb_stereo && pval != fb_wide); + resetMap[targetTag] = targetState; + } - if ((sv != cv) && ((tag == fmconfig_tag || tag == filterblock_tag))) + for (int i = 0; i < n_paramslots; i++) { - std::unordered_map resetMap; - - if (tag == fmconfig_tag) - { - auto targetTag = synth->storage.getPatch().scene[current_scene].fm_depth.id + start_paramtags; - auto targetState = (round(sv * n_fm_configuration) == fm_off); - resetMap[targetTag] = targetState; - } - - if (tag == filterblock_tag) + if (param[i] && (resetMap.find(param[i]->getTag()) != resetMap.end())) { - auto pval = round(sv * n_fb_configuration); + auto css = dynamic_cast(param[i]); - auto targetTag = synth->storage.getPatch().scene[current_scene].feedback.id + start_paramtags; - auto targetState = (pval == fb_serial); - resetMap[targetTag] = targetState; - - targetTag = synth->storage.getPatch().scene[current_scene].width.id + start_paramtags; - targetState = (pval != fb_stereo && pval != fb_wide); - resetMap[targetTag] = targetState; - } - - for (int i = 0; i < n_paramslots; i++) - { - if (param[i] && (resetMap.find(param[i]->getTag()) != resetMap.end())) + if (css) { - auto css = dynamic_cast(param[i]); - - if (css) - { - css->disabled = resetMap[param[i]->getTag()]; - css->invalid(); - } + css->disabled = resetMap[param[i]->getTag()]; + css->invalid(); } } } + } #if TARGET_VST2 - /* - ** This is a gross hack. The right thing is to have a remapper lambda on the control. - ** But for now we have this. The VST2 calls back into here when you setvalue to (basically) - ** double set value. But for the scenemod this means that the transformation doesn't occur - ** so you get a dance. Since we don't really care if scenemode is automatable for now we just do - */ - if( synth->storage.getPatch().param_ptr[j]->ctrltype != ct_scenemode ) - cc->setValue(synth->getParameter01(jid)); -#else + /* + ** This is a gross hack. The right thing is to have a remapper lambda on the control. + ** But for now we have this. The VST2 calls back into here when you setvalue to (basically) + ** double set value. But for the scenemod this means that the transformation doesn't occur + ** so you get a dance. Since we don't really care if scenemode is automatable for now we just do + */ + if( synth->storage.getPatch().param_ptr[j]->ctrltype != ct_scenemode ) cc->setValue(synth->getParameter01(jid)); +#else + cc->setValue(synth->getParameter01(jid)); #endif - // Integer switches also work differently - auto assw = dynamic_cast(cc); - if( assw ) + // Integer switches also work differently + auto assw = dynamic_cast(cc); + if( assw ) + { + if( assw->is_itype ) { - if( assw->is_itype ) - { - assw->ivalue = synth->storage.getPatch().param_ptr[j]->val.i + 1; - } + assw->ivalue = synth->storage.getPatch().param_ptr[j]->val.i + 1; } - - cc->setDirty(); - cc->invalid(); } + + cc->setDirty(); + cc->invalid(); + } #if 0 - /* - ** A set of special things which invalidate the entire UI. - ** See #2226 for why this is currently off. - */ - else if( j == synth->storage.getPatch().scene_active.id ) + /* + ** A set of special things which invalidate the entire UI. + ** See #2226 for why this is currently off. + */ + else if( j == synth->storage.getPatch().scene_active.id ) + { + if( current_scene != synth->storage.getPatch().scene_active.val.i ) { - if( current_scene != synth->storage.getPatch().scene_active.val.i ) - { - synth->refresh_editor = true; - current_scene = synth->storage.getPatch().scene_active.val.i; - } - + synth->refresh_editor = true; + current_scene = synth->storage.getPatch().scene_active.val.i; } + + } #endif - else - { - // printf( "Bailing out of all possible refreshes on %d\n", j ); - // This is not really a problem - } + else + { + // printf( "Bailing out of all possible refreshes on %d\n", j ); + // This is not really a problem } } for (int i = 0; i < n_customcontrollers; i++)