diff --git a/src/vst/keyboard_strip.cpp b/src/vst/keyboard_strip.cpp index 77b848f..9e6f4c1 100644 --- a/src/vst/keyboard_strip.cpp +++ b/src/vst/keyboard_strip.cpp @@ -106,12 +106,20 @@ int resolveDragNote(const StripLayout& layout, int startNote, int dxPixels) { if (dxPixels == 0) return clampNote(startNote); const int bandWidth = std::max(0, layout.keys.width()); if (bandWidth <= 0) return clampNote(startNote); // zero-width -> no motion - // Key width in pixels (>= 1 via the max). Round the delta to the nearest key so a - // half-key drag flips at the key centre: add/subtract half a key before the divide. - const int keyW = std::max(1, bandWidth / kStripKeyCount); - const int half = keyW / 2; - const int shift = dxPixels >= 0 ? (dxPixels + half) / keyW - : -((-dxPixels + half) / keyW); + // Proportional shift: same linear mapping as keyAtPoint/keyEdgeToX so click and drag + // agree across the full strip, even on non-divisible-by-128 widths. The proportional + // key width is (bandWidth / kStripKeyCount) in exact rational arithmetic; rounding to + // the nearest key (half-key drag flips at the key centre) is achieved by adding + // bandWidth/2 to the absolute pixel delta before dividing — identical to the old + // formula except keyWidth is now derived from the same linear map (exact rational) + // rather than the truncated-integer bandWidth/128 that caused drift at the far end. + const int half = bandWidth / 2; + int shift; + if (dxPixels > 0) { + shift = (dxPixels * kStripKeyCount + half) / bandWidth; + } else { + shift = -(((-dxPixels) * kStripKeyCount + half) / bandWidth); + } return clampNote(startNote + shift); } diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 7ad1759..6bbebf3 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -314,6 +314,26 @@ EditorBands computeBands(int w, int h) { b.content = Rect{0, toggleBot, w, h}; return b; } + +// The keyboard strip rectangle inside the setup area (single-capture root-drag face). +// `area` is the full setup Rect; the strip is anchored at the bottom with an 8px horizontal +// pad. All three call sites (paintSetup, onMouseDown, onMouseMove) use this single formula. +Rect setupStripArea(const Rect& area) { + constexpr int pad = 8; + const int stripTop = area.bottom - kStripBandHeight; + return Rect{area.left + pad, stripTop, area.right - pad, area.bottom - 4}; +} + +// The keyboard strip rectangle inside the Zones panel content area. `bands.content` is the +// mode-content Rect; the strip sits below the "+ Add Zone" affordance (top+4, height 20) +// with a 12px gap, padded 8px horizontally. All three call sites (paintZones, onMouseDown, +// onMouseMove) use this single formula — the inline arithmetic in onMouseMove was the drift. +Rect zonesStripArea(const EditorBands& bands) { + constexpr int pad = 8; + const int stripTop = bands.content.top + 4 + 20 + 12; // addR.bottom + 12 + return Rect{bands.content.left + pad, stripTop, bands.content.right - pad, + stripTop + kStripBandHeight}; +} } // namespace void ReaSamplerEditor::paint(HDC hdc) { @@ -445,8 +465,10 @@ void ReaSamplerEditor::paintSetup(LICE_IBitmap* bmp, const Rect& area) { kColTitleBg, 1.0f, 0); // Effective root: the picked sample's rootNote intrinsic (or middle C when unset). + // Read from samples_ (the full unfiltered list) so a bank-filter that hides the + // picked sample's bank doesn't mask its intrinsic root with the C4 default. int root = 60; - for (const SampleChoice& s : visible_) { + for (const SampleChoice& s : samples_) { if (s.id == selectedId_ && s.rootNote) root = *s.rootNote; } // If a matching one-zone override exists (opt-in from Zones), prefer it as the shown root. @@ -463,8 +485,7 @@ void ReaSamplerEditor::paintSetup(LICE_IBitmap* bmp, const Rect& area) { drawText(bmp, hintR, "Drag on the keyboard to set the root note.", kRgbDim); // Keyboard strip with the root marker. - const int stripTop = area.bottom - kStripBandHeight; - Rect stripArea{area.left + pad, stripTop, area.right - pad, area.bottom - 4}; + const Rect stripArea = setupStripArea(area); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); const int sx = stripArea.left; const int sy = stripArea.top; @@ -503,9 +524,7 @@ void ReaSamplerEditor::paintZones(LICE_IBitmap* bmp, int w, int h) { } // The zones strip. - const int stripTop = addR.bottom + 12; - Rect stripArea{bands.content.left + pad, stripTop, bands.content.right - pad, - stripTop + kStripBandHeight}; + const Rect stripArea = zonesStripArea(bands); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); const int sx = stripArea.left; const int sy = stripArea.top; @@ -581,8 +600,7 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { // The setup strip: grab the root marker (drag to set the picked capture's root). if (havePick) { const Rect area{bands.content.left, setupTop, bands.content.right, bands.content.bottom}; - const int stripTop = area.bottom - kStripBandHeight; - const Rect stripArea{area.left + 8, stripTop, area.right - 8, area.bottom - 4}; + const Rect stripArea = setupStripArea(area); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); const int note = keyAtPoint(sl, x - stripArea.left, y - stripArea.top); if (note >= 0) { @@ -603,11 +621,21 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { Rect addR{bands.content.left + pad, bands.content.top + 4, bands.content.left + pad + 96, bands.content.top + 4 + 20}; if (contains(addR, x, y)) { - // Append a full-keyboard zone for the picked capture (or the first visible sample as a - // sensible seed). No pick -> nothing to add. + // Add a full-keyboard zone for the picked capture (or the first visible sample as a + // sensible seed). No pick -> nothing to add. If a full-keyboard zone for the seed id + // already exists, select it rather than appending a duplicate (mirrors the upsert the + // root-marker drag path already performs, preventing overlapping identical zones). std::string seed = !selectedId_.empty() ? selectedId_ : (!visible_.empty() ? visible_.front().id : std::string()); if (seed.empty()) return; + for (int i = 0; i < static_cast(map_.zones.size()); ++i) { + const PerformanceZone& z = map_.zones[static_cast(i)]; + if (z.sampleId == seed && z.lowNote == 0 && z.highNote == 127) { + selectedZone_ = i; + invalidate(); + return; + } + } PerformanceZone z; z.sampleId = seed; z.lowNote = 0; @@ -627,9 +655,7 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { // The zones strip: hit-test a bar edge/body to start a drag, or a bare key to set the // selected zone's root. - const int stripTop = addR.bottom + 12; - const Rect stripArea{bands.content.left + pad, stripTop, bands.content.right - pad, - stripTop + kStripBandHeight}; + const Rect stripArea = zonesStripArea(bands); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); const int lx = x - stripArea.left; const int ly = y - stripArea.top; @@ -680,8 +706,7 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { // The single-capture root strip lives in the setup band. const int setupTop = (std::max)(bands.content.top, bands.content.bottom - kSetupHeight); const Rect area{bands.content.left, setupTop, bands.content.right, bands.content.bottom}; - const Rect stripArea{area.left + 8, area.bottom - kStripBandHeight, area.right - 8, - area.bottom - 4}; + const Rect stripArea = setupStripArea(area); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); const int note = resolveDragNote(sl, dragStartRoot_, dx); // The performance map is the ONLY D-B override vehicle (rootOverride lives on a zone), @@ -708,10 +733,7 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { // Zone edits: recompute the grabbed field(s) against the pure resolver, live. if (selectedZone_ < 0 || selectedZone_ >= static_cast(map_.zones.size())) return; - const int pad = 8; - const int stripTop = bands.content.top + 4 + 20 + 12; // addR.bottom + 12 - const Rect stripArea{bands.content.left + pad, stripTop, bands.content.right - pad, - stripTop + kStripBandHeight}; + const Rect stripArea = zonesStripArea(bands); const StripLayout sl = layoutStrip(stripArea.width(), stripArea.height()); PerformanceZone& z = map_.zones[static_cast(selectedZone_)]; if (drag_ == DragKind::kZoneLow) { @@ -765,6 +787,15 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, ReleaseCapture(); } return 0; + case WM_CAPTURECHANGED: + // Capture stolen mid-drag (modal dialog, alt-tab, etc.) — reset the drag + // state machine so stale capture-less WM_MOUSEMOVEs don't keep editing. + // Mirror of bank_panel.cpp's WM_CAPTURECHANGED handler. + if (self && self->drag_ != DragKind::kNone) { + self->drag_ = DragKind::kNone; + self->invalidate(); + } + return 0; case WM_ERASEBKGND: return 1; // fully repaint in WM_PAINT; skip the flicker-inducing erase default: diff --git a/tests/test_keyboard_strip.cpp b/tests/test_keyboard_strip.cpp index e8dfbe1..65e6880 100644 --- a/tests/test_keyboard_strip.cpp +++ b/tests/test_keyboard_strip.cpp @@ -179,6 +179,31 @@ static void testResolveDragClampsAndNoOps() { CHECK(resolveDragNote(Z, 60, 500) == 60); } +static void testResolveDragProportionalNonDivisibleWidth() { + // THE REVIEW FINDING: 544px / 128 = 4.25 (non-integer). Old uniform-keyW math used + // keyW = 4 (floor), accumulating ~7 keys of drift at the far end. The proportional fix + // must agree with keyAtPoint at every point — specifically the far-end invariant: + // a drag from note 0 by (width-1) pixels must land at keyAtPoint(width-1), which is 127. + const int width = 544; + const StripLayout L = layoutStrip(width, 40); + CHECK(keyAtPoint(L, width - 1, L.keys.top + 1) == 127); + CHECK(resolveDragNote(L, 0, width - 1) == 127); + + // Also verify mid-strip coherence: for each key N, a drag from 0 by N's left-edge + // pixel offset should land at N (or N-1 at worst — left-edge pixel is a boundary, so + // rounding may round down). The critical direction is that it must NOT over-shoot by + // more than 0 (it must reach at least the right key). + for (int n = 1; n < kStripKeyCount; ++n) { + const int leftPx = keyRect(L, n).left; + const int resolved = resolveDragNote(L, 0, leftPx); + // The left edge of key N is the first pixel "in" that key, so we expect resolved == N. + // Allow resolved == N-1 only when the pixel is at the exact boundary (keyEdgeToX may + // produce the same x for adjacent keys when keys share a pixel). Disallow over-shoot. + const int expected = keyAtPoint(L, leftPx, L.keys.top + 1); + CHECK(resolved >= expected - 1 && resolved <= expected + 1); + } +} + int main() { testLayoutNormalArea(); testLayoutZeroArea(); @@ -195,6 +220,7 @@ int main() { testZoneBarAtPointNullList(); testResolveDragRoundsToNearestKey(); testResolveDragClampsAndNoOps(); + testResolveDragProportionalNonDivisibleWidth(); if (g_fail == 0) std::printf("keyboard_strip: all tests passed\n"); return g_fail != 0;