Skip to content

Commit

Permalink
menus: clamp to avoid overflow on shift+scroll (#925)
Browse files Browse the repository at this point in the history
* menus: clamp to avoid overflow on shift+scroll

This change is the result of auditing all menu items that override
selectEncoderAction to ensure none of them can go out-of-bounds if
abs(offset) > 1

Fixes #871

* enumeration: use 3 offset for 4 length with shift-scroll
  • Loading branch information
sapphire-arches committed Jan 7, 2024
1 parent 0e63b04 commit fd7f9d8
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 110 deletions.
12 changes: 5 additions & 7 deletions src/deluge/gui/menu_item/audio_clip/mod_fx/type.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ class Type final : public menu_item::mod_fx::Type {
// We override this to set min value to 1. We don't inherit any getMinValue() function to override more easily
void selectEncoderAction(int32_t offset) override {
auto current = this->getValue() + offset;
int32_t numOptions = getOptions().size();
int32_t numOptions = getOptions().size() - 1;

if (current >= numOptions) {
current -= (numOptions - 1);
}
else if (current < 1) {
current += (numOptions - 1);
}
// Shift current down by 1, clamp it (with wrapping) to the legal range, and then shift it back up
current -= 1;
current = ((current % numOptions) + numOptions) % numOptions;
current += 1;

this->setValue(static_cast<ModFXType>(current));
Value::selectEncoderAction(offset);
Expand Down
22 changes: 22 additions & 0 deletions src/deluge/gui/menu_item/enumeration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,28 @@ void Enumeration::beginSession(MenuItem* navigatedBackwardFrom) {
void Enumeration::selectEncoderAction(int32_t offset) {
this->setValue(this->getValue() + offset);
int32_t numOptions = size();
int32_t sign = (offset < 0) ? -1 : ((offset > 0) ? 1 : 0);

switch (numOptions) {
case 0:
[[fallthrough]];
case 1:
[[fallthrough]];
case 2:
offset = 1 * sign;
break;
case 3:
offset = std::min<int32_t>(offset * sign, 2) * sign;
break;
case 4:
offset = std::min<int32_t>(offset * sign, 3) * sign;
break;
case 5:
offset = std::min<int32_t>(offset * sign, 4) * sign;
break;
default:
break;
}

if (display->haveOLED()) {
if (this->getValue() >= numOptions) {
Expand Down
52 changes: 7 additions & 45 deletions src/deluge/gui/menu_item/integer_range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,47 +43,20 @@ void IntegerRange::selectEncoderAction(int32_t offset) {

// Editing lower
if (soundEditor.editingRangeEdge == RangeEdit::LEFT) {
if (offset == 1) {
if (lower == upper) {
if (upper >= maxValue) {
goto justDrawRange;
}
else {
upper++;
}
}
lower = std::clamp(lower + offset, minValue, maxValue);
if (upper < lower) {
upper = lower;
}
else {
if (lower <= minValue) {
goto justDrawRange;
}
}

lower += offset;
}

// Editing upper
else {
if (offset == 1) {
if (upper >= maxValue) {
goto justDrawRange;
}
}
else {
if (upper == lower) {
if (lower <= minValue) {
goto justDrawRange;
}
else {
lower--;
}
}
upper = std::clamp(upper + offset, minValue, maxValue);
if (upper < lower) {
lower = upper;
}

upper += offset;
}

justDrawRange:
drawValueForEditingRange(false);
}

Expand All @@ -92,18 +65,7 @@ void IntegerRange::selectEncoderAction(int32_t offset) {
return;
}

if (offset == 1) {
if (lower == maxValue) {
goto justDrawOneNumber;
}
}
else {
if (lower == minValue) {
goto justDrawOneNumber;
}
}

lower += offset;
lower = std::clamp(lower + offset, minValue, maxValue);
upper = lower;

justDrawOneNumber:
Expand Down
58 changes: 6 additions & 52 deletions src/deluge/gui/menu_item/key_range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,61 +24,22 @@
namespace deluge::gui::menu_item {

void KeyRange::selectEncoderAction(int32_t offset) {
int32_t const KEY_MIN = 0;
int32_t const KEY_MAX = 11;

// If editing the range
if (soundEditor.editingRangeEdge != RangeEdit::OFF) {

// Editing lower
if (soundEditor.editingRangeEdge == RangeEdit::LEFT) {

int32_t newValue = lower + offset;
if (newValue < 0) {
newValue += 12;
}
else if (newValue >= 12) {
newValue -= 12;
}

if (offset == 1) {
if (lower == upper) {
goto justDrawRange;
}
}
else {
if (newValue == upper) {
goto justDrawRange;
}
}

lower = newValue;
// Do not allow lower to pass upper
lower = std::clamp(lower + offset, KEY_MIN, upper);
}

// Editing upper
else {

int32_t newValue = upper + offset;
if (newValue < 0) {
newValue += 12;
}
else if (newValue >= 12) {
newValue -= 12;
}

if (offset == 1) {
if (newValue == lower) {
goto justDrawRange;
}
}
else {
if (upper == lower) {
goto justDrawRange;
}
}

upper = newValue;
upper = std::clamp(upper + offset, lower, KEY_MAX);
}

justDrawRange:
drawValueForEditingRange(false);
}

Expand All @@ -87,14 +48,7 @@ void KeyRange::selectEncoderAction(int32_t offset) {
return;
}

lower += offset;
if (lower < 0) {
lower += 12;
}
else if (lower >= 12) {
lower -= 12;
}

lower = std::clamp(lower + offset, KEY_MIN, KEY_MAX);
upper = lower;

drawValue();
Expand Down
2 changes: 2 additions & 0 deletions src/deluge/gui/menu_item/midi/devices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ void Devices::beginSession(MenuItem* navigatedBackwardFrom) {
}

void Devices::selectEncoderAction(int32_t offset) {
offset = std::clamp<int32_t>(offset, -1, 1);

do {
int32_t newValue = this->getValue() + offset;

Expand Down
4 changes: 2 additions & 2 deletions src/deluge/gui/menu_item/patch_cables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ void PatchCables::selectEncoderAction(int32_t offset) {
}
else {
if (newValue >= set->numPatchCables) {
newValue -= set->numPatchCables;
newValue %= set->numPatchCables;
}
else if (newValue < 0) {
newValue += set->numPatchCables;
newValue = (newValue % set->numPatchCables + set->numPatchCables) % set->numPatchCables;
}
}

Expand Down
5 changes: 1 addition & 4 deletions src/deluge/gui/menu_item/source_selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,7 @@ void SourceSelection::selectEncoderAction(int32_t offset) {
}
}
else {
if (newValue >= kNumPatchSources)
newValue -= kNumPatchSources;
else if (newValue < 0)
newValue += kNumPatchSources;
newValue = ((newValue % kNumPatchSources) + kNumPatchSources) % kNumPatchSources;
}

s = sourceMenuContents[newValue];
Expand Down

0 comments on commit fd7f9d8

Please sign in to comment.