From cf93d4dc50c0120fab26a348369ca05b76040fab Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 19:09:51 -0400 Subject: [PATCH] fix(param_slider): 7->5 o'clock arc (50% straight up), seam contract single-sourced, boundary/normDeg/wrap-boundary fixes --- src/vst/param_slider.cpp | 6 +++- src/vst/param_slider.h | 26 ++++++++++------- tests/test_param_slider.cpp | 58 ++++++++++++++++++++++++------------- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/src/vst/param_slider.cpp b/src/vst/param_slider.cpp index 6fa84b6..a080e43 100644 --- a/src/vst/param_slider.cpp +++ b/src/vst/param_slider.cpp @@ -89,6 +89,9 @@ constexpr double kPi = 3.14159265358979323846; double normDeg(double deg) { deg = std::fmod(deg, 360.0); if (deg < 0.0) deg += 360.0; + // Guard: fmod can return exactly 360.0 on some implementations due to floating-point + // rounding; fold it back to 0. + if (deg >= 360.0) deg -= 360.0; return deg; } @@ -109,7 +112,8 @@ bool knobHitTest(const KnobGeometry& knob, int x, int y) { if (knob.radius <= 0.0) return false; const double dx = x - knob.centerX; const double dy = y - knob.centerY; - return dx * dx + dy * dy <= knob.radius * knob.radius; + // Boundary exclusive: matches the module's half-open Rect convention. + return dx * dx + dy * dy < knob.radius * knob.radius; } double knobSweepDeg(const KnobArc& arc) { diff --git a/src/vst/param_slider.h b/src/vst/param_slider.h index 10b0fe3..b853eaa 100644 --- a/src/vst/param_slider.h +++ b/src/vst/param_slider.h @@ -105,17 +105,18 @@ double valueAtPoint(const Rect& control, int x); // (value 0) to endDeg (value 1); an endDeg at-or-behind startDeg wraps +360, so equal // angles mean a full 360° sweep. // -// The DEFAULT arc encodes Daniel's "6 to 4 o'clock" spec: min at 6 o'clock (180°) sweeping -// clockwise 300° around to max at 4 o'clock (120°), leaving a minimal 60° dead arc at the -// bottom-right. The angles are PARAMETERS, not hardcoded — the shell sets the final sweep -// when the parallel layout spec lands. -inline constexpr double kKnobArcStartDeg = 180.0; // value 0 — 6 o'clock -inline constexpr double kKnobArcEndDeg = 120.0; // value 1 — 4 o'clock (clockwise wrap) +// The DEFAULT arc is the conventional 7→5 o'clock layout: min at 7 o'clock (210°) sweeping +// clockwise 300° around to max at 5 o'clock (150°), leaving a symmetric 60° dead arc at the +// bottom. The 50% (midpoint) value lands at 12 o'clock (0°/360°) — straight up. The angles +// are PARAMETERS, not hardcoded — the shell sets the final sweep when the parallel layout +// spec lands. +inline constexpr double kKnobArcStartDeg = 210.0; // value 0 — 7 o'clock +inline constexpr double kKnobArcEndDeg = 150.0; // value 1 — 5 o'clock (clockwise wrap) // Default vertical-drag sensitivity: pixels of upward drag for one full 0->1 sweep. inline constexpr int kKnobDragRangePixels = 128; -// The configurable value arc of a knob. Defaults to the 6->4 o'clock reading above. +// The configurable value arc of a knob. Defaults to the 7->5 o'clock reading above. struct KnobArc { double startDeg = kKnobArcStartDeg; double endDeg = kKnobArcEndDeg; @@ -136,12 +137,15 @@ struct KnobPoint { }; // The knob circle inscribed in `cell`, centered, radius = half the smaller dimension. A -// degenerate cell yields radius 0. The shell passes whatever cell it wants the knob in -// (the whole control column, or a square sub-cell from its own layout). Pure. +// degenerate cell yields radius 0. CONTRACT: the shell MUST pass `row.control` (the full +// control column) both when drawing and when hit-testing — `controlAtPoint` always uses +// `r.control` as the cell, so the draw cell and hit cell must be the same. If the shell +// wants to draw a smaller circle it must center it within `row.control` and accept that the +// hit area is the larger column-inscribed circle. Pure. KnobGeometry computeKnob(const Rect& cell); -// True if (x, y) falls inside the knob circle (boundary inclusive). A degenerate knob -// (radius <= 0) hits nothing. Pure. +// True if (x, y) falls strictly inside the knob circle (boundary exclusive, matching the +// module's half-open Rect convention). A degenerate knob (radius <= 0) hits nothing. Pure. bool knobHitTest(const KnobGeometry& knob, int x, int y); // The clockwise sweep of `arc` in degrees, in (0, 360]: normalized end - start, wrapping diff --git a/tests/test_param_slider.cpp b/tests/test_param_slider.cpp index c115f6c..35005a4 100644 --- a/tests/test_param_slider.cpp +++ b/tests/test_param_slider.cpp @@ -11,12 +11,14 @@ // of the handle position; controlAtPoint routing a point to the right control id (toggle whole // area vs slider track vs knob circle) and MISSING in the label column, a row gap, and // off-panel. FA4 adds the radial KNOB: computeKnob inscribing the circle in its cell, the -// circular hit-test, the arc angle<->value mapping (min at startDeg, max at endDeg, linear -// midpoint; default = the 6->4 o'clock 300-degree sweep), the needle endpoint on the circle, -// and the vertical-drag delta->value map (up = increase) with clamping at 0/1. +// circular hit-test (boundary exclusive), the arc angle<->value mapping (min at startDeg, max at +// endDeg, linear midpoint; default = the 7->5 o'clock 300-degree sweep with 50% landing at 12 +// o'clock), wrap-boundary + un-normalized arc inputs, the needle endpoint on the circle, and +// the vertical-drag delta->value map (up = increase) with clamping at 0/1. #include "../src/vst/param_slider.h" +#include #include #include @@ -158,7 +160,7 @@ static void testValueAtPointDegenerateTrack() { // --- knob (FA4) ----------------------------------------------------------------- -static bool near(double a, double b, double tol) { return (a - b) < tol && (b - a) < tol; } +static bool nearWithin(double a, double b, double tol) { return (a - b) < tol && (b - a) < tol; } static void testKnobGeometryInscribesCell() { // A 44x44 cell at (100,0): center (122,22), radius 22. @@ -177,20 +179,21 @@ static void testKnobGeometryInscribesCell() { static void testKnobHitTestCircle() { const KnobGeometry g = computeKnob(Rect{100, 0, 144, 44}); // center (122,22), r 22 - CHECK(knobHitTest(g, 122, 22)); // center - CHECK(knobHitTest(g, 122 + 22, 22)); // on the circle boundary (inclusive) + CHECK(knobHitTest(g, 122, 22)); // center — always hits + CHECK(!knobHitTest(g, 122 + 22, 22)); // exactly on the boundary — boundary exclusive CHECK(!knobHitTest(g, 122 + 22, 44)); // cell corner: inside the rect, outside the circle - CHECK(!knobHitTest(g, 122, 45)); // just below the circle + CHECK(!knobHitTest(g, 122, 45)); // just below the circle + CHECK(knobHitTest(g, 122 + 21, 22)); // one pixel inside the boundary — hits CHECK(!knobHitTest(KnobGeometry{}, 0, 0)); // degenerate knob hits nothing } -static void testKnobDefaultArcIsSixToFourOClock() { - const KnobArc arc; // default: 180 (6 o'clock) clockwise to 120 (4 o'clock) +static void testKnobDefaultArcIsSevenToFiveOClock() { + const KnobArc arc; // default: 210 (7 o'clock) clockwise to 150 (5 o'clock) CHECK(approx(knobSweepDeg(arc), 300.0)); - CHECK(approx(knobValueAngleDeg(arc, 0.0), kKnobArcStartDeg)); // min at 6 o'clock - CHECK(approx(knobValueAngleDeg(arc, 1.0), kKnobArcEndDeg)); // max at 4 o'clock - // Midpoint: halfway around the clockwise sweep -> 180 + 150 = 330. - CHECK(approx(knobValueAngleDeg(arc, 0.5), 330.0)); + CHECK(approx(knobValueAngleDeg(arc, 0.0), kKnobArcStartDeg)); // min at 7 o'clock (210°) + CHECK(approx(knobValueAngleDeg(arc, 1.0), kKnobArcEndDeg)); // max at 5 o'clock (150°) + // Midpoint: 210 + 150 = 360 -> normalized to 0 (12 o'clock, straight up). + CHECK(approx(knobValueAngleDeg(arc, 0.5), 0.0)); // Out-of-range values clamp to the arc ends. CHECK(approx(knobValueAngleDeg(arc, -0.5), kKnobArcStartDeg)); CHECK(approx(knobValueAngleDeg(arc, 1.5), kKnobArcEndDeg)); @@ -207,21 +210,35 @@ static void testKnobArcIsParameterized() { CHECK(approx(knobSweepDeg(KnobArc{0.0, 0.0}), 360.0)); } +static void testKnobArcWrapBoundary() { + // A 1-degree arc starting at 180: end 181, sweep must be 1, NOT 361. + const KnobArc tiny{180.0, 181.0}; + CHECK(approx(knobSweepDeg(tiny), 1.0)); + // Un-normalized inputs: start -180 (== 180) sweeping to end 120. + // normDeg(-180) = 180; normDeg(120) = 120; sweep = 120-180 = -60 <= 0 -> 300. + const KnobArc unnorm{-180.0, 120.0}; + CHECK(approx(knobSweepDeg(unnorm), 300.0)); + CHECK(approx(knobValueAngleDeg(unnorm, 0.0), 180.0)); // min at 6 o'clock + CHECK(approx(knobValueAngleDeg(unnorm, 1.0), 120.0)); // max at 4 o'clock +} + static void testKnobNeedlePointOnCircle() { const KnobGeometry g = computeKnob(Rect{100, 0, 144, 44}); // center (122,22), r 22 - // Default arc, value 0 -> 6 o'clock -> straight DOWN from the center (screen +y). - const KnobPoint p6 = knobNeedlePoint(g, KnobArc{}, 0.0); - CHECK(near(p6.x, 122.0, 1e-6) && near(p6.y, 44.0, 1e-6)); + // Default arc, value 0 -> 7 o'clock -> needle points down-left from center. + const KnobPoint p7 = knobNeedlePoint(g, KnobArc{}, 0.0); + // 210° clockwise from 12: sin(210°)=-0.5, cos(210°)=-√3/2 -> x = cx - r/2, y = cy + r*√3/2 + CHECK(nearWithin(p7.x, 122.0 + 22.0 * std::sin(210.0 * 3.14159265358979323846 / 180.0), 1e-6)); + CHECK(nearWithin(p7.y, 22.0 - 22.0 * std::cos(210.0 * 3.14159265358979323846 / 180.0), 1e-6)); // A 12 o'clock needle points straight UP; 3 o'clock points RIGHT. const KnobPoint p12 = knobNeedlePoint(g, KnobArc{0.0, 180.0}, 0.0); - CHECK(near(p12.x, 122.0, 1e-6) && near(p12.y, 0.0, 1e-6)); + CHECK(nearWithin(p12.x, 122.0, 1e-6) && nearWithin(p12.y, 0.0, 1e-6)); const KnobPoint p3 = knobNeedlePoint(g, KnobArc{0.0, 180.0}, 0.5); - CHECK(near(p3.x, 144.0, 1e-6) && near(p3.y, 22.0, 1e-6)); + CHECK(nearWithin(p3.x, 144.0, 1e-6) && nearWithin(p3.y, 22.0, 1e-6)); // Every needle endpoint sits ON the circle. for (double v : {0.0, 0.25, 0.5, 0.75, 1.0}) { const KnobPoint p = knobNeedlePoint(g, KnobArc{}, v); const double dx = p.x - g.centerX, dy = p.y - g.centerY; - CHECK(near(dx * dx + dy * dy, g.radius * g.radius, 1e-6)); + CHECK(nearWithin(dx * dx + dy * dy, g.radius * g.radius, 1e-6)); } } @@ -297,8 +314,9 @@ int main() { testValueAtPointDegenerateTrack(); testKnobGeometryInscribesCell(); testKnobHitTestCircle(); - testKnobDefaultArcIsSixToFourOClock(); + testKnobDefaultArcIsSevenToFiveOClock(); testKnobArcIsParameterized(); + testKnobArcWrapBoundary(); testKnobNeedlePointOnCircle(); testKnobDragUpIncreases(); testKnobDragClamps();