Fix stale hover latch on drag release, tooltip anchor, border off-by-one; pin gutter golden test, drop DPI framing

This commit is contained in:
2026-07-30 09:35:18 -04:00
parent ae23ee0882
commit 9812690b96
8 changed files with 64 additions and 20 deletions
+1 -1
View File
@@ -1034,7 +1034,7 @@ target_link_libraries(capture_browser_tests PRIVATE capture_browser)
add_test(NAME capture_browser_tests COMMAND capture_browser_tests) add_test(NAME capture_browser_tests COMMAND capture_browser_tests)
add_executable(keyboard_strip_tests tests/test_keyboard_strip.cpp) add_executable(keyboard_strip_tests tests/test_keyboard_strip.cpp)
target_link_libraries(keyboard_strip_tests PRIVATE keyboard_strip) target_link_libraries(keyboard_strip_tests PRIVATE keyboard_strip sample_bands)
add_test(NAME keyboard_strip_tests COMMAND keyboard_strip_tests) add_test(NAME keyboard_strip_tests COMMAND keyboard_strip_tests)
# waveform_view (S11): the pure marker geometry + zero-crossing snap. Links waveform_view (+ # waveform_view (S11): the pure marker geometry + zero-crossing snap. Links waveform_view (+
+5
View File
@@ -242,3 +242,8 @@ slider couldn't. Two pure modules split the forward (draw) and inverse (edit) ma
Channel-mode (D-E) bus-renegotiation design and the earlier Preserve-onset-latency Channel-mode (D-E) bus-renegotiation design and the earlier Preserve-onset-latency
framing in the S16 guardrails. Root `CLAUDE.md` is the current source of truth framing in the S16 guardrails. Root `CLAUDE.md` is the current source of truth
for both — do not reintroduce either superseded design. for both — do not reintroduce either superseded design.
- **`keyboard_strip`'s width-uniformity guarantee is client-pixel only.** Its test sweep
covers client-pixel widths (including multiples standing in for larger client areas);
nothing in the instrument implements `IPlugViewContentScaleSupport`, so host-side DPI
scaling of the plugin window — which would resample the uniform integer key widths at the
physical-pixel level — is unverified.
+3 -4
View File
@@ -1,9 +1,7 @@
// keyboard_strip.h — piano-keyboard geometry for the editor's root strip: per-class key // keyboard_strip.h — piano-keyboard geometry for the editor's root strip: per-class key
// rects, hit-test, the root marker, and the note name a hovered key reports. // rects, hit-test, the root marker, and the note name a hovered key reports.
// //
// Same-class keys are one integer width by construction. An arbitrary band width is not // See src/core/instrument/CLAUDE.md for the uniform-width-vs-edge-to-edge-tiling tradeoff.
// divisible by the 75 white keys, so the residue lands in symmetric end margins — uniform
// key widths and gap-free edge-to-edge tiling cannot both hold, and uniformity wins.
#pragma once #pragma once
@@ -21,7 +19,8 @@ inline constexpr int kStripKeyCount = 128;
inline constexpr int kStripWhiteKeyCount = 75; inline constexpr int kStripWhiteKeyCount = 75;
struct StripLayout { struct StripLayout {
Rect band; // the surface handed in, edge to edge Rect band; // the surface handed in, edge to edge (no src/ consumer; kept as the tests'
// reference bound instead of recomputing Rect::ltrb(0, 0, w, h) at each call site)
Rect keys; // the tiled key area, kStripWhiteKeyCount * whiteWidth, centred in band Rect keys; // the tiled key area, kStripWhiteKeyCount * whiteWidth, centred in band
int whiteWidth = 0; int whiteWidth = 0;
int blackWidth = 0; int blackWidth = 0;
+1 -1
View File
@@ -4,7 +4,7 @@
#include <algorithm> #include <algorithm>
#include "core/instrument/ui/sample_bands.h" // kPad / kTitleHeight / kChromeRowHeight #include "core/instrument/ui/sample_bands.h" // kPad
namespace reasampler::instrument::ui { namespace reasampler::instrument::ui {
+5
View File
@@ -79,6 +79,11 @@ void ReaSamplerEditor::onMouseUp(int x, int y) {
drag_ = DragKind::kNone; drag_ = DragKind::kNone;
dragParamId_ = -1; dragParamId_ = -1;
curvePointIndex_ = -1; curvePointIndex_ = -1;
// hover_ is deliberately not re-resolved during a drag (see resolveHover's caller), so it
// still names wherever the drag started. Re-resolve now against the release position, for
// every drag kind — otherwise the next paint latches a stale hover (wrong note name/tooltip,
// wrong control outline) until the next WM_MOUSEMOVE.
resolveHover(x, y);
// A scrollbar drag is transient UI (no parameter change), and the processor-side knobs // A scrollbar drag is transient UI (no parameter change), and the processor-side knobs
// (the preview-velocity -2 sentinel, voice count, master gain) are per-instance settings // (the preview-velocity -2 sentinel, voice count, master gain) are per-instance settings
// that don't reload the instrument. Master gain is an atomic the audio thread reads // that don't reload the instrument. Master gain is an atomic the audio thread reads
@@ -75,7 +75,6 @@ bool ReaSamplerEditor::mouseDownChrome(const FaceLayout& fl, int x, int y) {
const StripLayout sl = layoutStrip(cr.rootStrip.width, cr.rootStrip.height); const StripLayout sl = layoutStrip(cr.rootStrip.width, cr.rootStrip.height);
if (keyAtPoint(sl, x - cr.rootStrip.x, y - cr.rootStrip.y) >= 0) { if (keyAtPoint(sl, x - cr.rootStrip.x, y - cr.rootStrip.y) >= 0) {
drag_ = DragKind::kRootMarker; drag_ = DragKind::kRootMarker;
dragStartX_ = x;
dragStartParams_ = params_; dragStartParams_ = params_;
onMouseMove(x, y); onMouseMove(x, y);
return true; return true;
+5 -2
View File
@@ -205,13 +205,16 @@ void ReaSamplerEditor::paintChromeTooltip(LICE_IBitmap* bmp, const FaceLayout& f
const std::string label = noteName(hover_.index); const std::string label = noteName(hover_.index);
const int textW = static_cast<int>(label.size()) * kTooltipCharPx; const int textW = static_cast<int>(label.size()) * kTooltipCharPx;
const TooltipBox tb = computeTooltip(area.x + key.x, area.y + key.y, key.width, key.height, // Anchor y/h to the whole strip row, not the hovered key: a black key (18px) is shorter
// than a white key (30px), and anchoring to the key rect placed the chip 12px higher for
// black keys — landing on the keyboard's bottom edge and, near the root, on the badge.
const TooltipBox tb = computeTooltip(area.x + key.x, area.y, key.width, area.height,
textW, kTooltipTextH, w, h, TooltipSpec{}); textW, kTooltipTextH, w, h, TooltipSpec{});
if (tb.empty()) return; if (tb.empty()) return;
const Rect box = Rect::ltrb(tb.x, tb.y, tb.x + tb.width, tb.y + tb.height); const Rect box = Rect::ltrb(tb.x, tb.y, tb.x + tb.width, tb.y + tb.height);
fillSurface(bmp, toKitBox(box), Role::BgCell, InteractionState::Hover); fillSurface(bmp, toKitBox(box), Role::BgCell, InteractionState::Hover);
LICE_DrawRect(bmp, box.x, box.y, box.width, box.height, LICE_DrawRect(bmp, box.x, box.y, box.width - 1, box.height - 1,
toLice(roleColor(Role::LineHairline)), 1.0f, 0); toLice(roleColor(Role::LineHairline)), 1.0f, 0);
kitTextCentered(bmp, box, label.c_str(), Font::Label, Role::TextPrimary); kitTextCentered(bmp, box, label.c_str(), Font::Label, Role::TextPrimary);
} }
+44 -11
View File
@@ -2,14 +2,23 @@
// framework. Same fast assert loop as the sibling pure tests. // framework. Same fast assert loop as the sibling pure tests.
// //
// Covers: layoutStrip (normal, degenerate, sub-key-width); same-class key-width uniformity // Covers: layoutStrip (normal, degenerate, sub-key-width); same-class key-width uniformity
// swept across editor widths AND DPI scale factors; the tiled key area staying centred // swept across editor client widths (including multiples of them, standing in for larger
// client sizes — see the client-pixel-only note below); the tiled key area staying centred
// inside a band that spans the full width it was handed; whiteIndexOf / isNaturalKey across // inside a band that spans the full width it was handed; whiteIndexOf / isNaturalKey across
// octave boundaries and the 0..127 extremes; keyRect tiling and black-over-white overlap; // octave boundaries and the 0..127 extremes; keyRect tiling and black-over-white overlap;
// keyAtPoint resolving black-over-white by zone and missing off-band; the root affordance's // keyAtPoint resolving black-over-white by zone and missing off-band; the root affordance's
// hit-to-marker round trip; resolveDragNote clamping a wandering pointer; and noteName under // hit-to-marker round trip; resolveDragNote clamping a wandering pointer; noteName under the
// the C4 (MIDI 60) DAW convention. // C4 (MIDI 60) DAW convention; and the gutter pinned at the shipped default window size.
//
// Client-pixel-only guarantee: every width swept below is a CLIENT-pixel width. Nothing in
// the instrument implements IPlugViewContentScaleSupport, so if a host scales the plugin
// window itself, uniform integer key widths get resampled at the physical-pixel level —
// unverified by this suite (see src/core/instrument/CLAUDE.md).
#include "../src/core/instrument/ui/keyboard_strip.h" #include "../src/core/instrument/ui/keyboard_strip.h"
#include "../src/core/instrument/ui/sample_bands.h" // kPad, to derive the strip width the
// shipped default window (840x620,
// editor_session.cpp) hands the strip
#include <cstdio> #include <cstdio>
#include <string> #include <string>
@@ -24,11 +33,12 @@ static int g_fail = 0;
// A comfortable strip: wide enough that every class is several pixels across. // A comfortable strip: wide enough that every class is several pixels across.
static StripLayout wideStrip() { return layoutStrip(1280, 30); } static StripLayout wideStrip() { return layoutStrip(1280, 30); }
// The widths a real editor hands the strip — the 560px minimum client up to a wide // The widths a real editor hands the strip — the 560px minimum client up to a wide window.
// window — crossed with the DPI scale factors Windows actually reports.
static const int kBaseWidths[] = {544, 600, 640, 700, 749, 750, 751, 824, 900, 1000, static const int kBaseWidths[] = {544, 600, 640, 700, 749, 750, 751, 824, 900, 1000,
1024, 1103, 1264, 1600, 1920, 2400}; 1024, 1103, 1264, 1600, 1920, 2400};
static const double kDpiScales[] = {1.0, 1.25, 1.5, 1.75, 2.0}; // Multiplies each base width to widen client-pixel coverage (e.g. a maximized/larger client
// area) — NOT a host DPI/content-scale factor; see the client-pixel-only note above.
static const double kWidthMultipliers[] = {1.0, 1.25, 1.5, 1.75, 2.0};
// --- layoutStrip -------------------------------------------------------------- // --- layoutStrip --------------------------------------------------------------
@@ -61,11 +71,11 @@ static void testDegenerateSizesYieldNoKeys() {
CHECK(keyAtPoint(narrow, 10, 10) == -1); CHECK(keyAtPoint(narrow, 10, 10) == -1);
} }
// --- the sharp one: same-class widths are uniform at every width and DPI scale --- // --- the sharp one: same-class widths are uniform at every client width tested ---
static void testSameClassKeysAreEqualWidthAcrossWidthsAndDpiScales() { static void testSameClassKeysAreEqualWidthAcrossClientWidths() {
for (const int base : kBaseWidths) { for (const int base : kBaseWidths) {
for (const double scale : kDpiScales) { for (const double scale : kWidthMultipliers) {
const int w = static_cast<int>(base * scale); const int w = static_cast<int>(base * scale);
const int h = static_cast<int>(30 * scale); const int h = static_cast<int>(30 * scale);
const StripLayout L = layoutStrip(w, h); const StripLayout L = layoutStrip(w, h);
@@ -98,7 +108,7 @@ static void testSameClassKeysAreEqualWidthAcrossWidthsAndDpiScales() {
static void testKeyAreaSpansTheBandWithinOneKeyAtEveryTestedWidth() { static void testKeyAreaSpansTheBandWithinOneKeyAtEveryTestedWidth() {
for (const int base : kBaseWidths) { for (const int base : kBaseWidths) {
for (const double scale : kDpiScales) { for (const double scale : kWidthMultipliers) {
const int w = static_cast<int>(base * scale); const int w = static_cast<int>(base * scale);
const StripLayout L = layoutStrip(w, 30); const StripLayout L = layoutStrip(w, 30);
CHECK(L.band.width == w); // the strip always spans the width it was handed CHECK(L.band.width == w); // the strip always spans the width it was handed
@@ -268,10 +278,32 @@ static void testNoteNamesFollowTheC4Convention() {
CHECK(noteName(500) == "G9"); CHECK(noteName(500) == "G9");
} }
// --- the gutter at the shipped default window size -----------------------------
// The rule-based sweep above pins `margins == w % 75` and `leftMargin == margins/2` at every
// width, but pins no concrete number — Daniel is making a visual call on the specific gutter
// at the shipped default, and neither kPad nor the 840 default is covered by another test
// firing if either ever changes. The strip is a sawtooth with period kStripWhiteKeyCount (75)
// px of window width, and the shipped 840 default lands on residue 74 — the cycle's maximum:
// one pixel of resize (840->841) collapses both gutters to zero and grows every white key
// from 10px to 11px.
static void testGutterAtTheShippedDefaultWindowSize() {
constexpr int kShippedDefaultWindowW = 840; // editor_session.cpp's ViewRect default
const int stripW = kShippedDefaultWindowW - 2 * kPad; // chromeRects insets rootStrip by kPad
CHECK(stripW == 824);
const StripLayout L = layoutStrip(stripW, 30);
CHECK(L.whiteWidth == 10);
const int margins = L.band.width - L.keys.width;
CHECK(margins == 74);
const int leftMargin = L.keys.x - L.band.x;
CHECK(leftMargin == 37);
}
int main() { int main() {
testLayoutFillsTheBandAndCentresTheKeys(); testLayoutFillsTheBandAndCentresTheKeys();
testDegenerateSizesYieldNoKeys(); testDegenerateSizesYieldNoKeys();
testSameClassKeysAreEqualWidthAcrossWidthsAndDpiScales(); testSameClassKeysAreEqualWidthAcrossClientWidths();
testKeyAreaSpansTheBandWithinOneKeyAtEveryTestedWidth(); testKeyAreaSpansTheBandWithinOneKeyAtEveryTestedWidth();
testIsNaturalKeyAcrossAnOctaveAndTheExtremes(); testIsNaturalKeyAcrossAnOctaveAndTheExtremes();
testWhiteIndexCountsNaturalsBelowTheNote(); testWhiteIndexCountsNaturalsBelowTheNote();
@@ -284,6 +316,7 @@ int main() {
testHitTestingAKeyMarksThatSameKey(); testHitTestingAKeyMarksThatSameKey();
testDragTracksThePointerAndClampsWhenItWanders(); testDragTracksThePointerAndClampsWhenItWanders();
testNoteNamesFollowTheC4Convention(); testNoteNamesFollowTheC4Convention();
testGutterAtTheShippedDefaultWindowSize();
if (g_fail == 0) { if (g_fail == 0) {
std::printf("keyboard_strip: all tests passed\n"); std::printf("keyboard_strip: all tests passed\n");