Skip to content

Commit

Permalink
Correct Const in get_extended (#4865)
Browse files Browse the repository at this point in the history
get_extended was mutating the state of Parameter; rather than
do that add a set_extend_range and call it consistently. Add
but don't activate a mode where you can force a write to extend
into a compiler error also

Closes #4861
Addresses #3808
  • Loading branch information
baconpaul authored Aug 18, 2021
1 parent 4d55c18 commit bb1b247
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/common/FxPresetAndClipboardManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ void loadPresetOnto(const Preset &p, SurgeStorage *storage, FxStorage *fxbuffer)
break;
}
fxbuffer->p[i].temposync = (int)p.ts[i];
fxbuffer->p[i].extend_range = (int)p.er[i];
fxbuffer->p[i].set_extend_range((int)p.er[i]);
fxbuffer->p[i].deactivated = (int)p.da[i];

if (p.dt[i] >= 0) // only set deform type if it could be read from the xml
Expand Down Expand Up @@ -411,7 +411,7 @@ void pasteFx(SurgeStorage *storage, FxStorage *fxbuffer, Clipboard &cb)
break;
}
fxbuffer->p[i].temposync = (int)cb.fxCopyPaste[tp];
fxbuffer->p[i].extend_range = (int)cb.fxCopyPaste[xp];
fxbuffer->p[i].set_extend_range((int)cb.fxCopyPaste[xp]);
fxbuffer->p[i].deactivated = (int)cb.fxCopyPaste[dp];

if (fxbuffer->p[i].has_deformoptions())
Expand Down
4 changes: 2 additions & 2 deletions src/common/ModulatorPresetManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ void loadPresetFrom(const fs::path &location, SurgeStorage *s, int scene, int lf
else
curr->deform_type = 0;
if (valNode->QueryIntAttribute("extend_range", &q) == TIXML_SUCCESS)
curr->extend_range = q;
curr->set_extend_range(q);
else
curr->extend_range = false;
curr->set_extend_range(false);
if (valNode->QueryIntAttribute("deactivated", &q) == TIXML_SUCCESS &&
curr->can_deactivate())
curr->deactivated = q;
Expand Down
56 changes: 39 additions & 17 deletions src/common/Parameter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ Parameter *Parameter::assign(ParameterIDCounter::promise_t idp, int pid, const c
void Parameter::clear_flags()
{
temposync = false;
extend_range = false;
set_extend_range(false);
absolute = false;
deactivated = true; // CHOICE: if you are a deactivatble parameter make it so you are by default
porta_constrate = false;
Expand Down Expand Up @@ -1849,8 +1849,14 @@ void Parameter::set_storage_value(float f)
}
}

float Parameter::get_extended(float f)
void Parameter::set_extend_range(bool er)
{
#if DEBUG_WRITABLE_EXTEND_RANGE
extend_range_internal = er;
#else
extend_range = er;
#endif

if (!extend_range)
{
switch (ctrltype)
Expand All @@ -1859,41 +1865,58 @@ float Parameter::get_extended(float f)
{
// Why the heck are we modifying this here?
val_max.f = -6.6305f; // 300 Hz
return f;
}
break;
case ct_freq_reson_band2:
{
val_min.f = -6.6305f; // 300 Hz
val_max.f = 21.23265f; // 1500 Hz
return f;
}
break;
case ct_freq_reson_band3:
{
val_min.f = 21.23265f; // 1500 Hz
return f;
}
break;
case ct_lfophaseshuffle:
{
val_default.f = 0.f;
return f;
}
break;
default:
break;
}
}
else
{
switch (ctrltype)
{
case ct_freq_reson_band1:
case ct_freq_reson_band2:
case ct_freq_reson_band3:
{
val_min.f = -34.4936f; // 60 Hz
val_max.f = 49.09578; // 7500 Hz
}
break;
case ct_lfophaseshuffle:
{
return f;
val_default.f = 0.5f;
}
break;
}
}
}

switch (ctrltype)
{
case ct_freq_reson_band1:
case ct_freq_reson_band2:
case ct_freq_reson_band3:
float Parameter::get_extended(float f) const
{
if (!extend_range)
{
val_min.f = -34.4936f; // 60 Hz
val_max.f = 49.09578; // 7500 Hz
return f;
}

switch (ctrltype)
{
case ct_freq_shift:
return 100.f * f;
case ct_pitch_semi7bp:
Expand All @@ -1916,7 +1939,6 @@ float Parameter::get_extended(float f)
return (2.f * f) - 1.f;
case ct_lfophaseshuffle:
{
val_default.f = 0.5f;
return (2.f * f) - 1.f;
}
case ct_fmratio:
Expand Down Expand Up @@ -2035,7 +2057,7 @@ std::string Parameter::tempoSyncNotationValue(float f) const

void Parameter::get_display_of_modulation_depth(char *txt, float modulationDepth, bool isBipolar,
ModulationDisplayMode displaymode,
ModulationDisplayInfoWindowStrings *iw)
ModulationDisplayInfoWindowStrings *iw) const
{
int detailedMode = false;

Expand Down Expand Up @@ -2826,7 +2848,7 @@ void Parameter::get_display_alt(char *txt, bool external, float ef) const
}
}

void Parameter::get_display(char *txt, bool external, float ef)
void Parameter::get_display(char *txt, bool external, float ef) const
{
if (ctrltype == ct_none)
{
Expand Down
18 changes: 14 additions & 4 deletions src/common/Parameter.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ class Parameter
const char *get_internal_name() const;
const char *get_storage_name() const;
const wchar_t *getUnit() const;
void get_display(char *txt, bool external = false, float ef = 0.f);
void get_display(char *txt, bool external = false, float ef = 0.f) const;
enum ModulationDisplayMode
{
TypeIn,
Expand All @@ -406,19 +406,20 @@ class Parameter

void get_display_of_modulation_depth(char *txt, float modulationDepth, bool isBipolar,
ModulationDisplayMode mode,
ModulationDisplayInfoWindowStrings *iw = nullptr);
ModulationDisplayInfoWindowStrings *iw = nullptr) const;
void get_display_alt(char *txt, bool external = false, float ef = 0.f) const;
char *get_storage_value(char *) const;
void set_storage_value(int i);
void set_storage_value(float f);
float get_extended(float f);
float get_extended(float f) const;
float get_value_f01() const;
float normalized_to_value(float value) const;
float value_to_normalized(float value) const;
float get_default_value_f01() const;
void set_value_f01(float v, bool force_integer = false);
bool set_value_from_string(const std::string &s);
bool set_value_from_string_onto(const std::string &s, pdata &ontoThis);
void set_extend_range(bool er);

/*
* These two functions convert the modulation depth to a -1,1 range appropriate
Expand Down Expand Up @@ -458,7 +459,16 @@ class Parameter
bool affect_other_parameters{};
float moverate{};
bool per_voice_processing{};
bool temposync{}, extend_range{}, absolute{}, deactivated{};
bool temposync{}, absolute{}, deactivated{};

#define DEBUG_WRITABLE_EXTEND_RANGE 0
#if DEBUG_WRITABLE_EXTEND_RANGE
bool extend_range_internal{};
const bool &extend_range = extend_range_internal;
#else
bool extend_range{};
#endif

bool porta_constrate{}, porta_gliss{}, porta_retrigger{};
int porta_curve{};
int deform_type{};
Expand Down
8 changes: 4 additions & 4 deletions src/common/SurgePatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1293,15 +1293,15 @@ void SurgePatch::load_xml(const void *data, int datasize, bool is_preset)
if (p->QueryIntAttribute("extend_range", &j) == TIXML_SUCCESS)
{
if (j == 1)
param_ptr[i]->extend_range = true;
param_ptr[i]->set_extend_range(true);
else
param_ptr[i]->extend_range = false;
param_ptr[i]->set_extend_range(false);
}
else
{
param_ptr[i]->extend_range = false;
param_ptr[i]->set_extend_range(false);
if (revision >= 16 && param_ptr[i]->ctrltype == ct_percent_oscdrift)
param_ptr[i]->extend_range = true;
param_ptr[i]->set_extend_range(true);
}

if ((p->QueryIntAttribute("absolute", &j) == TIXML_SUCCESS) && (j == 1))
Expand Down
4 changes: 2 additions & 2 deletions src/common/SurgeStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ SurgeStorage::SurgeStorage(std::string suppliedDataPath) : otherscene_clients(0)

for (int s = 0; s < n_scenes; ++s)
{
getPatch().scene[s].drift.extend_range = true;
getPatch().scene[s].drift.set_extend_range(true);
}

bool mtsMode = Surge::Storage::getUserDefaultValue(this, Surge::Storage::UseODDMTS, false);
Expand Down Expand Up @@ -1476,7 +1476,7 @@ void SurgeStorage::clipboard_paste(int type, int scene, int entry)
int pid = p.id + id;
getPatch().param_ptr[pid]->val.i = p.val.i;
getPatch().param_ptr[pid]->temposync = p.temposync;
getPatch().param_ptr[pid]->extend_range = p.extend_range;
getPatch().param_ptr[pid]->set_extend_range(p.extend_range);
getPatch().param_ptr[pid]->deactivated = p.deactivated;
getPatch().param_ptr[pid]->porta_constrate = p.porta_constrate;
getPatch().param_ptr[pid]->porta_gliss = p.porta_gliss;
Expand Down
4 changes: 2 additions & 2 deletions src/common/SurgeSynthesizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2381,7 +2381,7 @@ bool SurgeSynthesizer::loadOscalgos()

if (e->QueryIntAttribute(lbl, &j) == TIXML_SUCCESS)
{
storage.getPatch().scene[s].osc[i].p[k].extend_range = j;
storage.getPatch().scene[s].osc[i].p[k].set_extend_range(j);
}
}

Expand Down Expand Up @@ -3986,7 +3986,7 @@ void SurgeSynthesizer::reorderFx(int source, int target, FXReorderMode m)
auto cp = [](Parameter &to, const Parameter &from) {
to.val = from.val;
to.temposync = from.temposync;
to.extend_range = from.extend_range;
to.set_extend_range(from.extend_range);
to.deactivated = from.deactivated;
to.absolute = from.absolute;
};
Expand Down
4 changes: 2 additions & 2 deletions src/common/dsp/effects/DistortionEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ void DistortionEffect::handleStreamingMismatches(int streamingRevision,
if (streamingRevision <= 11)
{
fxdata->p[dist_model].val.i = 0;
fxdata->p[dist_preeq_gain].extend_range = false;
fxdata->p[dist_posteq_gain].extend_range = false;
fxdata->p[dist_preeq_gain].set_extend_range(false);
fxdata->p[dist_posteq_gain].set_extend_range(false);
}

if (streamingRevision <= 15)
Expand Down
2 changes: 1 addition & 1 deletion src/common/dsp/oscillators/StringOscillator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ void StringOscillator::init_default_values()
oscdata->p[str_str2_decay].val.f = 0.95f;

oscdata->p[str_str2_detune].val.f = 0.1f;
oscdata->p[str_str2_detune].extend_range = false;
oscdata->p[str_str2_detune].set_extend_range(false);
oscdata->p[str_str_balance].val.f = 0.f;

oscdata->p[str_stiffness].val.f = 0.f;
Expand Down
2 changes: 1 addition & 1 deletion src/common/dsp/oscillators/TwistOscillator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ void TwistOscillator::init_default_values()
oscdata->p[twist_timbre].val.f = 0.f;
oscdata->p[twist_morph].val.f = 0.f;
oscdata->p[twist_aux_mix].val.f = -1.f;
oscdata->p[twist_aux_mix].extend_range = false;
oscdata->p[twist_aux_mix].set_extend_range(false);
oscdata->p[twist_lpg_response].val.f = 0.f;
oscdata->p[twist_lpg_response].deactivated = true;
oscdata->p[twist_lpg_decay].val.f = 0.f;
Expand Down
4 changes: 2 additions & 2 deletions src/common/dsp/oscillators/WavetableOscillator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ void WavetableOscillator::init_ctrltypes()
void WavetableOscillator::init_default_values()
{
oscdata->p[wt_morph].val.f = 0.0f;
oscdata->p[wt_morph].extend_range = true;
oscdata->p[wt_morph].set_extend_range(true);
oscdata->p[wt_skewv].val.f = 0.0f;
oscdata->p[wt_saturate].val.f = 0.f;
oscdata->p[wt_formant].val.f = 0.f;
Expand Down Expand Up @@ -539,6 +539,6 @@ void WavetableOscillator::handleStreamingMismatches(int streamingRevision,
{
if (streamingRevision <= 16)
{
oscdata->p[wt_morph].extend_range = true;
oscdata->p[wt_morph].set_extend_range(true);
}
}
2 changes: 1 addition & 1 deletion src/gui/SurgeGUIEditorValueCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ int32_t SurgeGUIEditor::controlModifierClicked(Surge::GUI::IComponentTagValue *c

contextMenu.addItem(Surge::GUI::toOSCaseForMenu(txt), enable, isChecked,
[this, p]() {
p->extend_range = !p->extend_range;
p->set_extend_range(!p->extend_range);
this->synth->refresh_editor = true;
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/gui/widgets/XMLConfiguredMenus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,8 @@ void FxMenu::loadSnapshot(int type, TiXmlElement *e, int idx)
fxbuffer->p[i].temposync =
((e->QueryIntAttribute(sublbl, &j) == TIXML_SUCCESS) && (j == 1));
snprintf(sublbl, TXT_SIZE, "p%i_extend_range", i);
fxbuffer->p[i].extend_range =
((e->QueryIntAttribute(sublbl, &j) == TIXML_SUCCESS) && (j == 1));
fxbuffer->p[i].set_extend_range(
((e->QueryIntAttribute(sublbl, &j) == TIXML_SUCCESS) && (j == 1)));
snprintf(sublbl, TXT_SIZE, "p%i_deactivated", i);
fxbuffer->p[i].deactivated =
((e->QueryIntAttribute(sublbl, &j) == TIXML_SUCCESS) && (j == 1));
Expand Down
2 changes: 1 addition & 1 deletion src/headless/UnitTestsDSP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ TEST_CASE("Unison Absolute and Relative", "[osc]")
REQUIRE(f72_1 / f60_1 == Approx(2).margin(0.01));

// test extended mode
surge->storage.getPatch().scene[0].osc[0].p[5].extend_range = true;
surge->storage.getPatch().scene[0].osc[0].p[5].set_extend_range(true);
auto f60_0e = frequencyForNote(surge, 60, 5, 0);
auto f60_1e = frequencyForNote(surge, 60, 5, 1);
REQUIRE(f60_0e / f60_avg == Approx(pow(f60_0 / f60_avg, 12.f)).margin(0.05));
Expand Down

0 comments on commit bb1b247

Please sign in to comment.