Skip to content

Commit

Permalink
test/refactor: arpgeggiator::midi_cv::Gate values roundtrip (Synthstr…
Browse files Browse the repository at this point in the history
…omAudible#2087)

- Pull out the guts for testing: the current value math is identical to
  standard, so share the machinery, but the final value computation
  produces numbers that are slightly smaller near the max values,
  and needs to remain distinct. They use-provided values roundtrip
  despite the asymmetry.

- Speculate on the reason things are the way they are.
  • Loading branch information
nikodemus committed Jun 10, 2024
1 parent a8cbf00 commit 7474d94
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 4 deletions.
7 changes: 3 additions & 4 deletions src/deluge/gui/menu_item/arpeggiator/midi_cv/gate.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
#pragma once
#include "gui/menu_item/integer.h"
#include "gui/menu_item/value_scaling.h"
#include "gui/ui/sound_editor.h"
#include "model/clip/instrument_clip.h"
#include "model/song/song.h"
Expand All @@ -25,12 +26,10 @@ class Gate final : public Integer {
public:
using Integer::Integer;
void readCurrentValue() override {
auto* current_clip = getCurrentInstrumentClip();
int64_t arp_gate = (int64_t)current_clip->arpeggiatorGate + 2147483648;
this->setValue((arp_gate * kMaxMenuValue + 2147483648) >> 32);
this->setValue(computeCurrentValueForArpMidiCvGate(getCurrentInstrumentClip()->arpeggiatorGate));
}
void writeCurrentValue() override {
getCurrentInstrumentClip()->arpeggiatorGate = (uint32_t)this->getValue() * 85899345 - 2147483648;
getCurrentInstrumentClip()->arpeggiatorGate = computeFinalValueForArpMidiCvGate(this->getValue());
}
[[nodiscard]] int32_t getMaxValue() const override { return kMaxMenuValue; }
bool isRelevant(ModControllableAudio* modControllable, int32_t whichThing) override {
Expand Down
8 changes: 8 additions & 0 deletions src/deluge/gui/menu_item/value_scaling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ int32_t computeCurrentValueForPan(int32_t value) {
return ((int64_t)value * (kMaxMenuRelativeValue * 2) + 2147483648) >> 32;
}

int32_t computeCurrentValueForArpMidiCvGate(int32_t value) {
return computeCurrentValueForStandardMenuItem(value);
}

int32_t computeFinalValueForStandardMenuItem(int32_t value) {
if (value == kMaxMenuValue) {
return 2147483647;
Expand Down Expand Up @@ -64,3 +68,7 @@ int32_t computeFinalValueForPan(int32_t value) {
return ((int32_t)value * (2147483648 / (kMaxMenuRelativeValue * 2)) * 2);
}
}

int32_t computeFinalValueForArpMidiCvGate(int32_t value) {
return (uint32_t)value * 85899345 - 2147483648;
}
16 changes: 16 additions & 0 deletions src/deluge/gui/menu_item/value_scaling.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
///
/// Done:
/// - audio_compressor::CompParam
/// - arpeggiator::midi_cv::Gate
/// - osc::PulseWidth
/// - patched_param::Integer
/// - patched_param::Pan
Expand Down Expand Up @@ -68,3 +69,18 @@ int32_t computeCurrentValueForPan(int32_t value);
/** Scales -25 to 25 range to INT32_MIN-INT32_MAX for storage and use.
*/
int32_t computeFinalValueForPan(int32_t value);

/** Scales INT32_MIN-INT32_MAX range to 0-50 for display.
*
* This roundtrips with the final value math despite not
* being it's proper inverse.
*/
int32_t computeCurrentValueForArpMidiCvGate(int32_t value);

/** Scales 0-50 range to INT32_MIN-(INT32_MAX-45) for storage and use.
*
* This is presumably to have the gate go down even at 50: the values
* produced create a 2.5ms gate down period between 16th arp notes at Gate=50,
* which exactly matches the gate down period between regular 16h notes.
*/
int32_t computeFinalValueForArpMidiCvGate(int32_t value);
17 changes: 17 additions & 0 deletions tests/unit/value_scaling_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,20 @@ TEST(ValueScalingTest, panValueScaling) {
CHECK_EQUAL(0, computeFinalValueForPan(0));
CHECK_EQUAL(INT32_MAX, computeFinalValueForPan(25));
}

TEST(ValueScalingTest, arpMidiCvGateValueScaling) {
for (int i = kMinMenuValue; i <= kMaxMenuValue; i++) {
int32_t finalValue = computeFinalValueForArpMidiCvGate(i);
int32_t currentValue = computeCurrentValueForArpMidiCvGate(finalValue);
CHECK_EQUAL(i, currentValue);
}
CHECK_EQUAL(INT32_MIN, computeFinalValueForArpMidiCvGate(0));
CHECK_EQUAL(-23, computeFinalValueForArpMidiCvGate(25));
// As seen here, we diverge _slightly_ from the standard
// menu item scaling when computing final values, despite roundtripping
// and using the identical current value computation.
//
// See computeFinalValueForArpMidiCvGate()'s comment for possible
// motivation.
CHECK_EQUAL(INT32_MAX-45, computeFinalValueForArpMidiCvGate(50));
}

0 comments on commit 7474d94

Please sign in to comment.