From 4c9d5daa29e50b606ea0239e5ea6fc56de80d99b Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 6 Dec 2021 11:09:15 -0500 Subject: [PATCH] Handle NAN and INF from Formula (#5576) NaN and Inf from formula would rip through the engine and blow through various checks and so on. So inside formula do an explicit std::isfinite and set a flag if they return these. Also add a unit test of the behavior. Closes #5566 --- .../modulators/FormulaModulationHelper.cpp | 16 ++++- .../dsp/modulators/FormulaModulationHelper.h | 1 + src/surge-testrunner/UnitTestsLUA.cpp | 72 ++++++++++++++++++- .../gui/widgets/LFOAndStepDisplay.cpp | 21 ++++++ 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/common/dsp/modulators/FormulaModulationHelper.cpp b/src/common/dsp/modulators/FormulaModulationHelper.cpp index 16ae814f6c0..ad515ac9111 100644 --- a/src/common/dsp/modulators/FormulaModulationHelper.cpp +++ b/src/common/dsp/modulators/FormulaModulationHelper.cpp @@ -525,12 +525,22 @@ void valueAt(int phaseIntPart, float phaseFracPart, FormulaModulatorStorage *fs, // stack is now just the result if (lres == LUA_OK) { + s->isFinite = true; + auto checkFinite = [s](float f) { + if (!std::isfinite(f)) + { + s->isFinite = false; + return 0.f; + } + return f; + }; + if (lua_isnumber(s->L, -1)) { // OK so you returned a value. Just use it auto r = lua_tonumber(s->L, -1); lua_pop(s->L, 1); - output[0] = r; + output[0] = checkFinite(r); return; } if (!lua_istable(s->L, -1)) @@ -552,7 +562,7 @@ void valueAt(int phaseIntPart, float phaseFracPart, FormulaModulatorStorage *fs, float res = 0.0; if (lua_isnumber(s->L, -1)) { - output[0] = lua_tonumber(s->L, -1); + output[0] = checkFinite(lua_tonumber(s->L, -1)); } else if (lua_istable(s->L, -1)) { @@ -574,7 +584,7 @@ void valueAt(int phaseIntPart, float phaseFracPart, FormulaModulatorStorage *fs, } // Remember - LUA is 0 based - output[idx - 1] = lua_tonumber(s->L, -1); + output[idx - 1] = checkFinite(lua_tonumber(s->L, -1)); lua_pop(s->L, 1); len = std::max(len, idx - 1); } diff --git a/src/common/dsp/modulators/FormulaModulationHelper.h b/src/common/dsp/modulators/FormulaModulationHelper.h index c203b0a2d75..22a97235c04 100644 --- a/src/common/dsp/modulators/FormulaModulationHelper.h +++ b/src/common/dsp/modulators/FormulaModulationHelper.h @@ -38,6 +38,7 @@ struct EvaluatorState bool isvalid = false; bool useEnvelope = true; + bool isFinite = true; bool subVoice{false}, subLfoParams{true}, subLfoEnvelope{false}, subTiming{true}; bool subMacros[n_customcontrollers], subAnyMacro{false}; diff --git a/src/surge-testrunner/UnitTestsLUA.cpp b/src/surge-testrunner/UnitTestsLUA.cpp index 7899a737a95..09bc6c2afd4 100644 --- a/src/surge-testrunner/UnitTestsLUA.cpp +++ b/src/surge-testrunner/UnitTestsLUA.cpp @@ -826,4 +826,74 @@ TEST_CASE("Macros Are Available", "[formula]") auto pitchId = surge->storage.getPatch().scene[0].osc[0].pitch.id; surge->setModulation(pitchId, ms_lfo1, 0, 0, 0.1); REQUIRE(1); -} \ No newline at end of file +} + +TEST_CASE("Nan Clampsr", "[formula]") +{ + SECTION("Nan Formula") + { + auto surge = Surge::Test::surgeOnSine(); + surge->storage.getPatch().scene[0].lfo[0].shape.val.i = lt_formula; + auto pitchId = surge->storage.getPatch().scene[0].osc[0].pitch.id; + surge->setModulation(pitchId, ms_lfo1, 0, 0, 0.1); + + surge->storage.getPatch().formulamods[0][0].setFormula(R"FN( +function init(modstate) + modstate["depth"] = 0 + return modstate +end + +function process(modstate) + modstate["output"] = (modstate["phase"] * 2 - 1) / modstate["depth" ] + return modstate +end)FN"); + for (int i = 0; i < 10; ++i) + surge->process(); + + surge->playNote(0, 60, 100, 0); + for (int i = 0; i < 10; ++i) + surge->process(); + + REQUIRE(!surge->voices[0].empty()); + auto ms = surge->voices[0].front()->modsources[ms_lfo1]; + REQUIRE(ms); + auto lms = dynamic_cast(ms); + REQUIRE(lms); + + REQUIRE(lms->get_output(0) == 0.f); + REQUIRE(!lms->formulastate.isFinite); + } + + SECTION("Not Nan Formula") + { + auto surge = Surge::Test::surgeOnSine(); + surge->storage.getPatch().scene[0].lfo[0].shape.val.i = lt_formula; + auto pitchId = surge->storage.getPatch().scene[0].osc[0].pitch.id; + surge->setModulation(pitchId, ms_lfo1, 0, 0, 0.1); + + surge->storage.getPatch().formulamods[0][0].setFormula(R"FN( +function init(modstate) + modstate["depth"] = 1 + return modstate +end + +function process(modstate) + modstate["output"] = (modstate["phase"] * 2 - 1) / modstate["depth" ] + return modstate +end)FN"); + for (int i = 0; i < 10; ++i) + surge->process(); + + surge->playNote(0, 60, 100, 0); + for (int i = 0; i < 10; ++i) + surge->process(); + + REQUIRE(!surge->voices[0].empty()); + auto ms = surge->voices[0].front()->modsources[ms_lfo1]; + REQUIRE(ms); + auto lms = dynamic_cast(ms); + REQUIRE(lms); + + REQUIRE(lms->formulastate.isFinite); + } +} diff --git a/src/surge-xt/gui/widgets/LFOAndStepDisplay.cpp b/src/surge-xt/gui/widgets/LFOAndStepDisplay.cpp index f3e95309d78..a3875186b81 100644 --- a/src/surge-xt/gui/widgets/LFOAndStepDisplay.cpp +++ b/src/surge-xt/gui/widgets/LFOAndStepDisplay.cpp @@ -244,6 +244,9 @@ void LFOAndStepDisplay::paintWaveform(juce::Graphics &g) float priorval = 0.f, priorwval = 0.f; + bool warnForInvalid{false}; + std::string invalidMessage; + for (int i = 0; i < totalSamples; i += averagingWindow) { float val = 0; @@ -263,6 +266,16 @@ void LFOAndStepDisplay::paintWaveform(juce::Graphics &g) tFullWave->process_block(); } + if (lfodata->shape.val.i == lt_formula) + { + if (!tlfo->formulastate.isFinite || + (tFullWave && !tFullWave->formulastate.isFinite)) + { + warnForInvalid = true; + invalidMessage = "Formula produced nan or inf"; + } + } + if (susCountdown < 0 && tlfo->env_state == lfoeg_stuck) { susCountdown = susTime * samplerate / BLOCK_SIZE; @@ -606,6 +619,14 @@ void LFOAndStepDisplay::paintWaveform(juce::Graphics &g) dc->drawLine(sp, ep); #endif } + + if (warnForInvalid) + { + g.setColour(skin->getColor(Colors::LFO::Waveform::Wave)); + g.setFont(Surge::GUI::getFontManager()->getLatoAtSize(14, juce::Font::bold)); + g.drawText(invalidMessage, waveform_display.withTrimmedBottom(30), + juce::Justification::centred); + } } void LFOAndStepDisplay::paintStepSeq(juce::Graphics &g)