From 2e09776342e7ba4f969c2c7e7fd71a857829d606 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 13:18:21 -0400 Subject: [PATCH 1/4] =?UTF-8?q?fix:=20stroke=20arcs=20and=20splines=20anal?= =?UTF-8?q?ytically=20=E2=80=94=20opaque=20core,=20angle-independent=20wei?= =?UTF-8?q?ght?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LICE_Arc never reaches opacity and ThickFLine's width is minor-axis. One distance-to-polyline coverage mask, blended once, replaces both. --- docs/product/visual-design-language.md | 53 ++- src/core/instrument/engine/velocity_curve.cpp | 29 +- src/core/instrument/engine/velocity_curve.h | 10 + src/core/ui/CLAUDE.md | 15 + src/core/ui/CMakeLists.txt | 3 + src/core/ui/stroke_aa.cpp | 178 ++++++++ src/core/ui/stroke_aa.h | 87 ++++ src/shell/instrument/CLAUDE.md | 1 + src/shell/instrument/CMakeLists.txt | 3 +- src/shell/instrument/editor_internal.h | 43 +- src/shell/instrument/editor_paint_curve.cpp | 31 +- .../instrument/editor_paint_waveform.cpp | 39 +- src/shell/instrument/editor_stroke.cpp | 83 ++++ src/shell/instrument/editor_stroke.h | 50 +++ tests/test_stroke_aa.cpp | 404 ++++++++++++++++++ tests/test_velocity_curve.cpp | 44 ++ 16 files changed, 1001 insertions(+), 72 deletions(-) create mode 100644 src/core/ui/stroke_aa.cpp create mode 100644 src/core/ui/stroke_aa.h create mode 100644 src/shell/instrument/editor_stroke.cpp create mode 100644 src/shell/instrument/editor_stroke.h create mode 100644 tests/test_stroke_aa.cpp diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index 000aeea..74b117d 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -779,20 +779,42 @@ concession. Everything with a slope or a curve must draw through a primitive tha antialiased and adds width. - `LICE_FillTriangle` takes **no** `aa` parameter at all — its sloped edges alias, and the only fix inside the kit is to re-stroke those edges with an AA line in the same ink. -- LICE has no thick-arc primitive. A wider ring is drawn as adjacent 1 px `LICE_Arc` calls at - stepped radii, which keeps every ring antialiased. +- **`LICE_Arc` does not rasterize an arc.** It rasterizes a whole circle clipped to a + rectangular bounding box per 90° chunk (`lice_arc.cpp` `__DrawArc`), and its AA circle splits + one unit of ink across two adjacent pixels by the **fractional part of the radius** + (`w = yf - floor(yf)`, then `wa` and `ai - wa`). A half-integer radius therefore puts 50% on + each of two pixels at the cardinal points, and stacked radii do not tile — vertical spacing + between rings `r` and `r-1` dilates from 1.0 px at the top to 1.41 px at 45°. Measured on the + shipped 3-ring knob arc: weakest cross-section peak **138/255** and perpendicular weight + **1.62–3.24 px** against a nominal 3 (67% ripple). +- **`LICE_ThickFLine` lays its width along the MINOR axis**, so perpendicular weight is + `wid·cos θ`. Measured at width 2: **1.41–2.00 px** across a 0–90° sweep — it thins to + `1/√2` of nominal at every diagonal. +- Neither of those two is usable for a stroke that must hold a consistent weight. Arcs and + spline contours draw through the analytic stroker instead (`core/ui/stroke_aa` + + `shell/instrument/editor_stroke`): coverage is distance-to-polyline, MAX-accumulated into a + scratch mask and blended **once**. The single blend is the structural part — compositing + per segment re-lays ink over the previous segment's fringe. - A min/max waveform column plot cannot be antialiased by the column fill itself (the columns are vertical). The outline is what reads as jagged, so it is stroked separately. +> **Methodological lesson — why this table got two rows wrong.** The original audit verified +> *which primitive each surface called* and treated an `aa=true` argument as the answer. It +> never verified *what the primitive rasterized*. Both misses hid behind a true-looking +> statement: `LICE_Arc` really does antialias, and `LICE_ThickFLine` really is always +> antialiased — neither fact says anything about opacity or perpendicular weight, which is +> what was actually broken. **A disposition row is only earned by a measurement of the +> rendered output** (peak alpha, weight across angle), not by reading the call site. + | Surface | Where | Disposition | |---|---|---| -| Radial knob track + value arc | `editor_internal.h` `drawKnobFace` | Was AA (`LICE_Arc`, 1 px). **Widened** to a 3 px stacked-radius ring; the bigger knob is what made 1 px read thin. | -| Knob needle | `drawKnobFace` | **Fixed** — was integer-endpoint `LICE_Line`; now `LICE_ThickFLine` (always AA, float endpoints, 2 px). | -| Inner curve dial arc + needle | `drawInnerDial` | **Fixed** — 2 px stacked-radius arc; needle moved to `LICE_FLine` with float endpoints. | -| Staged envelope segment slopes | `editor_paint_waveform.cpp` | **Fixed** — `LICE_ThickFLine` at 2 px, replacing integer-endpoint `LICE_Line`. | -| Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed** — same treatment, one trace grammar. | -| Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed** — same treatment. | -| Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | Left at 1 px AA `LICE_Line` — a 2 px trace blots at thumbnail scale. | +| Radial knob track + value arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — the stacked-radius `LICE_Arc` ring never reached an opaque core and rippled 67% in weight. Now ONE analytic stroke (`strokeArcAA`), outer edge on the knob radius. Measured: peak **255/255** at every cross-section, weight **2.95–3.11 px** (5% ripple). | +| Knob needle | `drawKnobFace` | **Fixed (2026-08-01)** — `LICE_ThickFLine`'s minor-axis width thinned it to `cos θ` as the knob swept. Now `strokeLineAA`, 2 px. | +| Inner curve dial arc + needle | `drawInnerDial` | **Fixed (2026-08-01)** — same as the knob: one analytic 2 px arc; needle via `strokeLineAA`. | +| Staged envelope segment slopes | `editor_paint_waveform.cpp` | **Fixed (2026-08-01)** — one `strokePolylineAA` over the whole polyline, so the stage joints blend once. Vertices stay INTEGER by design: they are the positions the draggable handles are drawn at. | +| Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed (2026-08-01)** — the trace was never gapped; it was fully aliased (every pixel full or empty) because the loop passed INTEGER `cy`, quantizing the slope into alternating 1/2 px steps. Now sub-pixel y (`subpixelFromPoint`) through `strokePolylineAA`. Measured: peak **255/255**, weight **1.95–2.01 px** (3% ripple). | +| Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed (2026-08-01)** — same cause, same treatment. | +| Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | **Fixed (2026-08-01)** — stays a 1 px hairline (a 2 px trace blots at thumbnail scale), but strokes analytically at sub-pixel y instead of integer-endpoint `LICE_Line`. | | Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. **Measured cost** (Release, MSVC 14.44, real LICE, 24 stereo cards × 136 columns = 6528 columns): fill alone 0.070 ms per full-grid repaint, fill+stroke 0.48 ms — the stroke is ~0.41 ms, about 2.5% of a 60 Hz frame, and the grid repaints on hover/scroll/drag, not continuously. One-off scratchpad measurement, 2026-08-01, harness not committed — not a standing regression guard; re-measure before relying on it again. | | Preview play triangle | `editor_paint_chrome.cpp` | **Fixed** — `LICE_FillTriangle` has no `aa`; its two sloped edges are re-stroked with AA `LICE_FLine`. | | Envelope/spline node handles (squares) | `editor_paint_waveform.cpp` | Already clean — axis-aligned `LICE_FillRect`. | @@ -807,6 +829,19 @@ concession. Everything with a slope or a curve must draw through a primitive tha | Docked bank panel chrome | `panel_render.cpp` | Already clean — axis-aligned fills, rects and hairlines. Its only exposure to this pass is the shared `drawWaveform`. | | Text | `draw_kit.cpp` `text` | Already clean — `LICE_CachedFont` AA glyph cache (§1.1). | +**Analytic stroker cost** (Release, MSVC, real LICE, one-off scratchpad harness 2026-08-01, +not committed — re-measure before relying on it): 30 knob arcs **0.113 ms → 0.169 ms**; a +500 px spline contour **0.013 ms → 0.047 ms**. About +0.09 ms per full editor repaint, on a +surface that repaints on interaction rather than continuously. Both figures beat the +prototype's targets (0.285 ms / 0.106 ms). Micro-optimisation, each lever measured in +isolation: writing the blend straight to the bitmap's bits rather than through +`LICE_PutPixel` is the big one (arcs 0.169 vs 0.253 ms); reusing the scratch mask across +calls matters on the contour's large bounding box (0.047 vs 0.073 ms); `float` over `double` +is small but real (contour coverage 0.045 vs 0.051 ms). The per-row valid-extent bookkeeping +in the mask is a **wash** against the simpler clear-the-whole-box design (0.218 vs 0.218 ms +for a full repaint) — it wins on the contour and loses on the small arc boxes; it is kept +because the contour is the drag-interactive surface. + ### 8.1 Was the piano-key width defect an aliasing artifact? **No.** Every piano key is an axis-aligned `LICE_FillRect` with an integer width, so there is diff --git a/src/core/instrument/engine/velocity_curve.cpp b/src/core/instrument/engine/velocity_curve.cpp index 2e11bcb..fc1838d 100644 --- a/src/core/instrument/engine/velocity_curve.cpp +++ b/src/core/instrument/engine/velocity_curve.cpp @@ -25,18 +25,30 @@ double valuePerPixel(const VelocityCurve::Box& box, CurveDomain d) { if (h <= 1) return 0.0; return (kCurveYMax - curveYMin(d)) / static_cast(h - 1); } -int velToX(const VelocityCurve::Box& box, double velocity) { +// The integer maps are these rounded — ONE mapping, so a sub-pixel trace and an integer hit-test +// cannot drift. Rounding the OFFSET (not the absolute coordinate) keeps the int results identical +// to what they were before the sub-pixel form existed: the offset is non-negative, so truncation +// is floor regardless of where the box sits. +double velToXf(const VelocityCurve::Box& box, double velocity) { const int w = std::max(0, box.width); - if (w <= 0) return box.left; + if (w <= 0) return static_cast(box.left); const double frac = (clampVelocity(velocity) - kVelMin) / (kVelMax - kVelMin); - return box.left + static_cast(frac * static_cast(w) + 0.5); + return static_cast(box.left) + frac * static_cast(w); } -int valueToY(const VelocityCurve::Box& box, double value, CurveDomain d) { +double valueToYf(const VelocityCurve::Box& box, double value, CurveDomain d) { const int h = std::max(0, box.height); - if (h <= 1) return box.top; + if (h <= 1) return static_cast(box.top); const double lo = curveYMin(d); const double frac = (clampValue(value, d) - lo) / (kCurveYMax - lo); - return box.top + static_cast((1.0 - frac) * static_cast(h - 1) + 0.5); + return static_cast(box.top) + (1.0 - frac) * static_cast(h - 1); +} +int velToX(const VelocityCurve::Box& box, double velocity) { + return box.left + + static_cast(velToXf(box, velocity) - static_cast(box.left) + 0.5); +} +int valueToY(const VelocityCurve::Box& box, double value, CurveDomain d) { + return box.top + + static_cast(valueToYf(box, value, d) - static_cast(box.top) + 0.5); } } // namespace @@ -187,6 +199,11 @@ VelocityCurve::CurvePixel VelocityCurve::pixelFromPoint(const Box& box, return CurvePixel{velToX(box, p.velocity), valueToY(box, p.value, domain_)}; } +VelocityCurve::CurvePixelF VelocityCurve::subpixelFromPoint(const Box& box, + const VelocityPoint& p) const { + return CurvePixelF{velToXf(box, p.velocity), valueToYf(box, p.value, domain_)}; +} + VelocityPoint VelocityCurve::pointFromPixel(const Box& box, int x, int y) const { // Exact inverse of velToX/valueToY (within one pixel); degenerate dims collapse the same way. VelocityPoint p; diff --git a/src/core/instrument/engine/velocity_curve.h b/src/core/instrument/engine/velocity_curve.h index 9df5be2..34532ec 100644 --- a/src/core/instrument/engine/velocity_curve.h +++ b/src/core/instrument/engine/velocity_curve.h @@ -182,6 +182,16 @@ public: }; CurvePixel pixelFromPoint(const Box& box, const VelocityPoint& p) const; + // The SAME mapping before rounding: pixelFromPoint IS this, rounded, so a sub-pixel trace and + // an integer hit-test cannot drift. An antialiased stroke needs the fraction — quantizing y to + // a whole pixel forces the slope into alternating 1/2-px steps, and that beat-frequency + // staircase is what read as a dotted line where a contour steepened. + struct CurvePixelF { + double x = 0.0; + double y = 0.0; + }; + CurvePixelF subpixelFromPoint(const Box& box, const VelocityPoint& p) const; + // Exact inverse of pixelFromPoint (within the one-pixel quantum) — where an empty-space // click lands as a new point. Degenerate box: zero-width reads velocity 0; height <= 1 // reads the domain's max (the top row is what a collapsed box draws). diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index bb91c53..0f334da 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -101,6 +101,7 @@ L7 sub-pass, 2026-07-27): - `tooltip` — pure tooltip placement + prefix-strip: strips the `ReaSampler:` display prefix from the registered action phrase; width clamped to the client rect. - `card_drag` — pure drag-gesture precedence + slot hit-test: leave-client → OS drag-out; other-bank → move/copy; same-bank → reorder / Alt-over-occupied → replace. - `card_meta` — pure card-metadata formatters: bars.beats.subdivisions and seconds.milliseconds; blank when the sample is unstamped. +- `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc`. An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle and gives an opaque core for any width above 1 px. Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). ## Gotchas @@ -122,3 +123,17 @@ L7 sub-pass, 2026-07-27): - `rect`'s prior role names survive only as `using` aliases at their old call sites — changing `rect.h` itself ripples across every directory that aliases it (e.g. `editor_geometry::Rect`); check all alias sites, not just this one. +- **`stroke_aa`'s mask is deliberately NOT cleared on `reset`.** Only + `[rowLo, rowHi)` of each row holds meaningful coverage; everything else is + whatever the reused buffer last held. That is what keeps a stroke's cost + proportional to its ink rather than to its bounding box — but it means any new + reader must respect the row extents, and any new writer must grow them through + `extendRow`, which zero-fills the newly-valid cells INCLUDING the gap when a + stroke revisits a row far from where it left it (a circle touches most rows on + both sides). Reading the raw buffer outside the extents returns garbage by + design, not zero. +- **Neither `LICE_Arc` nor `LICE_ThickFLine` can draw these strokes** — the first + never reaches an opaque core, the second's width is along the minor axis so its + perpendicular weight falls off as `cos θ`. The evidence and the measurements + live in `docs/product/visual-design-language.md` §8; do not "simplify" a stroke + site back onto either primitive. diff --git a/src/core/ui/CMakeLists.txt b/src/core/ui/CMakeLists.txt index 8678d05..eea594f 100644 --- a/src/core/ui/CMakeLists.txt +++ b/src/core/ui/CMakeLists.txt @@ -38,3 +38,6 @@ reasampler_test(card_meta LINK card_meta) reasampler_pure_library(card_drag SOURCES card_drag.cpp LINK PUBLIC drag_out bank_grid) reasampler_test(card_drag LINK card_drag) + +reasampler_pure_library(stroke_aa SOURCES stroke_aa.cpp) +reasampler_test(stroke_aa LINK stroke_aa) diff --git a/src/core/ui/stroke_aa.cpp b/src/core/ui/stroke_aa.cpp new file mode 100644 index 0000000..2d65506 --- /dev/null +++ b/src/core/ui/stroke_aa.cpp @@ -0,0 +1,178 @@ +#include "core/ui/stroke_aa.h" + +#include +#include + +namespace reasampler::ui { +namespace { + +inline float clamp01(float v) { return v < 0.0f ? 0.0f : (v > 1.0f ? 1.0f : v); } + +} // namespace + +void StrokeCanvas::reset(const Rect& bounds) { + bounds_ = bounds; + if (bounds_.empty()) { + bounds_ = Rect{}; + return; + } + const std::size_t area = + static_cast(bounds_.width) * static_cast(bounds_.height); + if (coverage_.size() < area) coverage_.resize(area); + rowLo_.assign(static_cast(bounds_.height), bounds_.width); + rowHi_.assign(static_cast(bounds_.height), 0); +} + +float StrokeCanvas::coverageAt(int x, int y) const { + if (!contains(bounds_, x, y)) return 0.0f; + const int rel = x - bounds_.x; + if (rel < rowLo(y) || rel >= rowHi(y)) return 0.0f; + return rowData(y)[rel]; +} + +// Grows a row's valid span to cover [x0, x1) (canvas-relative), zero-filling only the cells that +// become valid. Cost is bounded by the growth, so a whole stroke stays O(ink). +void StrokeCanvas::extendRow(int y, int x0, int x1) { + const std::size_t r = static_cast(y - bounds_.y); + float* row = coverage_.data() + r * static_cast(bounds_.width); + int lo = rowLo_[r]; + int hi = rowHi_[r]; + if (hi <= lo) { + std::fill(row + x0, row + x1, 0.0f); + rowLo_[r] = x0; + rowHi_[r] = x1; + return; + } + // A stroke can revisit a row far from where it left it (an arc touches most rows on both + // sides of the circle), so the gap between the old span and the new one must be zeroed too. + if (x0 < lo) { + std::fill(row + x0, row + lo, 0.0f); + rowLo_[r] = x0; + } + if (x1 > hi) { + std::fill(row + hi, row + x1, 0.0f); + rowHi_[r] = x1; + } +} + +void StrokeCanvas::addPiece(float ax, float ay, float bx, float by, float halfWidth) { + const float reach = halfWidth + 0.5f; // beyond this the coverage is 0 + const float dx = bx - ax; + const float dy = by - ay; + const float len2 = dx * dx + dy * dy; + const float invLen2 = len2 > 0.0f ? 1.0f / len2 : 0.0f; + + int x0 = static_cast(std::floor((std::min)(ax, bx) - reach)); + int x1 = static_cast(std::ceil((std::max)(ax, bx) + reach)) + 1; + int y0 = static_cast(std::floor((std::min)(ay, by) - reach)); + int y1 = static_cast(std::ceil((std::max)(ay, by) + reach)) + 1; + x0 = (std::max)(x0, bounds_.x); + y0 = (std::max)(y0, bounds_.y); + x1 = (std::min)(x1, bounds_.right()); + y1 = (std::min)(y1, bounds_.bottom()); + if (x0 >= x1 || y0 >= y1) return; + + const int relX0 = x0 - bounds_.x; + const int relX1 = x1 - bounds_.x; + for (int y = y0; y < y1; ++y) { + extendRow(y, relX0, relX1); + float* row = coverage_.data() + static_cast(y - bounds_.y) * + static_cast(bounds_.width); + const float pyc = static_cast(y) + 0.5f; + const float qy = pyc - ay; + for (int x = x0; x < x1; ++x) { + const float qx = static_cast(x) + 0.5f - ax; + const float t = clamp01((qx * dx + qy * dy) * invLen2); + const float ex = qx - dx * t; + const float ey = qy - dy * t; + const float cov = clamp01(reach - std::sqrt(ex * ex + ey * ey)); + float& dst = row[x - bounds_.x]; + if (cov > dst) dst = cov; + } + } +} + +void StrokeCanvas::addSegment(float ax, float ay, float bx, float by, float halfWidth) { + if (bounds_.empty() || halfWidth <= 0.0f) return; + const float dx = bx - ax; + const float dy = by - ay; + const float len2 = dx * dx + dy * dy; + if (len2 <= kMaxPieceLen * kMaxPieceLen) { + addPiece(ax, ay, bx, by, halfWidth); + return; + } + const int pieces = static_cast(std::sqrt(len2) / kMaxPieceLen) + 1; + float px = ax; + float py = ay; + for (int i = 1; i <= pieces; ++i) { + const float t = static_cast(i) / static_cast(pieces); + const float qx = ax + dx * t; + const float qy = ay + dy * t; + addPiece(px, py, qx, qy, halfWidth); + px = qx; + py = qy; + } +} + +Rect strokeBounds(const StrokePoint* pts, std::size_t count, float halfWidth, const Rect& clip) { + if (pts == nullptr || count == 0 || halfWidth <= 0.0f || clip.empty()) return Rect{}; + float minX = pts[0].x, maxX = pts[0].x, minY = pts[0].y, maxY = pts[0].y; + for (std::size_t i = 1; i < count; ++i) { + minX = (std::min)(minX, pts[i].x); + maxX = (std::max)(maxX, pts[i].x); + minY = (std::min)(minY, pts[i].y); + maxY = (std::max)(maxY, pts[i].y); + } + const float reach = halfWidth + 0.5f; + const int x0 = (std::max)(clip.x, static_cast(std::floor(minX - reach))); + const int y0 = (std::max)(clip.y, static_cast(std::floor(minY - reach))); + const int x1 = (std::min)(clip.right(), static_cast(std::ceil(maxX + reach)) + 1); + const int y1 = (std::min)(clip.bottom(), static_cast(std::ceil(maxY + reach)) + 1); + if (x0 >= x1 || y0 >= y1) return Rect{}; + return Rect::ltrb(x0, y0, x1, y1); +} + +void strokePolyline(StrokeCanvas& canvas, const StrokePoint* pts, std::size_t count, + float halfWidth, const Rect& clip) { + canvas.reset(strokeBounds(pts, count, halfWidth, clip)); + if (canvas.bounds().empty()) return; + if (count == 1) { + canvas.addSegment(pts[0].x, pts[0].y, pts[0].x, pts[0].y, halfWidth); + return; + } + for (std::size_t i = 1; i < count; ++i) { + canvas.addSegment(pts[i - 1].x, pts[i - 1].y, pts[i].x, pts[i].y, halfWidth); + } +} + +void appendArc(std::vector& out, float cx, float cy, float radius, float startRad, + float endRad, float flatnessPx) { + if (radius <= 0.0f) { + out.push_back(StrokePoint{cx, cy}); + return; + } + if (!(flatnessPx > 0.0f)) flatnessPx = kArcFlatnessPx; + // Chord sagitta: r*(1 - cos(step/2)) <= flatness. A flatness at or past the diameter admits + // the whole sweep in one chord, which is what keeps `maxStep` strictly positive. + const float cosHalf = (std::max)(-1.0f, 1.0f - flatnessPx / radius); + const float maxStep = 2.0f * std::acos(cosHalf); + const float sweep = endRad - startRad; + int segments = 1; + if (maxStep > 0.0f) { + const float wanted = std::ceil(std::fabs(sweep) / maxStep); + segments = wanted >= static_cast(kMaxArcSegments) + ? kMaxArcSegments + : (std::max)(1, static_cast(wanted)); + } else { + // A radius large enough that flatness/radius underflows the cosine's resolution. The cap + // is the termination guarantee, not a quality choice. + segments = kMaxArcSegments; + } + out.reserve(out.size() + static_cast(segments) + 1); + for (int i = 0; i <= segments; ++i) { + const float a = startRad + sweep * (static_cast(i) / static_cast(segments)); + out.push_back(StrokePoint{cx + radius * std::sin(a), cy - radius * std::cos(a)}); + } +} + +} // namespace reasampler::ui diff --git a/src/core/ui/stroke_aa.h b/src/core/ui/stroke_aa.h new file mode 100644 index 0000000..a9d1b6d --- /dev/null +++ b/src/core/ui/stroke_aa.h @@ -0,0 +1,87 @@ +// stroke_aa.h — analytic antialiased thick-stroke coverage: distance-to-polyline, MAX-accumulated +// into a scratch mask that a shell blends ONCE. Pure geometry; no LICE, no host types. +// +// The single blend is the load-bearing part. Compositing a stroke segment-by-segment (or as a +// stack of 1px arcs) re-lays ink over the previous segment's antialiased fringe, which is what +// makes a stroke read as a soft glow that never reaches an opaque core. + +#pragma once + +#include +#include + +#include "core/ui/rect.h" + +namespace reasampler::ui { + +struct StrokePoint { + float x = 0.0f; + float y = 0.0f; +}; + +// Chord-flatness bound for arc flattening: an order of magnitude under the AA fringe the arc is +// drawn with, so the polyline is indistinguishable from the true arc at any radius the deck uses. +inline constexpr float kArcFlatnessPx = 0.05f; + +// Hard cap on flattening output, so a pathological radius/flatness pair terminates with a coarse +// arc rather than allocating without bound. +inline constexpr int kMaxArcSegments = 512; + +// Long segments are split before rasterizing. This is EXACT, not an approximation: the minimum +// distance to a partition of a segment is the minimum distance to the whole segment. It exists +// because a piece is rasterized over its bounding box, and one long diagonal's box has area +// O(len^2) — subdivision is what keeps a stroke's cost linear in its length. +inline constexpr float kMaxPieceLen = 4.0f; + +// A reusable coverage mask. Allocation is amortized across calls: `reset` grows the buffer but +// never clears it, because per-row valid extents make a clear unnecessary. +class StrokeCanvas { + public: + // Grows the buffer to fit `bounds` and marks every row empty. O(height), not O(area). + void reset(const Rect& bounds); + + // MAX-accumulates one segment. Endpoints are round-capped, so a zero-length segment is a dot + // of radius `halfWidth` and a polyline's joints are round by construction. + void addSegment(float ax, float ay, float bx, float by, float halfWidth); + + const Rect& bounds() const { return bounds_; } + + // Only [rowLo, rowHi) of a row holds meaningful coverage; outside that span the buffer is + // deliberately uninitialized, which is what keeps cost proportional to ink, not to the box. + int rowLo(int y) const { return rowLo_[static_cast(y - bounds_.y)]; } + int rowHi(int y) const { return rowHi_[static_cast(y - bounds_.y)]; } + + // Row base pointer; index it by (x - bounds().x) within [rowLo, rowHi). + const float* rowData(int y) const { + return coverage_.data() + + static_cast(y - bounds_.y) * static_cast(bounds_.width); + } + + // Bounds-checked single read — 0 outside the valid span. For tests and cold callers; the + // blend loop walks rows directly. + float coverageAt(int x, int y) const; + + private: + void addPiece(float ax, float ay, float bx, float by, float halfWidth); + void extendRow(int y, int x0, int x1); + + Rect bounds_{}; + std::vector coverage_; + std::vector rowLo_; + std::vector rowHi_; +}; + +// The pixel box a polyline of this half-width can touch, intersected with `clip`. +Rect strokeBounds(const StrokePoint* pts, std::size_t count, float halfWidth, const Rect& clip); + +// A whole stroke in one pass: bounds, reset, every segment MAX-accumulated. Blending the finished +// canvas exactly once is the caller's half of the contract. +void strokePolyline(StrokeCanvas& canvas, const StrokePoint* pts, std::size_t count, + float halfWidth, const Rect& clip); + +// Appends a flattened arc (n+1 points for n chords) to `out`. Angles are radians in LICE's +// convention: 0 is 12 o'clock, increasing clockwise — x = cx + r*sin(a), y = cy - r*cos(a). +void appendArc(std::vector& out, float cx, float cy, float radius, float startRad, + float endRad, float flatnessPx = kArcFlatnessPx); + +} // namespace reasampler::ui diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index c71c743..8155138 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -103,6 +103,7 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h - `reasampler_processor` (`shell/instrument/`: `reasampler_processor.cpp` lifecycle + `process()`, `processor_state.cpp` component-state I/O + UI-thread parameter accessors, `processor_reload.cpp` the off-audio-thread `reloadInstrument`/publish family — Q-W2v, T4-12 split; `process()` and its per-block work stay ONE TU on purpose, no cross-TU call on the per-sample path) — VST3 `SingleComponentEffect` shell: declares event-input bus + **permanently stereo** output (GA fix: dynamic mono↔stereo bus renegotiation deleted; `ChannelMode` is now decode-only), marshals MIDI note-on/off into the VoiceEngine, renders audio; owns off-audio-thread `reloadInstrument` + atomic pointer swap so `process()` does no allocation, no file I/O, no bridge calls. The instance state is `{loaded capture id, one InstrumentParams}`, and `reloadInstrument` resolves + decodes exactly that one capture into the `SampleData` the engine plays. **Self-contained playback (pS):** `ComponentState` v10 adds a `SampleRefs` table — per referenced sample, a project-relative path + decode intrinsics (root, loop, channels, displayName); `reloadInstrument` decodes directly from `SampleRefs`, bank-free (plays with the extension absent). The bank/bridge is a browser source: loading a capture copies its reference in; the reopen-heal timer + poll-to-play apparatus are removed. `retireIdleDrain()` retires fully-idle drain snapshots on the UI-timer cadence. Voice-param edits (`setVoiceCount`/`setVoiceMode`/`setMonoTrigger`) rebuild the engine from the already-decoded `SampleData` via the drain-slot swap — no bank re-read, no WAV re-decode, no audible cut to ringing tails. **FB1:** applies the post-mixer `masterGainLinear` (from `ComponentState` v8) as a per-sample ramp over the summed output — no zipper noise. **GA v9:** `channelModeExplicit_` flag persisted; `channelModeFor()` auto-defaults the mode from the loaded capture's channel count when the flag is not set. **pS:** `ComponentState` bumped v9→v10 (`SampleRefs` table); pre-v10 blobs lift to empty refs and re-save self-contained. **pS-usage:** publishes instance usage (held `SampleRefs` paths) to `rsusage_` at the tail of `reloadInstrument` (off audio thread) via `reaper_bridge::writeUsageExtState`; `ComponentState` bumped v10→**v11** (`instanceGuid` field); pre-v11 blobs mint guid on first publish. - `reasampler_editor` — VST3 `IPlugView` LICE editor shell: hosts a LICE-drawn child window; the Sample face is home and Browse is a modal picker over it. Split on the Sample face's BAND axis, mirroring the pure `sample_bands` allocator: `editor_session` (session/bridge state, caches, commit-and-reload), `editor_controls` (the ONE `faceLayout` band resolve every paint and hit-test path shares, the node-drag bounds, the value labels, and the per-instance controls the parameter set does not carry — the parameter-set binding itself is the pure `core/instrument/ui/deck_values` module this only adapts int ids onto), `editor_models` (the orthogonal half: which stored struct each transient editor selection names — the staged-envelope pack/unpack, the drawn contour, and the three velocity curves), then matching paint and input sets — `editor_paint`/`editor_input` (dispatch + drag router + hover dispatch), `_chrome`, `_waveform`, `_deck` — plus the two band-independent surfaces (`_browse` for the modal picker, `_curve` for the velocity-curve popup) and `editor_platform` (IPlugView/Win32 window plumbing). Shared internals in `editor_internal.h`, no TU of its own. Drop-onto-editor ingest is NOT shipped (deferred). - `reasampler_embed` — implements `IReaperUIEmbedInterface` so the instrument draws inline in the TCP/MCP without a plugin-owned HWND; delegates layout to `embed_strip`. A read-only readout: the loaded capture across the keyboard span with its root marked, plus the activity level. It takes no mouse input (there is nothing on the strip to select). +- `editor_stroke` — the editor's LICE side of the analytic stroker: builds a coverage mask with the pure `core/ui/stroke_aa` and blends it into the bitmap ONCE, writing straight to the bitmap's bits (the arithmetic matches LICE's own mode-0 combine, so a stroke composites identically to every other kit draw). Every radial and spline stroke on the editor routes through `strokeArcAA` / `strokePolylineAA` / `strokeLineAA`. Holds the draw-thread-only scratch mask and arc point list — reuse, not a hidden dependency: threading a canvas through the eight paint sites would grow those signatures to carry an allocation detail. Deliberately does NOT touch `shell/panel/draw_kit`: the waveform stroke, the docked bank panel and the browse cards are out of this seam's blast radius. - `vst_entry` — VST3 entry point: `GetPluginFactory` export, class registration, channel-forked class UIDs. - `editor_internal.h` — INTERNAL shared helpers for the `reasampler_editor` TU family, included only by the editor's own shell TUs (`editor_session` / `editor_controls` / `editor_paint_*` / `editor_input_*` / `editor_platform`), never a public seam: the `Rect`↔kit adapters, small draw primitives (knob face / title band), label helpers, and the velocity-curve box derivation — the helpers more than one band TU needs. The deck's control ids, group ids and group composition are the pure `deck_groups` module's, not this file's. The piano-strip and root-key draws live in `editor_paint_chrome`, their only consumer, not here. - `reasampler_vst.h` — shared identity constants for the ReaSampler VST3 instrument (Phase S): the plugin's class UID (the channel-selected `Steinberg::FUID`, built from the FOREVER-FROZEN macros in `core/wire/reasampler_uid.h`), vendor name/URL/email, so the processor, factory, and editor agree. A class UID is FOREVER-STABLE once shipped — minted once, never regenerated. *(Newly authored per this dispatch's brief — no existing root-CLAUDE.md bullet; verified by reading `src/shell/instrument/reasampler_vst.h` directly.)* diff --git a/src/shell/instrument/CMakeLists.txt b/src/shell/instrument/CMakeLists.txt index 6309fb1..86a2fc5 100644 --- a/src/shell/instrument/CMakeLists.txt +++ b/src/shell/instrument/CMakeLists.txt @@ -64,6 +64,7 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") editor_input_deck.cpp editor_input_browse.cpp editor_input_curve.cpp + editor_stroke.cpp editor_platform.cpp reasampler_embed.cpp reaper_bridge.cpp @@ -87,7 +88,7 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") waveform_view bank_sync browser_scroll param_slider tooltip theme component_geometry bank_grid trigger_seam envelope_overlay envelope_edit knob_deck deck_groups deck_values curve_popup spline_edit master_gain sample_usage - file_bytes curve_law) + file_bytes curve_law stroke_aa) # SDK_INC gives the REAPER VST3 interfaces + API header for the bridge; WDL_INC gives # LICE for the editor. The VST3 SDK headers arrive via vst3_sdk PUBLIC. target_include_directories(reasampler_vst PRIVATE ${REASAMPLER_SRC_DIR} ${SDK_INC} ${WDL_INC}) diff --git a/src/shell/instrument/editor_internal.h b/src/shell/instrument/editor_internal.h index 438492d..561b14a 100644 --- a/src/shell/instrument/editor_internal.h +++ b/src/shell/instrument/editor_internal.h @@ -25,6 +25,7 @@ #include "core/instrument/ui/param_slider.h" // KnobGeometry / KnobArc (drawKnobFace) #include "core/ui/component_geometry.h" // KitBox / waveformColumnCount #include "core/ui/theme.h" // Role / InteractionState / KitColor / spectralColor +#include "shell/instrument/editor_stroke.h" // strokeArcAA / strokePolylineAA #include "shell/panel/draw_kit.h" // the L1 draw kit: fillSurface/text/drawWaveform/toLice #endif @@ -111,11 +112,11 @@ inline void drawTitleBand(LICE_IBitmap* bmp, const instrument::ui::Rect& title, kitText(bmp, titleText, readout.c_str(), Font::Title, ui::Role::TextPrimary); } -// Stroke widths for the radial faces. The value arc is drawn as adjacent 1px AA arcs rather -// than one thick primitive — LICE has no thick-arc call, and stacking radii is what keeps every -// ring antialiased. -inline constexpr int kKnobValueArcPx = 3; -inline constexpr int kInnerDialArcPx = 2; +// Stroke widths for the radial faces. Every arc is ONE analytic stroke (editor_stroke.h) whose +// outer edge sits on the knob's radius, so the centerline is inset by half the width. +inline constexpr float kKnobTrackArcPx = 1.0f; +inline constexpr float kKnobValueArcPx = 3.0f; +inline constexpr float kInnerDialArcPx = 2.0f; inline constexpr int kKnobNeedlePx = 2; // Draws one radial knob face: param_slider owns the value<->angle map; this turns it into @@ -143,27 +144,26 @@ inline void drawKnobFace(LICE_IBitmap* bmp, const instrument::ui::Rect& knobRect const float a0 = static_cast((arc.startDeg - 360.0) * kDegToRad); const float a1 = static_cast( (arc.startDeg + instrument::ui::knobSweepDeg(arc) - 360.0) * kDegToRad); - LICE_Arc(bmp, cx, cy, rOuter, a0, a1, toLice(ui::roleColor(ui::Role::LineHairline)), 1.0f, 0, - true); + strokeArcAA(bmp, cx, cy, rOuter - kKnobTrackArcPx * 0.5f, a0, a1, kKnobTrackArcPx, + toLice(ui::roleColor(ui::Role::LineHairline))); const double v = value01 < 0.0 ? 0.0 : (value01 > 1.0 ? 1.0 : value01); if (v > 0.0) { const float av = static_cast( (arc.startDeg + v * instrument::ui::knobSweepDeg(arc) - 360.0) * kDegToRad); const ui::Role valueRole = disabled ? ui::Role::TextDim : (hot ? ui::Role::AccentHot : ui::Role::AccentPrimary); - const LICE_pixel valueCol = toLice(ui::roleColor(valueRole)); - for (int i = 0; i < kKnobValueArcPx; ++i) { - LICE_Arc(bmp, cx, cy, rOuter - static_cast(i), a0, av, valueCol, 1.0f, 0, true); - } + strokeArcAA(bmp, cx, cy, rOuter - kKnobValueArcPx * 0.5f, a0, av, kKnobValueArcPx, + toLice(ui::roleColor(valueRole))); } - // Needle: from ~35% radius out to the rim at the value's angle. ThickFLine keeps the float - // endpoints AND is always antialiased, so the needle is smooth at every angle. + // Needle: from ~35% radius out to the rim at the value's angle, stroked through the same + // analytic path as the arcs so it holds its weight at every knob position. const KnobPoint tip = instrument::ui::knobNeedlePoint(kg, arc, v); const double ix = kg.centerX + (tip.x - kg.centerX) * 0.35; const double iy = kg.centerY + (tip.y - kg.centerY) * 0.35; const ui::Role needleRole = disabled ? ui::Role::TextDim : ui::Role::TextPrimary; - LICE_ThickFLine(bmp, ix, iy, tip.x, tip.y, toLice(ui::roleColor(needleRole)), 1.0f, 0, - kKnobNeedlePx); + strokeLineAA(bmp, static_cast(ix), static_cast(iy), + static_cast(tip.x), static_cast(tip.y), + static_cast(kKnobNeedlePx), toLice(ui::roleColor(needleRole))); } // The concentric INNER dial: a second value on the same cell, drawn in the categorical @@ -194,15 +194,12 @@ inline void drawInnerDial(LICE_IBitmap* bmp, const instrument::ui::Rect& innerRe (arc.startDeg + v * instrument::ui::knobSweepDeg(arc) - 360.0) * kDegToRad); const ui::Role arcRole = disabled ? ui::Role::TextDim : (hot ? ui::Role::AccentHot : ui::Role::AccentTertiary); - const LICE_pixel arcCol = toLice(ui::roleColor(arcRole)); - for (int i = 0; i < kInnerDialArcPx; ++i) { - LICE_Arc(bmp, cx, cy, r - static_cast(i), a0, av, arcCol, 1.0f, 0, true); - } + strokeArcAA(bmp, cx, cy, r - kInnerDialArcPx * 0.5f, a0, av, kInnerDialArcPx, + toLice(ui::roleColor(arcRole))); const KnobPoint tip = instrument::ui::knobNeedlePoint(kg, arc, v); - LICE_FLine(bmp, static_cast(kg.centerX), static_cast(kg.centerY), - static_cast(tip.x), static_cast(tip.y), - toLice(ui::roleColor(disabled ? ui::Role::TextDim : ui::Role::AccentTertiary)), - 1.0f, 0, true); + strokeLineAA(bmp, static_cast(kg.centerX), static_cast(kg.centerY), + static_cast(tip.x), static_cast(tip.y), 1.0f, + toLice(ui::roleColor(disabled ? ui::Role::TextDim : ui::Role::AccentTertiary))); } #endif // _WIN32 diff --git a/src/shell/instrument/editor_paint_curve.cpp b/src/shell/instrument/editor_paint_curve.cpp index 5b26534..8f01b4b 100644 --- a/src/shell/instrument/editor_paint_curve.cpp +++ b/src/shell/instrument/editor_paint_curve.cpp @@ -6,6 +6,8 @@ #ifdef _WIN32 +#include + #include "core/instrument/ui/curve_popup.h" // centered curve-popup sheet geometry #include "shell/instrument/editor_internal.h" // kit adapters + curveBoxFromRect #include "shell/instrument/reasampler_processor.h" @@ -17,6 +19,10 @@ using namespace reasampler::instrument::ui; // popup geometry namespace { +// The deck thumbnail traces a hairline; the full editor matches the envelope traces' weight. +constexpr float kMiniTracePx = 1.0f; +constexpr float kCurveTracePx = 2.0f; + const char* curveTitle(CurveTarget target) { switch (target) { case CurveTarget::kPitch: return "VELOCITY -> PITCH"; @@ -57,15 +63,15 @@ void ReaSamplerEditor::paintCurveButton(LICE_IBitmap* bmp, const Rect& r, CurveT } const LICE_pixel trace = toLice(roleColor(disabled ? Role::LineHairline : Role::AccentSecondary)); - int prevX = 0, prevY = 0; + static thread_local std::vector pts; + pts.clear(); for (int px = 0; px <= mini.width; ++px) { const int mx = mini.left + px; const double vel = curve.pointFromPixel(mini, mx, mini.top).velocity; - const int my = curve.pixelFromPoint(mini, {vel, curve.eval(vel)}).y; - if (px > 0) LICE_Line(bmp, prevX, prevY, mx, my, trace, 1.0f, 0, true); - prevX = mx; - prevY = my; + const auto p = curve.subpixelFromPoint(mini, {vel, curve.eval(vel)}); + pts.push_back(ui::StrokePoint{static_cast(mx), static_cast(p.y)}); } + strokePolylineAA(bmp, pts, kMiniTracePx, trace); } } @@ -114,19 +120,18 @@ void ReaSamplerEditor::paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r) { // Trace the monotone spline — ONE eval per x column over the mapping box, in the categorical // secondary accent (the same grammar as the envelope trace over the waveform). The x -> // velocity and value -> y mappings both go through the pure module so the trace, the node - // handles, and the hit-test all share one coordinate system. + // handles, and the hit-test all share one coordinate system (sub-pixel for the trace — see + // subpixelFromPoint). const LICE_pixel line = toLice(roleColor(Role::AccentSecondary)); - int prevX = 0, prevY = 0; + static thread_local std::vector pts; + pts.clear(); for (int px = 0; px <= box.width; ++px) { const int cx = box.left + px; const double vel = curve.pointFromPixel(box, cx, box.top).velocity; - const int cy = curve.pixelFromPoint(box, {vel, curve.eval(vel)}).y; - // Same weight and antialiasing the envelope traces use — one trace grammar across every - // curve surface (editor_paint_waveform.cpp owns why ThickFLine and not LICE_Line). - if (px > 0) LICE_ThickFLine(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, 2); - prevX = cx; - prevY = cy; + const auto p = curve.subpixelFromPoint(box, {vel, curve.eval(vel)}); + pts.push_back(ui::StrokePoint{static_cast(cx), static_cast(p.y)}); } + strokePolylineAA(bmp, pts, kCurveTracePx, line); // Draggable node handles (mirror of the envelope overlay's): accent-primary squares lifted // to accent-hot when grabbed or hovered, or warn when a drag-off delete is armed (cursor diff --git a/src/shell/instrument/editor_paint_waveform.cpp b/src/shell/instrument/editor_paint_waveform.cpp index 833b6ec..1338f6e 100644 --- a/src/shell/instrument/editor_paint_waveform.cpp +++ b/src/shell/instrument/editor_paint_waveform.cpp @@ -42,11 +42,9 @@ constexpr int kEnvHandleRadius = 3; constexpr int kEnvHandleGrabbedRadius = 5; constexpr int kEnvHandleRingPx = 2; -// Both envelope traces — staged and drawn — are one grammar and one weight. LICE_ThickFLine is -// ALWAYS antialiased (unlike LICE_Line, whose aa flag does nothing on an axis-aligned run), and -// the second pixel of width is what stops a shallow slope reading as a staircase over the -// waveform behind it. -constexpr int kEnvTracePx = 2; +// Both envelope traces — staged and drawn — are one grammar and one weight. Two pixels is what +// reads as a trace rather than a hairline over the waveform behind it. +constexpr float kEnvTracePx = 2.0f; } // namespace void ReaSamplerEditor::paintWaveform(LICE_IBitmap* bmp, const Rect& band) { @@ -143,21 +141,22 @@ void ReaSamplerEditor::paintSplineOverlay(LICE_IBitmap* bmp, const OverlayArea& if (box.width <= 0 || box.height <= 1) return; const VelocityCurve& curve = splineFor(overlayEnv_); - // One eval per drawn column, through the curve's own pixel maps, so the trace and the - // handles share the coordinate system the hit-test resolves against. + // One eval per drawn column, through the curve's own pixel maps, so the trace and the handles + // share the coordinate system the hit-test resolves against (sub-pixel here — see + // subpixelFromPoint). const LICE_pixel line = toLice(roleColor(Role::OverlayTrace)); - int prevX = 0, prevY = 0; + static thread_local std::vector trace; + trace.clear(); // < not <=: box.left + box.width is the overlay's own EXCLUSIVE right edge (the box has no // inset, unlike the popup's), so a <= column paints one pixel into the next band's pad — // and it is redundant with the clamped endpoint handle below anyway. for (int px = 0; px < box.width; ++px) { const int cx = box.left + px; const double t = curve.pointFromPixel(box, cx, box.top).velocity; - const int cy = curve.pixelFromPoint(box, {t, curve.eval(t)}).y; - if (px > 0) LICE_ThickFLine(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, kEnvTracePx); - prevX = cx; - prevY = cy; + const auto p = curve.subpixelFromPoint(box, {t, curve.eval(t)}); + trace.push_back(ui::StrokePoint{static_cast(cx), static_cast(p.y)}); } + strokePolylineAA(bmp, trace, kEnvTracePx, line); // Handles carry two independent states on the same mark, so they use two independent // channels: SIZE is the grab (the staged painter's grammar — a hotter hue reads as lower @@ -211,18 +210,18 @@ void ReaSamplerEditor::paintEnvelopeOverlay(LICE_IBitmap* bmp, const OverlayArea const StageEnvelope env = packEnvelope(overlayEnv_, params_.play, frames, startFrame); const std::vector poly = buildEnvelopePolyline(env, waveArea, totalSeconds); - // Clip x to the wave rect. Knots are handles, not line vertices. + // Clip x to the wave rect. Knots are handles, not line vertices. Vertices stay INTEGER here + // — unlike the spline traces above — because they are the same positions the draggable + // handles are drawn at, and a sub-pixel trace would sit off its own handles. const LICE_pixel line = toLice(roleColor(Role::OverlayTrace)); - const EnvVertex* prev = nullptr; + static thread_local std::vector trace; + trace.clear(); for (const EnvVertex& v : poly) { if (v.knot) continue; - if (prev != nullptr) { - const int x0 = (std::max)(area.x, (std::min)(area.right() - 1, prev->x)); - const int x1 = (std::max)(area.x, (std::min)(area.right() - 1, v.x)); - LICE_ThickFLine(bmp, x0, prev->y, x1, v.y, line, 1.0f, 0, kEnvTracePx); - } - prev = &v; + const int vx = (std::max)(area.x, (std::min)(area.right() - 1, v.x)); + trace.push_back(ui::StrokePoint{static_cast(vx), static_cast(v.y)}); } + strokePolylineAA(bmp, trace, kEnvTracePx, line); // Handles: a square per draggable stage node, a ROUND knot per curvable segment. Every // vertex is guaranteed in-bounds; the handle is additionally clamped inside the band so one // on an edge node never overhangs into the neighbouring bands. diff --git a/src/shell/instrument/editor_stroke.cpp b/src/shell/instrument/editor_stroke.cpp new file mode 100644 index 0000000..7a50ce1 --- /dev/null +++ b/src/shell/instrument/editor_stroke.cpp @@ -0,0 +1,83 @@ +#include "shell/instrument/editor_stroke.h" + +#ifdef _WIN32 + +#include + +namespace reasampler::vst { +namespace { + +// Draw-thread-only scratch. It lives here rather than on the editor because a stroke is a leaf +// draw call reached from eight paint sites — threading a canvas through every one of them would +// grow those signatures to carry an allocation detail. Reuse is the point: after the first paint +// the mask and the point list are resized, never reallocated. +thread_local ui::StrokeCanvas g_canvas; +thread_local std::vector g_arcPoints; + +// One blend of the finished mask. The arithmetic matches LICE's own mode-0 combine +// (src + (dst-src)*(256-a)/256 on all four channels, alpha in .8 fixed point) so a stroke +// composites identically to every other kit draw on the same surface — written straight to the +// bitmap's bits rather than through LICE_PutPixel, which re-derives the row pointer per pixel. +void blendCanvas(LICE_IBitmap* bmp, const ui::StrokeCanvas& canvas, LICE_pixel color, + float alpha) { + const ui::Rect& b = canvas.bounds(); + if (b.empty() || alpha <= 0.0f) return; + LICE_pixel* const bits = bmp->getBits(); + const int span = bmp->getRowSpan(); + if (bits == nullptr || span <= 0) return; + const bool flipped = bmp->isFlipped(); + const int lastRow = bmp->getHeight() - 1; + + const int sr = LICE_GETR(color); + const int sg = LICE_GETG(color); + const int sb = LICE_GETB(color); + const int sa = LICE_GETA(color); + for (int y = b.y; y < b.bottom(); ++y) { + const int lo = canvas.rowLo(y); + const int hi = canvas.rowHi(y); + if (hi <= lo) continue; + const float* const cov = canvas.rowData(y); + LICE_pixel* const row = + bits + static_cast(flipped ? lastRow - y : y) * + static_cast(span) + static_cast(b.x); + for (int i = lo; i < hi; ++i) { + const int ia = static_cast(cov[i] * alpha * 256.0f); + if (ia <= 0) continue; + if (ia >= 256) { + row[i] = color; + continue; + } + const int sc = 256 - ia; + LICE_pixel_chan* const d = reinterpret_cast(row + i); + d[LICE_PIXEL_R] = static_cast(sr + ((d[LICE_PIXEL_R] - sr) * sc) / 256); + d[LICE_PIXEL_G] = static_cast(sg + ((d[LICE_PIXEL_G] - sg) * sc) / 256); + d[LICE_PIXEL_B] = static_cast(sb + ((d[LICE_PIXEL_B] - sb) * sc) / 256); + d[LICE_PIXEL_A] = static_cast(sa + ((d[LICE_PIXEL_A] - sa) * sc) / 256); + } + } +} + +ui::Rect bitmapRect(LICE_IBitmap* bmp) { + return ui::Rect{0, 0, bmp->getWidth(), bmp->getHeight()}; +} + +} // namespace + +void strokePolylineAA(LICE_IBitmap* bmp, const ui::StrokePoint* pts, std::size_t count, + float widthPx, LICE_pixel color, float alpha) { + if (bmp == nullptr || pts == nullptr || count == 0 || widthPx <= 0.0f) return; + ui::strokePolyline(g_canvas, pts, count, widthPx * 0.5f, bitmapRect(bmp)); + blendCanvas(bmp, g_canvas, color, alpha); +} + +void strokeArcAA(LICE_IBitmap* bmp, float cx, float cy, float radius, float startRad, float endRad, + float widthPx, LICE_pixel color, float alpha) { + if (bmp == nullptr || widthPx <= 0.0f) return; + g_arcPoints.clear(); + ui::appendArc(g_arcPoints, cx, cy, radius, startRad, endRad); + strokePolylineAA(bmp, g_arcPoints.data(), g_arcPoints.size(), widthPx, color, alpha); +} + +} // namespace reasampler::vst + +#endif // _WIN32 diff --git a/src/shell/instrument/editor_stroke.h b/src/shell/instrument/editor_stroke.h new file mode 100644 index 0000000..338dfaa --- /dev/null +++ b/src/shell/instrument/editor_stroke.h @@ -0,0 +1,50 @@ +// editor_stroke.h — the editor's LICE side of the analytic stroker: build a coverage mask with +// the pure `core/ui/stroke_aa` module, blend it into the bitmap ONCE. +// +// Every radial and spline stroke on the editor goes through here. LICE's own primitives cannot +// serve these two shapes: LICE_Arc rasterizes a whole circle clipped per 90-degree box and splits +// one unit of ink across two pixels by the radius's fractional part (so a stacked-radii arc never +// reaches an opaque core), and LICE_ThickFLine lays its width along the MINOR axis (so a curve's +// perpendicular weight falls off as cos(theta) and thins at every diagonal). + +#pragma once + +#ifdef _WIN32 + +#include +#include + +#include "lice/lice.h" +#include "wdltypes.h" + +#include "core/ui/rect.h" +#include "core/ui/stroke_aa.h" + +namespace reasampler::vst { + +// Blends a polyline as one antialiased stroke of `widthPx`. Clipped to the bitmap. Takes a raw +// span so a short stroke (a knob needle) can pass a stack array and cost no allocation. +void strokePolylineAA(LICE_IBitmap* bmp, const ui::StrokePoint* pts, std::size_t count, + float widthPx, LICE_pixel color, float alpha = 1.0f); + +inline void strokePolylineAA(LICE_IBitmap* bmp, const std::vector& pts, + float widthPx, LICE_pixel color, float alpha = 1.0f) { + strokePolylineAA(bmp, pts.data(), pts.size(), widthPx, color, alpha); +} + +// A single antialiased segment — the two-point polyline, spelled out for the call sites that +// have exactly two endpoints. +inline void strokeLineAA(LICE_IBitmap* bmp, float x0, float y0, float x1, float y1, float widthPx, + LICE_pixel color, float alpha = 1.0f) { + const ui::StrokePoint pts[2] = {{x0, y0}, {x1, y1}}; + strokePolylineAA(bmp, pts, 2, widthPx, color, alpha); +} + +// Blends a circular arc as one antialiased stroke of `widthPx`. Angles are radians in LICE's +// convention (0 = 12 o'clock, increasing clockwise), matching what LICE_Arc took. +void strokeArcAA(LICE_IBitmap* bmp, float cx, float cy, float radius, float startRad, float endRad, + float widthPx, LICE_pixel color, float alpha = 1.0f); + +} // namespace reasampler::vst + +#endif // _WIN32 diff --git a/tests/test_stroke_aa.cpp b/tests/test_stroke_aa.cpp new file mode 100644 index 0000000..a4d7609 --- /dev/null +++ b/tests/test_stroke_aa.cpp @@ -0,0 +1,404 @@ +// Standalone tests for reasampler::ui::stroke_aa — no REAPER, no LICE, no framework. +// Same fast assert loop as the sibling pure tests. +// +// The three properties here are the ones the shipped LICE draws failed, so each is asserted as a +// NUMBER rather than eyeballed: an opaque core (LICE_Arc's AA circle splits one unit of ink across +// two pixels by the radius's fraction, so no pixel ever reached 255); perpendicular weight that +// does not vary with angle (LICE_ThickFLine lays its width along the minor axis, rippling 42% +// around a knob sweep); and MAX-into-scratch accumulation (per-segment blending re-lays ink over +// the previous segment's fringe, which is what made a stroke read as a glow). + +#include "../src/core/ui/stroke_aa.h" + +#include +#include +#include + +using namespace reasampler; +using namespace reasampler::ui; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static const Rect kBig{0, 0, 240, 240}; +static constexpr float kPi = 3.14159265358979323846f; + +// --- helpers ----------------------------------------------------------------- + +static float peakCoverage(const StrokeCanvas& c) { + float peak = 0.0f; + const Rect& b = c.bounds(); + for (int y = b.y; y < b.bottom(); ++y) + for (int x = b.x; x < b.right(); ++x) + if (c.coverageAt(x, y) > peak) peak = c.coverageAt(x, y); + return peak; +} + +static float totalInk(const StrokeCanvas& c) { + float sum = 0.0f; + const Rect& b = c.bounds(); + for (int y = b.y; y < b.bottom(); ++y) + for (int x = b.x; x < b.right(); ++x) sum += c.coverageAt(x, y); + return sum; +} + +// Ink per unit length across a window in the MIDDLE of a straight stroke, binned by each pixel's +// projection onto the stroke direction. This is literally "perpendicular weight": for a stroke of +// half-width hw the answer is 2*hw at every angle, and it is the measurement that fails against a +// minor-axis-width primitive. +static float perpendicularWeight(float angleDeg, float halfWidth) { + const float a = angleDeg * kPi / 180.0f; + const float dx = std::cos(a), dy = std::sin(a); + const float cx = 120.0f, cy = 120.0f; + const float half = 90.0f; // stroke reaches well past the window on both sides + const float win = 40.0f; // window half-length, clear of both round caps + const StrokePoint pts[2] = {{cx - dx * half, cy - dy * half}, + {cx + dx * half, cy + dy * half}}; + StrokeCanvas c; + strokePolyline(c, pts, 2, halfWidth, kBig); + float sum = 0.0f; + const Rect& b = c.bounds(); + for (int y = b.y; y < b.bottom(); ++y) { + for (int x = b.x; x < b.right(); ++x) { + const float s = (static_cast(x) + 0.5f - cx) * dx + + (static_cast(y) + 0.5f - cy) * dy; + if (s >= -win && s < win) sum += c.coverageAt(x, y); + } + } + return sum / (2.0f * win); +} + +// --- coverage / distance math ------------------------------------------------ + +static void testStraightStrokeHasAnOpaqueCore() { + // The shipped defect stated numerically: peak alpha must reach full, not 137-192/255. + for (float w : {2.0f, 3.0f}) { + for (float deg : {0.0f, 17.0f, 45.0f, 63.0f, 90.0f}) { + const float a = deg * kPi / 180.0f; + const StrokePoint pts[2] = {{120.0f - 80.0f * std::cos(a), 120.0f - 80.0f * std::sin(a)}, + {120.0f + 80.0f * std::cos(a), 120.0f + 80.0f * std::sin(a)}}; + StrokeCanvas c; + strokePolyline(c, pts, 2, w * 0.5f, kBig); + CHECK(peakCoverage(c) >= 0.999f); + } + } +} + +static void testPerpendicularWeightIsAngleIndependent() { + // The criterion that killed the ThickFLine option: it dips to wid*cos(theta) at every 45 + // degrees. An axis-aligned-only sample would pass against it, so sample the diagonals. + for (float w : {2.0f, 3.0f}) { + float lo = 1e9f, hi = -1e9f; + for (float deg = 0.0f; deg <= 90.0f; deg += 7.5f) { + const float m = perpendicularWeight(deg, w * 0.5f); + if (m < lo) lo = m; + if (m > hi) hi = m; + CHECK(std::fabs(m - w) < 0.06f * w); // within 6% of nominal at every angle + } + CHECK((hi - lo) / w < 0.08f); // and the spread across angles is under 8% + } +} + +static void testWeightHoldsAtTheExactDiagonal() { + // Pinned separately because 45 degrees is where the rejected primitive was worst (0.707x). + const float m = perpendicularWeight(45.0f, 1.0f); + CHECK(m > 1.88f && m < 2.12f); +} + +static void testZeroLengthSegmentIsARoundDot() { + StrokeCanvas c; + c.reset(kBig); + c.addSegment(60.0f, 60.0f, 60.0f, 60.0f, 1.5f); // degenerate: start == end + CHECK(peakCoverage(c) >= 0.999f); + // Radially symmetric about the point, and zero well outside the reach. + CHECK(std::fabs(c.coverageAt(58, 60) - c.coverageAt(61, 60)) < 1e-5f); + CHECK(std::fabs(c.coverageAt(60, 58) - c.coverageAt(60, 61)) < 1e-5f); + CHECK(c.coverageAt(65, 60) == 0.0f); + CHECK(c.coverageAt(60, 65) == 0.0f); +} + +static void testZeroLengthPolylineOfOnePointDraws() { + const StrokePoint one[1] = {{40.0f, 40.0f}}; + StrokeCanvas c; + strokePolyline(c, one, 1, 1.5f, kBig); + CHECK(!c.bounds().empty()); + CHECK(peakCoverage(c) >= 0.999f); +} + +static void testVerticalSegmentIsContinuousAndFullWeight() { + // Infinite slope: dx == 0 exactly, the case an x-stepping rasterizer cannot express. + const StrokePoint pts[2] = {{100.0f, 20.0f}, {100.0f, 200.0f}}; + StrokeCanvas c; + strokePolyline(c, pts, 2, 1.0f, kBig); + for (int y = 25; y < 195; ++y) { + float rowPeak = 0.0f, rowInk = 0.0f; + for (int x = 90; x < 110; ++x) { + rowPeak = c.coverageAt(x, y) > rowPeak ? c.coverageAt(x, y) : rowPeak; + rowInk += c.coverageAt(x, y); + } + CHECK(rowPeak >= 0.999f); // no gap, no weak row + CHECK(std::fabs(rowInk - 2.0f) < 0.02f); // and uniform weight down the whole run + } +} + +static void testNearVerticalSegmentIsContinuous() { + // The slope that broke into dots on screen: steep but not exactly vertical, so the old + // integer-y loop quantized it into alternating 1/2-px steps. + const StrokePoint pts[2] = {{100.0f, 20.0f}, {103.0f, 200.0f}}; + StrokeCanvas c; + strokePolyline(c, pts, 2, 1.0f, kBig); + for (int y = 25; y < 195; ++y) { + float rowPeak = 0.0f; + for (int x = 90; x < 115; ++x) + rowPeak = c.coverageAt(x, y) > rowPeak ? c.coverageAt(x, y) : rowPeak; + CHECK(rowPeak >= 0.999f); + } +} + +static void testCoverageFallsOffOverExactlyOnePixel() { + // The AA fringe is one pixel wide by construction: cov = clamp(hw + 0.5 - d, 0, 1). With the + // centreline on an integer y, pixel centres sit at d = 0.5, 1.5, 2.5 — one saturated row, one + // exactly-half fringe row, then nothing. + const StrokePoint pts[2] = {{20.0f, 100.0f}, {220.0f, 100.0f}}; + StrokeCanvas c; + strokePolyline(c, pts, 2, 1.5f, kBig); + CHECK(std::fabs(c.coverageAt(120, 99) - 1.0f) < 1e-4f); // d = 0.5 -> saturated + CHECK(std::fabs(c.coverageAt(120, 98) - 0.5f) < 1e-4f); // d = 1.5 -> half + CHECK(c.coverageAt(120, 97) == 0.0f); // d = 2.5 -> past the reach +} + +// --- accumulation semantics -------------------------------------------------- + +static void testOverlappingSegmentsTakeTheMaxNotTheSum() { + // The whole point of the scratch mask: an overlap must not read brighter than one stroke, or + // the joints of a 500-segment contour build up into a glow. + StrokeCanvas one; + one.reset(kBig); + one.addSegment(40.0f, 100.3f, 160.0f, 100.3f, 1.0f); + + // Locate a genuinely partial fringe pixel rather than assuming which row it lands on — the + // assertion below is only meaningful on a pixel that is neither empty nor already saturated. + int fy = -1; + for (int y = 95; y < 106; ++y) { + const float v = one.coverageAt(100, y); + if (v > 0.05f && v < 0.95f) { fy = y; break; } + } + CHECK(fy != -1); + if (fy == -1) return; + const float soloEdge = one.coverageAt(100, fy); + + StrokeCanvas both; + both.reset(kBig); + both.addSegment(40.0f, 100.3f, 160.0f, 100.3f, 1.0f); + both.addSegment(40.0f, 100.3f, 160.0f, 100.3f, 1.0f); // exactly on top of the first + CHECK(std::fabs(both.coverageAt(100, fy) - soloEdge) < 1e-6f); + CHECK(totalInk(both) <= totalInk(one) + 1e-3f); +} + +static void testNoPixelEverExceedsFullCoverage() { + // Many mutually overlapping segments through one point — the pile-up case. + StrokeCanvas c; + c.reset(kBig); + for (int i = 0; i < 12; ++i) { + const float a = static_cast(i) * kPi / 12.0f; + c.addSegment(120.0f - 60.0f * std::cos(a), 120.0f - 60.0f * std::sin(a), + 120.0f + 60.0f * std::cos(a), 120.0f + 60.0f * std::sin(a), 1.5f); + } + const Rect& b = c.bounds(); + for (int y = b.y; y < b.bottom(); ++y) + for (int x = b.x; x < b.right(); ++x) CHECK(c.coverageAt(x, y) <= 1.0f); +} + +static void testRevisitedRowGapReadsZeroNotGarbage() { + // A circle touches most rows on BOTH sides, leaving an untouched gap between the two spans. + // The row's valid extent grows over that gap, so the gap must be zero-filled, not left at + // whatever the reused scratch buffer held. + StrokeCanvas c; + c.reset(kBig); + c.addSegment(30.0f, 100.0f, 30.0f, 140.0f, 1.5f); // dirty the buffer on the left + c.reset(kBig); + std::vector ring; + appendArc(ring, 120.0f, 120.0f, 60.0f, 0.0f, 2.0f * kPi); + strokePolyline(c, ring.data(), ring.size(), 1.5f, kBig); + for (int x = 100; x < 140; ++x) CHECK(c.coverageAt(x, 120) == 0.0f); // hollow middle +} + +// --- bounds / clipping ------------------------------------------------------- + +static void testBoundsClipToTheClipRectAndCoverTheReach() { + const StrokePoint pts[2] = {{10.0f, 10.0f}, {50.0f, 50.0f}}; + const Rect b = strokeBounds(pts, 2, 1.5f, kBig); + CHECK(b.x <= 8 && b.y <= 8); + CHECK(b.right() >= 52 && b.bottom() >= 52); + + const Rect clipped = strokeBounds(pts, 2, 1.5f, Rect{20, 20, 10, 10}); + CHECK(clipped.x == 20 && clipped.y == 20); + CHECK(clipped.right() == 30 && clipped.bottom() == 30); + + const StrokePoint away[2] = {{500.0f, 500.0f}, {600.0f, 600.0f}}; + CHECK(strokeBounds(away, 2, 1.5f, kBig).empty()); // wholly outside -> nothing to draw +} + +static void testStrokeEntirelyOutsideTheClipDrawsNothing() { + const StrokePoint away[2] = {{500.0f, 500.0f}, {600.0f, 600.0f}}; + StrokeCanvas c; + strokePolyline(c, away, 2, 1.5f, kBig); + CHECK(c.bounds().empty()); +} + +static void testCoverageOutsideTheValidSpanReadsZero() { + StrokeCanvas c; + c.reset(kBig); + c.addSegment(100.0f, 100.0f, 140.0f, 100.0f, 1.0f); + CHECK(c.coverageAt(10, 100) == 0.0f); // same row, outside the touched span + CHECK(c.coverageAt(120, 10) == 0.0f); // an untouched row entirely + CHECK(c.coverageAt(-5, 100) == 0.0f); // outside the canvas + CHECK(c.coverageAt(1000, 1000) == 0.0f); +} + +// --- long-segment subdivision ------------------------------------------------ + +static void testSubdivisionDoesNotChangeTheRenderedStroke() { + // Pieces exist to keep each bounding box tight; min-distance to a partition IS min-distance + // to the whole, so the output must be identical to the same span drawn in short hops. + const StrokePoint whole[2] = {{20.0f, 20.0f}, {200.0f, 140.0f}}; + StrokeCanvas a; + strokePolyline(a, whole, 2, 1.0f, kBig); + + std::vector hops; + for (int i = 0; i <= 180; ++i) { + const float t = static_cast(i) / 180.0f; + hops.push_back(StrokePoint{20.0f + 180.0f * t, 20.0f + 120.0f * t}); + } + StrokeCanvas b; + strokePolyline(b, hops.data(), hops.size(), 1.0f, kBig); + + CHECK(a.bounds() == b.bounds()); + float worst = 0.0f; + for (int y = kBig.y; y < kBig.bottom(); ++y) + for (int x = kBig.x; x < kBig.right(); ++x) { + const float d = std::fabs(a.coverageAt(x, y) - b.coverageAt(x, y)); + if (d > worst) worst = d; + } + // Not exactly zero: the two paths split the line at different parameter values, so the + // projections differ in the last float bits. Measured worst case 1.6e-5 — a rounding + // difference, not a coverage one, and the identical bounds above rule out a clipping gap. + CHECK(worst < 1e-4f); +} + +// --- arc flattening ---------------------------------------------------------- + +static void testArcPointsLieOnTheCircleAndRespectTheFlatness() { + std::vector pts; + appendArc(pts, 100.0f, 100.0f, 17.5f, -2.618f, 2.618f); // the knob's 300-degree sweep + CHECK(pts.size() >= 3); + for (const StrokePoint& p : pts) { + const float r = std::sqrt((p.x - 100.0f) * (p.x - 100.0f) + + (p.y - 100.0f) * (p.y - 100.0f)); + CHECK(std::fabs(r - 17.5f) < 1e-2f); + } + // Chord sagitta stays inside the requested flatness, with a little numeric slack. + for (std::size_t i = 1; i < pts.size(); ++i) { + const float mx = 0.5f * (pts[i - 1].x + pts[i].x); + const float my = 0.5f * (pts[i - 1].y + pts[i].y); + const float rm = std::sqrt((mx - 100.0f) * (mx - 100.0f) + (my - 100.0f) * (my - 100.0f)); + CHECK(17.5f - rm < kArcFlatnessPx * 1.5f); + } +} + +static void testArcDensityGrowsWithRadius() { + std::vector small, large; + appendArc(small, 0.0f, 0.0f, 10.0f, 0.0f, kPi); + appendArc(large, 0.0f, 0.0f, 200.0f, 0.0f, kPi); + CHECK(large.size() > small.size()); +} + +static void testArcFlatteningTerminatesOnDegenerateInputs() { + std::vector pts; + appendArc(pts, 10.0f, 10.0f, 0.0f, 0.0f, kPi); // zero radius + CHECK(pts.size() == 1); + + pts.clear(); + appendArc(pts, 0.0f, 0.0f, 20.0f, 1.0f, 1.0f); // zero sweep + CHECK(pts.size() == 2 && std::fabs(pts[0].x - pts[1].x) < 1e-5f); + + pts.clear(); + appendArc(pts, 0.0f, 0.0f, 20.0f, 0.0f, kPi, 0.0f); // flatness 0 falls back + CHECK(!pts.empty() && + pts.size() <= static_cast(kMaxArcSegments) + 1); + + pts.clear(); + appendArc(pts, 0.0f, 0.0f, 20.0f, 0.0f, kPi, -3.0f); // negative flatness falls back + CHECK(!pts.empty() && + pts.size() <= static_cast(kMaxArcSegments) + 1); + + pts.clear(); + appendArc(pts, 0.0f, 0.0f, 1.0e9f, 0.0f, 2.0f * kPi, 1e-6f); // the cap is the guarantee + CHECK(pts.size() <= static_cast(kMaxArcSegments) + 1); + + pts.clear(); + appendArc(pts, 0.0f, 0.0f, 4.0f, 0.0f, kPi, 50.0f); // flatness past the diameter + CHECK(pts.size() >= 2 && pts.size() <= 8); +} + +// --- an arc as a rendered stroke --------------------------------------------- + +static void testKnobArcIsOpaqueAndEvenAllTheWayRound() { + // The shipped defect, measured: 27 of 33 columns never reached full opacity and column ink + // ran 1.60-3.13 px against a nominal 3. Sweep radially instead of by column so the check is + // a true perpendicular cut at every angle, including the cardinals the old code was worst at. + const float cx = 120.0f, cy = 120.0f, radius = 16.0f, width = 3.0f; + std::vector pts; + appendArc(pts, cx, cy, radius, -2.618f, 2.618f); + StrokeCanvas c; + strokePolyline(c, pts.data(), pts.size(), width * 0.5f, kBig); + + float lo = 1e9f, hi = -1e9f; + for (int i = 0; i < 72; ++i) { + const float a = -2.5f + (5.0f * static_cast(i)) / 71.0f; // inside the sweep + const float ux = std::sin(a), uy = -std::cos(a); + float ink = 0.0f, peak = 0.0f; + // Integrate along the radial ray in fine steps, converting to a per-pixel weight. + constexpr int kSteps = 400; + constexpr float kSpan = 8.0f; // radial window centred on the arc + for (int s = 0; s < kSteps; ++s) { + const float r = radius - kSpan * 0.5f + kSpan * static_cast(s) / kSteps; + const int px = static_cast(std::floor(cx + ux * r)); + const int py = static_cast(std::floor(cy + uy * r)); + const float v = c.coverageAt(px, py); + ink += v * (kSpan / kSteps); + if (v > peak) peak = v; + } + CHECK(peak >= 0.999f); // an opaque core at EVERY angle + if (ink < lo) lo = ink; + if (ink > hi) hi = ink; + } + CHECK(lo > width * 0.90f); + CHECK(hi < width * 1.10f); + CHECK((hi - lo) / width < 0.15f); +} + +int main() { + testStraightStrokeHasAnOpaqueCore(); + testPerpendicularWeightIsAngleIndependent(); + testWeightHoldsAtTheExactDiagonal(); + testZeroLengthSegmentIsARoundDot(); + testZeroLengthPolylineOfOnePointDraws(); + testVerticalSegmentIsContinuousAndFullWeight(); + testNearVerticalSegmentIsContinuous(); + testCoverageFallsOffOverExactlyOnePixel(); + testOverlappingSegmentsTakeTheMaxNotTheSum(); + testNoPixelEverExceedsFullCoverage(); + testRevisitedRowGapReadsZeroNotGarbage(); + testBoundsClipToTheClipRectAndCoverTheReach(); + testStrokeEntirelyOutsideTheClipDrawsNothing(); + testCoverageOutsideTheValidSpanReadsZero(); + testSubdivisionDoesNotChangeTheRenderedStroke(); + testArcPointsLieOnTheCircleAndRespectTheFlatness(); + testArcDensityGrowsWithRadius(); + testArcFlatteningTerminatesOnDegenerateInputs(); + testKnobArcIsOpaqueAndEvenAllTheWayRound(); + if (g_fail == 0) std::printf("test_stroke_aa: all tests passed\n"); + return g_fail == 0 ? 0 : 1; +} diff --git a/tests/test_velocity_curve.cpp b/tests/test_velocity_curve.cpp index 69c5db5..fdd5a2c 100644 --- a/tests/test_velocity_curve.cpp +++ b/tests/test_velocity_curve.cpp @@ -391,6 +391,48 @@ static void testPixelFromPointMapsCornersAndMidpoint() { CHECK(clamped.x == 110 && clamped.y == 20); } +static void testSubpixelMapIsTheIntegerMapBeforeRounding() { + // The antialiased trace draws off subpixelFromPoint while the hit-test still resolves against + // pixelFromPoint. If the two were separate formulas the trace would drift off its own handles, + // so pin the relationship rather than the sub-pixel values: int == round(subpixel), everywhere. + const Box boxes[] = {{10, 20, 100, 51}, {0, 0, 127, 101}, {7, 3, 33, 17}}; + for (const Box& box : boxes) { + for (const VelocityCurve& c : {uni(), bip()}) { + for (double v = 0.0; v <= 127.0; v += 1.0) { + for (double a = -1.0; a <= 1.0; a += 0.125) { + const auto ip = c.pixelFromPoint(box, {v, a}); + const auto fp = c.subpixelFromPoint(box, {v, a}); + CHECK(ip.x == box.left + static_cast(fp.x - box.left + 0.5)); + CHECK(ip.y == box.top + static_cast(fp.y - box.top + 0.5)); + } + } + } + } +} + +static void testSubpixelMapResolvesSlopesTheIntegerMapFlattens() { + // The defect this exists for: adjacent columns of a gentle slope round to the SAME integer + // row, so an integer-only trace is a staircase. The sub-pixel map must separate them. + const Box box{0, 0, 400, 101}; + const VelocityCurve c = VelocityCurve::linear(); + int identicalIntRows = 0; + double maxSubpixelStep = 0.0, minSubpixelStep = 1e18; + for (int x = 1; x <= 200; ++x) { + const double v0 = c.pointFromPixel(box, x - 1, box.top).velocity; + const double v1 = c.pointFromPixel(box, x, box.top).velocity; + const int y0 = c.pixelFromPoint(box, {v0, c.eval(v0)}).y; + const int y1 = c.pixelFromPoint(box, {v1, c.eval(v1)}).y; + if (y0 == y1) ++identicalIntRows; + const double d = std::fabs(c.subpixelFromPoint(box, {v1, c.eval(v1)}).y - + c.subpixelFromPoint(box, {v0, c.eval(v0)}).y); + if (d > maxSubpixelStep) maxSubpixelStep = d; + if (d < minSubpixelStep) minSubpixelStep = d; + } + CHECK(identicalIntRows > 100); // the integer map really does flatten + CHECK(maxSubpixelStep - minSubpixelStep < 1e-9); // the sub-pixel one advances evenly + CHECK(maxSubpixelStep > 0.0); +} + static void testPointFromPixelInvertsAndClamps() { const Box box{10, 20, 100, 51}; // Exact corners invert exactly. @@ -472,6 +514,8 @@ int main() { testBipolarPixelMapPutsZeroOnTheCentreLine(); testBipolarDragCoversTwiceTheValueRange(); testPixelFromPointMapsCornersAndMidpoint(); + testSubpixelMapIsTheIntegerMapBeforeRounding(); + testSubpixelMapResolvesSlopesTheIntegerMapFlattens(); testPointFromPixelInvertsAndClamps(); testPixelMapsRoundTripWithinOnePixelQuantum(); testPixelFromPointAgreesWithPointAtPixel(); From 3fb77027c66b56091d45d828a8c5a276c4a6c9c1 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 13:45:43 -0400 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20close=20=CE=98-W7-T1=20review=20?= =?UTF-8?q?=E2=80=94=20scaling=20guard,=20opacity=20claims,=20two=20vacuou?= =?UTF-8?q?s=20test=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guards the stroke blend against LICE_EXT_GET_SCALING, tightens the analytic-stroker's boxes and NaN handling, corrects the opaque-core threshold and inner-dial rationale in the docs, and re-derives two review-flagged tautological tests so they actually fail against the bugs they claim to catch. --- docs/product/visual-design-language.md | 11 ++-- src/core/ui/CLAUDE.md | 2 +- src/core/ui/stroke_aa.cpp | 35 +++++++++--- src/core/ui/stroke_aa.h | 10 ++++ src/shell/instrument/editor_internal.h | 6 +- src/shell/instrument/editor_paint_curve.cpp | 4 +- .../instrument/editor_paint_waveform.cpp | 13 +++-- src/shell/instrument/editor_stroke.cpp | 41 +++++++++++++- src/shell/instrument/editor_stroke.h | 9 +++ tests/test_stroke_aa.cpp | 56 +++++++++++++++++-- tests/test_velocity_curve.cpp | 24 ++++++++ 11 files changed, 181 insertions(+), 30 deletions(-) diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index 74b117d..afa4691 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -808,9 +808,11 @@ concession. Everything with a slope or a curve must draw through a primitive tha | Surface | Where | Disposition | |---|---|---| -| Radial knob track + value arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — the stacked-radius `LICE_Arc` ring never reached an opaque core and rippled 67% in weight. Now ONE analytic stroke (`strokeArcAA`), outer edge on the knob radius. Measured: peak **255/255** at every cross-section, weight **2.95–3.11 px** (5% ripple). | +| Radial knob track arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — the stacked-radius `LICE_Arc` ring never reached an opaque core. Now ONE analytic stroke (`strokeArcAA`) at `kKnobTrackArcPx` = 1 px, **below the ≥2 px opaque-core threshold** (`core/ui/CLAUDE.md`) — peak alpha still modulates with the arc's exact pixel-grid alignment (measured ~128–255/255 across the sweep) rather than pinning to 255. An improvement over the old stacked-radius ripple, not full opacity; see `test_stroke_aa.cpp`'s 1 px case for the pinned behaviour. | +| Radial knob value arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — same stroke, `kKnobValueArcPx` = 3 px, clear of the opaque-core threshold. Measured: peak **255/255** at every cross-section, weight **2.95–3.11 px** (5% ripple). | | Knob needle | `drawKnobFace` | **Fixed (2026-08-01)** — `LICE_ThickFLine`'s minor-axis width thinned it to `cos θ` as the knob swept. Now `strokeLineAA`, 2 px. | -| Inner curve dial arc + needle | `drawInnerDial` | **Fixed (2026-08-01)** — same as the knob: one analytic 2 px arc; needle via `strokeLineAA`. | +| Inner curve dial arc | `drawInnerDial` | **Fixed (2026-08-01)** — the arc shared the knob track/value arc's stacked-radius opacity defect. Same one analytic fix, at `kInnerDialArcPx` = 2 px (at the opaque-core threshold). | +| Inner curve dial needle | `drawInnerDial` | **Converted (2026-08-01), not a defect fix** — this needle was already `LICE_FLine` (float endpoints, always AA), not `LICE_ThickFLine`; a 1 px AA line has no width to lay along a minor axis, so it never had the knob needle's `cos θ` defect. Moved to `strokeLineAA` anyway for one-seam consistency with every other stroke on the editor, not because it was broken. | | Staged envelope segment slopes | `editor_paint_waveform.cpp` | **Fixed (2026-08-01)** — one `strokePolylineAA` over the whole polyline, so the stage joints blend once. Vertices stay INTEGER by design: they are the positions the draggable handles are drawn at. | | Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed (2026-08-01)** — the trace was never gapped; it was fully aliased (every pixel full or empty) because the loop passed INTEGER `cy`, quantizing the slope into alternating 1/2 px steps. Now sub-pixel y (`subpixelFromPoint`) through `strokePolylineAA`. Measured: peak **255/255**, weight **1.95–2.01 px** (3% ripple). | | Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed (2026-08-01)** — same cause, same treatment. | @@ -832,9 +834,8 @@ concession. Everything with a slope or a curve must draw through a primitive tha **Analytic stroker cost** (Release, MSVC, real LICE, one-off scratchpad harness 2026-08-01, not committed — re-measure before relying on it): 30 knob arcs **0.113 ms → 0.169 ms**; a 500 px spline contour **0.013 ms → 0.047 ms**. About +0.09 ms per full editor repaint, on a -surface that repaints on interaction rather than continuously. Both figures beat the -prototype's targets (0.285 ms / 0.106 ms). Micro-optimisation, each lever measured in -isolation: writing the blend straight to the bitmap's bits rather than through +surface that repaints on interaction rather than continuously. Micro-optimisation, each lever +measured in isolation: writing the blend straight to the bitmap's bits rather than through `LICE_PutPixel` is the big one (arcs 0.169 vs 0.253 ms); reusing the scratch mask across calls matters on the contour's large bounding box (0.047 vs 0.073 ms); `float` over `double` is small but real (contour coverage 0.045 vs 0.051 ms). The per-row valid-extent bookkeeping diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index 0f334da..6e110d7 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -101,7 +101,7 @@ L7 sub-pass, 2026-07-27): - `tooltip` — pure tooltip placement + prefix-strip: strips the `ReaSampler:` display prefix from the registered action phrase; width clamped to the client rect. - `card_drag` — pure drag-gesture precedence + slot hit-test: leave-client → OS drag-out; other-bank → move/copy; same-bank → reorder / Alt-over-occupied → replace. - `card_meta` — pure card-metadata formatters: bars.beats.subdivisions and seconds.milliseconds; blank when the sample is unstamped. -- `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc`. An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle and gives an opaque core for any width above 1 px. Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). +- `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc`. An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle. **The guaranteed-opaque-core threshold is width ≥ 2 px, not any width above 1 px**: opacity needs `distance <= halfWidth - 0.5`, and the worst-case distance from a pixel centre to the centreline is 0.5, so a 1 px stroke (`halfWidth = 0.5`) has zero slack — its peak alpha modulates with the stroke's exact alignment to the pixel grid instead of pinning to 255 (the knob track arc and the mini curve-thumbnail trace are both 1 px and both live with this). Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). ## Gotchas diff --git a/src/core/ui/stroke_aa.cpp b/src/core/ui/stroke_aa.cpp index 2d65506..f8fc8f0 100644 --- a/src/core/ui/stroke_aa.cpp +++ b/src/core/ui/stroke_aa.cpp @@ -56,16 +56,26 @@ void StrokeCanvas::extendRow(int y, int x0, int x1) { } void StrokeCanvas::addPiece(float ax, float ay, float bx, float by, float halfWidth) { + // Every current caller feeds bounded geometry, but this is a pure module: a NaN/Inf + // coordinate would otherwise reach static_cast below, which is UB rather than a + // clipped no-op. + if (!(std::isfinite(ax) && std::isfinite(ay) && std::isfinite(bx) && std::isfinite(by) && + std::isfinite(halfWidth))) { + return; + } const float reach = halfWidth + 0.5f; // beyond this the coverage is 0 const float dx = bx - ax; const float dy = by - ay; const float len2 = dx * dx + dy * dy; const float invLen2 = len2 > 0.0f ? 1.0f / len2 : 0.0f; - int x0 = static_cast(std::floor((std::min)(ax, bx) - reach)); - int x1 = static_cast(std::ceil((std::max)(ax, bx) + reach)) + 1; - int y0 = static_cast(std::floor((std::min)(ay, by) - reach)); - int y1 = static_cast(std::ceil((std::max)(ay, by) + reach)) + 1; + // A pixel can only take ink when its centre (x+0.5) is within `reach` of the piece, i.e. + // x + 0.5 < maxX + reach — so the exclusive upper bound is floor(maxX + reach + 0.5), not + // ceil(maxX + reach) + 1 (a whole extra pixel of guaranteed-zero coverage on every side). + int x0 = static_cast(std::ceil((std::min)(ax, bx) - reach - 0.5f)); + int x1 = static_cast(std::floor((std::max)(ax, bx) + reach + 0.5f)); + int y0 = static_cast(std::ceil((std::min)(ay, by) - reach - 0.5f)); + int y1 = static_cast(std::floor((std::max)(ay, by) + reach + 0.5f)); x0 = (std::max)(x0, bounds_.x); y0 = (std::max)(y0, bounds_.y); x1 = (std::min)(x1, bounds_.right()); @@ -123,11 +133,20 @@ Rect strokeBounds(const StrokePoint* pts, std::size_t count, float halfWidth, co minY = (std::min)(minY, pts[i].y); maxY = (std::max)(maxY, pts[i].y); } + // Same NaN/Inf guard as addPiece: an unbounded coordinate must clip to nothing, not reach + // the static_cast below as UB. + if (!(std::isfinite(minX) && std::isfinite(maxX) && std::isfinite(minY) && + std::isfinite(maxY) && std::isfinite(halfWidth))) { + return Rect{}; + } const float reach = halfWidth + 0.5f; - const int x0 = (std::max)(clip.x, static_cast(std::floor(minX - reach))); - const int y0 = (std::max)(clip.y, static_cast(std::floor(minY - reach))); - const int x1 = (std::min)(clip.right(), static_cast(std::ceil(maxX + reach)) + 1); - const int y1 = (std::min)(clip.bottom(), static_cast(std::ceil(maxY + reach)) + 1); + // Same tightened box as addPiece (see its comment): a pixel only takes ink when its centre + // is within `reach`, so this is [ceil(min-reach-0.5), floor(max+reach+0.5)) rather than the + // old ceil/floor pair that padded a whole extra pixel on every side. + const int x0 = (std::max)(clip.x, static_cast(std::ceil(minX - reach - 0.5f))); + const int y0 = (std::max)(clip.y, static_cast(std::ceil(minY - reach - 0.5f))); + const int x1 = (std::min)(clip.right(), static_cast(std::floor(maxX + reach + 0.5f))); + const int y1 = (std::min)(clip.bottom(), static_cast(std::floor(maxY + reach + 0.5f))); if (x0 >= x1 || y0 >= y1) return Rect{}; return Rect::ltrb(x0, y0, x1, y1); } diff --git a/src/core/ui/stroke_aa.h b/src/core/ui/stroke_aa.h index a9d1b6d..d93b09e 100644 --- a/src/core/ui/stroke_aa.h +++ b/src/core/ui/stroke_aa.h @@ -84,4 +84,14 @@ void strokePolyline(StrokeCanvas& canvas, const StrokePoint* pts, std::size_t co void appendArc(std::vector& out, float cx, float cy, float radius, float startRad, float endRad, float flatnessPx = kArcFlatnessPx); +// The row-major element offset of row `y` within a `rowSpan`-elements-per-row pixel buffer, +// accounting for a possibly bottom-up (`flipped`) layout. Pulled out of the shell's LICE blend +// so its flipped branch — dead for every bitmap type the shell actually constructs, and +// otherwise unverifiable without a live LICE surface — is pinned by a host-free test. Matches +// LICE's own row math (`lice.cpp`'s `LICE_SysBitmap` pixel accessor: `(h-1-y)*rowspan + x`). +inline std::size_t rasterRowOffset(int y, int height, int rowSpan, bool flipped) { + const int row = flipped ? height - 1 - y : y; + return static_cast(row) * static_cast(rowSpan); +} + } // namespace reasampler::ui diff --git a/src/shell/instrument/editor_internal.h b/src/shell/instrument/editor_internal.h index 561b14a..da82131 100644 --- a/src/shell/instrument/editor_internal.h +++ b/src/shell/instrument/editor_internal.h @@ -117,7 +117,7 @@ inline void drawTitleBand(LICE_IBitmap* bmp, const instrument::ui::Rect& title, inline constexpr float kKnobTrackArcPx = 1.0f; inline constexpr float kKnobValueArcPx = 3.0f; inline constexpr float kInnerDialArcPx = 2.0f; -inline constexpr int kKnobNeedlePx = 2; +inline constexpr float kKnobNeedlePx = 2.0f; // Draws one radial knob face: param_slider owns the value<->angle map; this turns it into // LICE calls. LICE takes radians, and drawing the 7->5 o'clock sweep through the top needs @@ -162,8 +162,8 @@ inline void drawKnobFace(LICE_IBitmap* bmp, const instrument::ui::Rect& knobRect const double iy = kg.centerY + (tip.y - kg.centerY) * 0.35; const ui::Role needleRole = disabled ? ui::Role::TextDim : ui::Role::TextPrimary; strokeLineAA(bmp, static_cast(ix), static_cast(iy), - static_cast(tip.x), static_cast(tip.y), - static_cast(kKnobNeedlePx), toLice(ui::roleColor(needleRole))); + static_cast(tip.x), static_cast(tip.y), kKnobNeedlePx, + toLice(ui::roleColor(needleRole))); } // The concentric INNER dial: a second value on the same cell, drawn in the categorical diff --git a/src/shell/instrument/editor_paint_curve.cpp b/src/shell/instrument/editor_paint_curve.cpp index 8f01b4b..b1ac9ab 100644 --- a/src/shell/instrument/editor_paint_curve.cpp +++ b/src/shell/instrument/editor_paint_curve.cpp @@ -63,7 +63,7 @@ void ReaSamplerEditor::paintCurveButton(LICE_IBitmap* bmp, const Rect& r, CurveT } const LICE_pixel trace = toLice(roleColor(disabled ? Role::LineHairline : Role::AccentSecondary)); - static thread_local std::vector pts; + std::vector& pts = scratchPoints(); pts.clear(); for (int px = 0; px <= mini.width; ++px) { const int mx = mini.left + px; @@ -123,7 +123,7 @@ void ReaSamplerEditor::paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r) { // handles, and the hit-test all share one coordinate system (sub-pixel for the trace — see // subpixelFromPoint). const LICE_pixel line = toLice(roleColor(Role::AccentSecondary)); - static thread_local std::vector pts; + std::vector& pts = scratchPoints(); pts.clear(); for (int px = 0; px <= box.width; ++px) { const int cx = box.left + px; diff --git a/src/shell/instrument/editor_paint_waveform.cpp b/src/shell/instrument/editor_paint_waveform.cpp index 1338f6e..17d5d5c 100644 --- a/src/shell/instrument/editor_paint_waveform.cpp +++ b/src/shell/instrument/editor_paint_waveform.cpp @@ -145,11 +145,13 @@ void ReaSamplerEditor::paintSplineOverlay(LICE_IBitmap* bmp, const OverlayArea& // share the coordinate system the hit-test resolves against (sub-pixel here — see // subpixelFromPoint). const LICE_pixel line = toLice(roleColor(Role::OverlayTrace)); - static thread_local std::vector trace; + std::vector& trace = scratchPoints(); trace.clear(); // < not <=: box.left + box.width is the overlay's own EXCLUSIVE right edge (the box has no - // inset, unlike the popup's), so a <= column paints one pixel into the next band's pad — - // and it is redundant with the clamped endpoint handle below anyway. + // inset, unlike the popup's), so a <= column would re-trace a duplicate vertex one pixel + // past it. This bound does NOT contain the stroke to the box either way — the round cap on + // the last vertex extends halfWidth + 0.5 px past it regardless, same as the top/bottom + // edges the loop never clips against. for (int px = 0; px < box.width; ++px) { const int cx = box.left + px; const double t = curve.pointFromPixel(box, cx, box.top).velocity; @@ -214,13 +216,16 @@ void ReaSamplerEditor::paintEnvelopeOverlay(LICE_IBitmap* bmp, const OverlayArea // — unlike the spline traces above — because they are the same positions the draggable // handles are drawn at, and a sub-pixel trace would sit off its own handles. const LICE_pixel line = toLice(roleColor(Role::OverlayTrace)); - static thread_local std::vector trace; + std::vector& trace = scratchPoints(); trace.clear(); for (const EnvVertex& v : poly) { if (v.knot) continue; const int vx = (std::max)(area.x, (std::min)(area.right() - 1, v.x)); trace.push_back(ui::StrokePoint{static_cast(vx), static_cast(v.y)}); } + // A degenerate envelope (every stage collapsed to zero span) can reduce this to ONE vertex. + // strokePolylineAA's round-cap zero-length case then draws a dot at it, marking the sole + // point rather than drawing nothing — kept deliberately as more legible than a blank trace. strokePolylineAA(bmp, trace, kEnvTracePx, line); // Handles: a square per draggable stage node, a ROUND knot per curvable segment. Every // vertex is guaranteed in-bounds; the handle is additionally clamped inside the band so one diff --git a/src/shell/instrument/editor_stroke.cpp b/src/shell/instrument/editor_stroke.cpp index 7a50ce1..e2fe007 100644 --- a/src/shell/instrument/editor_stroke.cpp +++ b/src/shell/instrument/editor_stroke.cpp @@ -13,20 +13,54 @@ namespace { // the mask and the point list are resized, never reallocated. thread_local ui::StrokeCanvas g_canvas; thread_local std::vector g_arcPoints; +thread_local std::vector g_scratchPoints; + +// Per-pixel fallback through LICE_PutPixel, which itself reads LICE_EXT_GET_SCALING and scales +// its target coordinate (lice.cpp's LICE_PutPixel) — unlike the raw-bits path below, this is +// correct under a scaled bitmap. Only taken when scaling is active (see blendCanvas), so it +// costs nothing at the common unscaled call site. +void blendScaledFallback(LICE_IBitmap* bmp, const ui::StrokeCanvas& canvas, LICE_pixel color, + float alpha) { + const ui::Rect& b = canvas.bounds(); + for (int y = b.y; y < b.bottom(); ++y) { + const int lo = canvas.rowLo(y); + const int hi = canvas.rowHi(y); + if (hi <= lo) continue; + const float* const cov = canvas.rowData(y); + for (int i = lo; i < hi; ++i) { + const float a = cov[i] * alpha; + if (a <= 0.0f) continue; + LICE_PutPixel(bmp, b.x + i, y, color, a, LICE_BLIT_MODE_COPY); + } + } +} // One blend of the finished mask. The arithmetic matches LICE's own mode-0 combine // (src + (dst-src)*(256-a)/256 on all four channels, alpha in .8 fixed point) so a stroke // composites identically to every other kit draw on the same surface — written straight to the // bitmap's bits rather than through LICE_PutPixel, which re-derives the row pointer per pixel. +// +// That raw write assumes getWidth()/getHeight() (LOGICAL) and getRowSpan() (the DIB's PHYSICAL +// stride) agree — true only when unscaled. LICE_Arc/LICE_Line read LICE_EXT_GET_SCALING and +// scale their coordinates before touching the DIB (lice_arc.cpp:543, lice_line.cpp:1932); +// LICE_SysBitmap::__resize keeps m_width logical while sizing the DIB by +// (w*m_draw_scaling)>>8 (lice.cpp:165-173). Under a scale this loop's geometry and its target +// stride would disagree — a scale >256 lands the stroke in the wrong quadrant, a scale <256 +// runs the write past the DIB allocation. Nothing calls SET_SCALING today, but the guard has to +// stay ahead of the day something does. void blendCanvas(LICE_IBitmap* bmp, const ui::StrokeCanvas& canvas, LICE_pixel color, float alpha) { const ui::Rect& b = canvas.bounds(); if (b.empty() || alpha <= 0.0f) return; + if (bmp->Extended(LICE_EXT_GET_SCALING, nullptr) != 0) { + blendScaledFallback(bmp, canvas, color, alpha); + return; + } LICE_pixel* const bits = bmp->getBits(); const int span = bmp->getRowSpan(); if (bits == nullptr || span <= 0) return; const bool flipped = bmp->isFlipped(); - const int lastRow = bmp->getHeight() - 1; + const int height = bmp->getHeight(); const int sr = LICE_GETR(color); const int sg = LICE_GETG(color); @@ -38,8 +72,7 @@ void blendCanvas(LICE_IBitmap* bmp, const ui::StrokeCanvas& canvas, LICE_pixel c if (hi <= lo) continue; const float* const cov = canvas.rowData(y); LICE_pixel* const row = - bits + static_cast(flipped ? lastRow - y : y) * - static_cast(span) + static_cast(b.x); + bits + ui::rasterRowOffset(y, height, span, flipped) + static_cast(b.x); for (int i = lo; i < hi; ++i) { const int ia = static_cast(cov[i] * alpha * 256.0f); if (ia <= 0) continue; @@ -78,6 +111,8 @@ void strokeArcAA(LICE_IBitmap* bmp, float cx, float cy, float radius, float star strokePolylineAA(bmp, g_arcPoints.data(), g_arcPoints.size(), widthPx, color, alpha); } +std::vector& scratchPoints() { return g_scratchPoints; } + } // namespace reasampler::vst #endif // _WIN32 diff --git a/src/shell/instrument/editor_stroke.h b/src/shell/instrument/editor_stroke.h index 338dfaa..6b4da9c 100644 --- a/src/shell/instrument/editor_stroke.h +++ b/src/shell/instrument/editor_stroke.h @@ -45,6 +45,15 @@ inline void strokeLineAA(LICE_IBitmap* bmp, float x0, float y0, float x1, float void strokeArcAA(LICE_IBitmap* bmp, float cx, float cy, float radius, float startRad, float endRad, float widthPx, LICE_pixel color, float alpha = 1.0f); +// The draw-thread-only point-list scratch, shared by every paint site that builds a polyline +// column-by-column before one strokePolylineAA call (the curve thumbnail, the curve-popup +// trace, the two waveform-overlay traces). Reuse only: `clear()` and refill before each use, +// never read across paint calls. This module already owns the draw-thread scratch (the mask + +// the arc point list this header's own functions use internally); routing every external +// point-list consumer through the same accessor keeps that ownership one fact in one place +// instead of four independent function-local statics. +std::vector& scratchPoints(); + } // namespace reasampler::vst #endif // _WIN32 diff --git a/tests/test_stroke_aa.cpp b/tests/test_stroke_aa.cpp index a4d7609..42d4d23 100644 --- a/tests/test_stroke_aa.cpp +++ b/tests/test_stroke_aa.cpp @@ -73,6 +73,8 @@ static float perpendicularWeight(float angleDeg, float halfWidth) { static void testStraightStrokeHasAnOpaqueCore() { // The shipped defect stated numerically: peak alpha must reach full, not 137-192/255. + // 2 and 3 px only: below the >= 2 px opaque-core threshold (core/ui/CLAUDE.md), a stroke + // does NOT reliably reach full alpha — see testSubOpaqueCoreAtOnePixelWidth below. for (float w : {2.0f, 3.0f}) { for (float deg : {0.0f, 17.0f, 45.0f, 63.0f, 90.0f}) { const float a = deg * kPi / 180.0f; @@ -85,6 +87,25 @@ static void testStraightStrokeHasAnOpaqueCore() { } } +static void testSubOpaqueCoreAtOnePixelWidth() { + // Below the >= 2 px opaque-core threshold: a 1 px stroke (halfWidth = 0.5) has zero slack + // against the 0.5 px worst-case pixel-centre distance (core/ui/CLAUDE.md), so peak alpha + // tracks the stroke's alignment to the pixel grid instead of reaching 255 everywhere. Pin + // both ends of that modulation — this is the knob track arc's and the mini curve-trace's + // actual behaviour, not a hypothetical. + const StrokePoint onRowCentre[2] = {{20.0f, 100.5f}, {220.0f, 100.5f}}; // centred on row 100 + StrokeCanvas aligned; + strokePolyline(aligned, onRowCentre, 2, 0.5f, kBig); + CHECK(aligned.coverageAt(120, 100) >= 0.999f); // aligned to the grid: reaches opaque + + const StrokePoint onRowBoundary[2] = {{20.0f, 100.0f}, {220.0f, 100.0f}}; // on the boundary + StrokeCanvas misaligned; + strokePolyline(misaligned, onRowBoundary, 2, 0.5f, kBig); + CHECK(std::fabs(misaligned.coverageAt(120, 99) - 0.5f) < 1e-4f); // split evenly... + CHECK(std::fabs(misaligned.coverageAt(120, 100) - 0.5f) < 1e-4f); // ...across both rows + CHECK(peakCoverage(misaligned) < 0.999f); // and never reaches the opaque core here +} + static void testPerpendicularWeightIsAngleIndependent() { // The criterion that killed the ThickFLine option: it dips to wid*cos(theta) at every 45 // degrees. An axis-aligned-only sample would pass against it, so sample the diagonals. @@ -214,12 +235,23 @@ static void testRevisitedRowGapReadsZeroNotGarbage() { // A circle touches most rows on BOTH sides, leaving an untouched gap between the two spans. // The row's valid extent grows over that gap, so the gap must be zero-filled, not left at // whatever the reused scratch buffer held. - StrokeCanvas c; - c.reset(kBig); - c.addSegment(30.0f, 100.0f, 30.0f, 140.0f, 1.5f); // dirty the buffer on the left - c.reset(kBig); + // + // The dirtying pass must land at the SAME bounds/stride the arc pass will reuse, or the two + // writes address disjoint buffer offsets and the "old" value the gap reads back is just the + // scratch buffer's original zero-init — the guard would then have nothing to prove itself + // against (verified: deleting extendRow's fill at stroke_aa.cpp:48-51 left this test passing + // when the dirtying pass used `kBig` while the arc pass reset to its own tighter bounds). std::vector ring; appendArc(ring, 120.0f, 120.0f, 60.0f, 0.0f, 2.0f * kPi); + const Rect arcBounds = strokeBounds(ring.data(), ring.size(), 1.5f, kBig); + + StrokeCanvas c; + c.reset(arcBounds); + // A horizontal segment straight across the row/columns the assertion below checks, so the + // buffer genuinely holds nonzero ink there before the arc's own pass reuses the canvas. + c.addSegment(100.0f, 120.0f, 140.0f, 120.0f, 1.5f); + CHECK(c.coverageAt(120, 120) > 0.9f); // sanity: the dirtying pass actually landed here + strokePolyline(c, ring.data(), ring.size(), 1.5f, kBig); for (int x = 100; x < 140; ++x) CHECK(c.coverageAt(x, 120) == 0.0f); // hollow middle } @@ -257,6 +289,20 @@ static void testCoverageOutsideTheValidSpanReadsZero() { CHECK(c.coverageAt(1000, 1000) == 0.0f); } +// --- raster row addressing ---------------------------------------------------- + +static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() { + // Unflipped: row y is just y*rowSpan (LICE's top-down layout). + CHECK(rasterRowOffset(0, 100, 240, false) == 0u); + CHECK(rasterRowOffset(5, 100, 240, false) == 5u * 240u); + CHECK(rasterRowOffset(99, 100, 240, false) == 99u * 240u); + // Flipped (bottom-up DIBs): row y is (height-1-y)*rowSpan — LICE_SysBitmap's own pixel + // accessor, `(h-1-y)*rowspan + x` (lice.cpp:2262). + CHECK(rasterRowOffset(0, 100, 240, true) == 99u * 240u); + CHECK(rasterRowOffset(99, 100, 240, true) == 0u); + CHECK(rasterRowOffset(40, 100, 240, true) == 59u * 240u); +} + // --- long-segment subdivision ------------------------------------------------ static void testSubdivisionDoesNotChangeTheRenderedStroke() { @@ -381,6 +427,7 @@ static void testKnobArcIsOpaqueAndEvenAllTheWayRound() { int main() { testStraightStrokeHasAnOpaqueCore(); + testSubOpaqueCoreAtOnePixelWidth(); testPerpendicularWeightIsAngleIndependent(); testWeightHoldsAtTheExactDiagonal(); testZeroLengthSegmentIsARoundDot(); @@ -394,6 +441,7 @@ int main() { testBoundsClipToTheClipRectAndCoverTheReach(); testStrokeEntirelyOutsideTheClipDrawsNothing(); testCoverageOutsideTheValidSpanReadsZero(); + testRasterRowOffsetMatchesUnflippedAndFlippedLayouts(); testSubdivisionDoesNotChangeTheRenderedStroke(); testArcPointsLieOnTheCircleAndRespectTheFlatness(); testArcDensityGrowsWithRadius(); diff --git a/tests/test_velocity_curve.cpp b/tests/test_velocity_curve.cpp index fdd5a2c..558e29b 100644 --- a/tests/test_velocity_curve.cpp +++ b/tests/test_velocity_curve.cpp @@ -16,6 +16,7 @@ #include "../src/core/instrument/engine/velocity_curve.h" +#include #include #include #include @@ -398,12 +399,35 @@ static void testSubpixelMapIsTheIntegerMapBeforeRounding() { const Box boxes[] = {{10, 20, 100, 51}, {0, 0, 127, 101}, {7, 3, 33, 17}}; for (const Box& box : boxes) { for (const VelocityCurve& c : {uni(), bip()}) { + const double lo = curveYMin(c.domain()); for (double v = 0.0; v <= 127.0; v += 1.0) { for (double a = -1.0; a <= 1.0; a += 0.125) { const auto ip = c.pixelFromPoint(box, {v, a}); const auto fp = c.subpixelFromPoint(box, {v, a}); + // Relationship guard: int IS round(subpixel), everywhere — a forward guard + // against the trace drifting off its own handles. This alone is a tautology + // against the CURRENT formula (int literally computes this expression), so + // it cannot catch a regression against the PRE-subpixel formula below. CHECK(ip.x == box.left + static_cast(fp.x - box.left + 0.5)); CHECK(ip.y == box.top + static_cast(fp.y - box.top + 0.5)); + + // Independent re-derivation of the formula the int map claims to reproduce — + // box.left + (int)(frac*w + 0.5) — computed here from scratch rather than + // through subpixelFromPoint's box.left-add-then-subtract round trip, so a + // flip introduced by that round trip (however unlikely per Sterbenz) has + // something to fail against. + const double vClamped = (std::min)(kVelMax, (std::max)(kVelMin, v)); + const double fracX = (vClamped - kVelMin) / (kVelMax - kVelMin); + const int oldX = + box.left + static_cast(fracX * static_cast(box.width) + 0.5); + CHECK(ip.x == oldX); + + const double aClamped = (std::min)(kCurveYMax, (std::max)(lo, a)); + const double fracY = (aClamped - lo) / (kCurveYMax - lo); + const int oldY = + box.top + static_cast((1.0 - fracY) * + static_cast(box.height - 1) + 0.5); + CHECK(ip.y == oldY); } } } From 07628a205960bd699360c9febdc44304a3066d67 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 13:59:09 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20close=20=CE=98-W7-T1=20round-2=20lef?= =?UTF-8?q?tovers=20=E2=80=94=20NaN=20guard=20placement,=20comment=20attri?= =?UTF-8?q?bution,=20CLAUDE.md=20export?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the NaN/Inf finiteness check to addSegment where the UB-causing cast actually happens, makes strokeBounds reject an interior non-finite point instead of swallowing it, corrects a LICE_GetPixel misattribution, and documents rasterRowOffset. --- src/core/ui/CLAUDE.md | 4 +++- src/core/ui/stroke_aa.cpp | 32 ++++++++++++++++++-------------- src/core/ui/stroke_aa.h | 2 +- tests/test_stroke_aa.cpp | 31 +++++++++++++++++++++++++++++-- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index 6e110d7..f6209db 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -101,7 +101,9 @@ L7 sub-pass, 2026-07-27): - `tooltip` — pure tooltip placement + prefix-strip: strips the `ReaSampler:` display prefix from the registered action phrase; width clamped to the client rect. - `card_drag` — pure drag-gesture precedence + slot hit-test: leave-client → OS drag-out; other-bank → move/copy; same-bank → reorder / Alt-over-occupied → replace. - `card_meta` — pure card-metadata formatters: bars.beats.subdivisions and seconds.milliseconds; blank when the sample is unstamped. -- `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc`. An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle. **The guaranteed-opaque-core threshold is width ≥ 2 px, not any width above 1 px**: opacity needs `distance <= halfWidth - 0.5`, and the worst-case distance from a pixel centre to the centreline is 0.5, so a 1 px stroke (`halfWidth = 0.5`) has zero slack — its peak alpha modulates with the stroke's exact alignment to the pixel grid instead of pinning to 255 (the knob track arc and the mini curve-thumbnail trace are both 1 px and both live with this). Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). +- `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc` / `rasterRowOffset` (the row-major offset + math for a possibly bottom-up raster, pulled out of the shell's LICE blend so its flipped + branch is pinned by a host-free test). An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle. **The guaranteed-opaque-core threshold is width ≥ 2 px, not any width above 1 px**: opacity needs `distance <= halfWidth - 0.5`, and the worst-case distance from a pixel centre to the centreline is 0.5, so a 1 px stroke (`halfWidth = 0.5`) has zero slack — its peak alpha modulates with the stroke's exact alignment to the pixel grid instead of pinning to 255 (the knob track arc and the mini curve-thumbnail trace are both 1 px and both live with this). Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). ## Gotchas diff --git a/src/core/ui/stroke_aa.cpp b/src/core/ui/stroke_aa.cpp index f8fc8f0..e9f372d 100644 --- a/src/core/ui/stroke_aa.cpp +++ b/src/core/ui/stroke_aa.cpp @@ -56,13 +56,7 @@ void StrokeCanvas::extendRow(int y, int x0, int x1) { } void StrokeCanvas::addPiece(float ax, float ay, float bx, float by, float halfWidth) { - // Every current caller feeds bounded geometry, but this is a pure module: a NaN/Inf - // coordinate would otherwise reach static_cast below, which is UB rather than a - // clipped no-op. - if (!(std::isfinite(ax) && std::isfinite(ay) && std::isfinite(bx) && std::isfinite(by) && - std::isfinite(halfWidth))) { - return; - } + // addSegment (the only caller) guarantees finite inputs before this point. const float reach = halfWidth + 0.5f; // beyond this the coverage is 0 const float dx = bx - ax; const float dy = by - ay; @@ -104,6 +98,14 @@ void StrokeCanvas::addPiece(float ax, float ay, float bx, float by, float halfWi void StrokeCanvas::addSegment(float ax, float ay, float bx, float by, float halfWidth) { if (bounds_.empty() || halfWidth <= 0.0f) return; + // Must guard here, not in addPiece: len2 below goes NaN/Inf on a bad input too, so the + // `len2 <= kMaxPieceLen^2` comparison is false either way (NaN compares false against + // anything) and the pieces-count cast a few lines down is reached as UB regardless of which + // branch is taken. + if (!(std::isfinite(ax) && std::isfinite(ay) && std::isfinite(bx) && std::isfinite(by) && + std::isfinite(halfWidth))) { + return; + } const float dx = bx - ax; const float dy = by - ay; const float len2 = dx * dx + dy * dy; @@ -125,20 +127,22 @@ void StrokeCanvas::addSegment(float ax, float ay, float bx, float by, float half } Rect strokeBounds(const StrokePoint* pts, std::size_t count, float halfWidth, const Rect& clip) { - if (pts == nullptr || count == 0 || halfWidth <= 0.0f || clip.empty()) return Rect{}; + if (pts == nullptr || count == 0 || halfWidth <= 0.0f || clip.empty() || + !std::isfinite(halfWidth)) { + return Rect{}; + } + if (!(std::isfinite(pts[0].x) && std::isfinite(pts[0].y))) return Rect{}; float minX = pts[0].x, maxX = pts[0].x, minY = pts[0].y, maxY = pts[0].y; for (std::size_t i = 1; i < count; ++i) { + // Checked per-point, not via isfinite(minX/maxX) after the reduction: std::min/max + // against NaN silently returns the OTHER (finite) operand, so a NaN anywhere but pts[0] + // would otherwise vanish from the reduction instead of rejecting the stroke. + if (!(std::isfinite(pts[i].x) && std::isfinite(pts[i].y))) return Rect{}; minX = (std::min)(minX, pts[i].x); maxX = (std::max)(maxX, pts[i].x); minY = (std::min)(minY, pts[i].y); maxY = (std::max)(maxY, pts[i].y); } - // Same NaN/Inf guard as addPiece: an unbounded coordinate must clip to nothing, not reach - // the static_cast below as UB. - if (!(std::isfinite(minX) && std::isfinite(maxX) && std::isfinite(minY) && - std::isfinite(maxY) && std::isfinite(halfWidth))) { - return Rect{}; - } const float reach = halfWidth + 0.5f; // Same tightened box as addPiece (see its comment): a pixel only takes ink when its centre // is within `reach`, so this is [ceil(min-reach-0.5), floor(max+reach+0.5)) rather than the diff --git a/src/core/ui/stroke_aa.h b/src/core/ui/stroke_aa.h index d93b09e..d716427 100644 --- a/src/core/ui/stroke_aa.h +++ b/src/core/ui/stroke_aa.h @@ -88,7 +88,7 @@ void appendArc(std::vector& out, float cx, float cy, float radius, // accounting for a possibly bottom-up (`flipped`) layout. Pulled out of the shell's LICE blend // so its flipped branch — dead for every bitmap type the shell actually constructs, and // otherwise unverifiable without a live LICE surface — is pinned by a host-free test. Matches -// LICE's own row math (`lice.cpp`'s `LICE_SysBitmap` pixel accessor: `(h-1-y)*rowspan + x`). +// LICE's own row math (`lice.cpp`'s free-function `LICE_GetPixel`: `(h-1-y)*rowspan + x`). inline std::size_t rasterRowOffset(int y, int height, int rowSpan, bool flipped) { const int row = flipped ? height - 1 - y : y; return static_cast(row) * static_cast(rowSpan); diff --git a/tests/test_stroke_aa.cpp b/tests/test_stroke_aa.cpp index 42d4d23..1594fce 100644 --- a/tests/test_stroke_aa.cpp +++ b/tests/test_stroke_aa.cpp @@ -289,6 +289,31 @@ static void testCoverageOutsideTheValidSpanReadsZero() { CHECK(c.coverageAt(1000, 1000) == 0.0f); } +// --- non-finite input rejection ------------------------------------------------ + +static void testInteriorNonFiniteCoordinateRejectsTheWholeStroke() { + // Only pts[0] going non-finite used to propagate through strokeBounds's min/max reduction: + // std::min/max against NaN silently returns the OTHER, finite, operand, so a NaN anywhere + // else in the polyline vanished from the reduction instead of rejecting it. pts[1] here is + // the case that check missed. + const StrokePoint pts[3] = {{10.0f, 10.0f}, {NAN, 50.0f}, {90.0f, 10.0f}}; + CHECK(strokeBounds(pts, 3, 1.5f, kBig).empty()); + + StrokeCanvas c; + strokePolyline(c, pts, 3, 1.5f, kBig); + CHECK(c.bounds().empty()); +} + +static void testAddSegmentRejectsNonFiniteInputsDirectly() { + // addSegment casts a pieces-count derived from len2 to int; a NaN/Inf endpoint must be + // caught before that cast, not one call layer down in addPiece where it's already too late. + StrokeCanvas c; + c.reset(kBig); + c.addSegment(10.0f, 10.0f, NAN, 50.0f, 1.5f); + c.addSegment(10.0f, 10.0f, INFINITY, 50.0f, 1.5f); + CHECK(peakCoverage(c) == 0.0f); +} + // --- raster row addressing ---------------------------------------------------- static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() { @@ -296,8 +321,8 @@ static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() { CHECK(rasterRowOffset(0, 100, 240, false) == 0u); CHECK(rasterRowOffset(5, 100, 240, false) == 5u * 240u); CHECK(rasterRowOffset(99, 100, 240, false) == 99u * 240u); - // Flipped (bottom-up DIBs): row y is (height-1-y)*rowSpan — LICE_SysBitmap's own pixel - // accessor, `(h-1-y)*rowspan + x` (lice.cpp:2262). + // Flipped (bottom-up DIBs): row y is (height-1-y)*rowSpan — matches the free function + // LICE_GetPixel's own row math, `(h-1-y)*rowspan + x` (lice.cpp:2262). CHECK(rasterRowOffset(0, 100, 240, true) == 99u * 240u); CHECK(rasterRowOffset(99, 100, 240, true) == 0u); CHECK(rasterRowOffset(40, 100, 240, true) == 59u * 240u); @@ -441,6 +466,8 @@ int main() { testBoundsClipToTheClipRectAndCoverTheReach(); testStrokeEntirelyOutsideTheClipDrawsNothing(); testCoverageOutsideTheValidSpanReadsZero(); + testInteriorNonFiniteCoordinateRejectsTheWholeStroke(); + testAddSegmentRejectsNonFiniteInputsDirectly(); testRasterRowOffsetMatchesUnflippedAndFlippedLayouts(); testSubdivisionDoesNotChangeTheRenderedStroke(); testArcPointsLieOnTheCircleAndRespectTheFlatness(); From 704b7ef373f6f2fafcaa024507a9308046a1f468 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 15:11:58 -0400 Subject: [PATCH 4/4] fix: enlarge every sub-2px stroker width to 2px, per Daniel's ruling Knob track arc, inner-dial needle, and mini velocity-trace all sat below the analytic stroker's opaque-core floor. Raised to 2px; updated visual-design-language.md and core/ui/CLAUDE.md to match; kept the 1px-behavior test, corrected its stale comment. --- docs/product/visual-design-language.md | 6 +++--- src/core/ui/CLAUDE.md | 2 +- src/shell/instrument/editor_internal.h | 9 ++++++--- src/shell/instrument/editor_paint_curve.cpp | 5 +++-- tests/test_stroke_aa.cpp | 5 +++-- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index afa4691..20928e6 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -808,15 +808,15 @@ concession. Everything with a slope or a curve must draw through a primitive tha | Surface | Where | Disposition | |---|---|---| -| Radial knob track arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — the stacked-radius `LICE_Arc` ring never reached an opaque core. Now ONE analytic stroke (`strokeArcAA`) at `kKnobTrackArcPx` = 1 px, **below the ≥2 px opaque-core threshold** (`core/ui/CLAUDE.md`) — peak alpha still modulates with the arc's exact pixel-grid alignment (measured ~128–255/255 across the sweep) rather than pinning to 255. An improvement over the old stacked-radius ripple, not full opacity; see `test_stroke_aa.cpp`'s 1 px case for the pinned behaviour. | +| Radial knob track arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01), widened (2026-08-01)** — the stacked-radius `LICE_Arc` ring never reached an opaque core. Now ONE analytic stroke (`strokeArcAA`); `kKnobTrackArcPx` was initially left at 1 px, below the ≥2 px opaque-core threshold (`core/ui/CLAUDE.md`), and surfaced to Daniel as a by-eye call — he ruled to enlarge all sub-2 px stroker widths, so it is now 2 px and reaches a guaranteed opaque core. | | Radial knob value arc | `editor_internal.h` `drawKnobFace` | **Fixed (2026-08-01)** — same stroke, `kKnobValueArcPx` = 3 px, clear of the opaque-core threshold. Measured: peak **255/255** at every cross-section, weight **2.95–3.11 px** (5% ripple). | | Knob needle | `drawKnobFace` | **Fixed (2026-08-01)** — `LICE_ThickFLine`'s minor-axis width thinned it to `cos θ` as the knob swept. Now `strokeLineAA`, 2 px. | | Inner curve dial arc | `drawInnerDial` | **Fixed (2026-08-01)** — the arc shared the knob track/value arc's stacked-radius opacity defect. Same one analytic fix, at `kInnerDialArcPx` = 2 px (at the opaque-core threshold). | -| Inner curve dial needle | `drawInnerDial` | **Converted (2026-08-01), not a defect fix** — this needle was already `LICE_FLine` (float endpoints, always AA), not `LICE_ThickFLine`; a 1 px AA line has no width to lay along a minor axis, so it never had the knob needle's `cos θ` defect. Moved to `strokeLineAA` anyway for one-seam consistency with every other stroke on the editor, not because it was broken. | +| Inner curve dial needle | `drawInnerDial` | **Converted (2026-08-01), widened (2026-08-01)** — this needle was already `LICE_FLine` (float endpoints, always AA), not `LICE_ThickFLine`; a 1 px AA line has no width to lay along a minor axis, so it never had the knob needle's `cos θ` defect. Moved to `strokeLineAA` at 1 px for one-seam consistency, not because it was broken — but 1 px is below the analytic stroker's opaque-core threshold, so it fell under Daniel's later blanket ruling and is now `kInnerDialNeedlePx` = 2 px. | | Staged envelope segment slopes | `editor_paint_waveform.cpp` | **Fixed (2026-08-01)** — one `strokePolylineAA` over the whole polyline, so the stage joints blend once. Vertices stay INTEGER by design: they are the positions the draggable handles are drawn at. | | Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed (2026-08-01)** — the trace was never gapped; it was fully aliased (every pixel full or empty) because the loop passed INTEGER `cy`, quantizing the slope into alternating 1/2 px steps. Now sub-pixel y (`subpixelFromPoint`) through `strokePolylineAA`. Measured: peak **255/255**, weight **1.95–2.01 px** (3% ripple). | | Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed (2026-08-01)** — same cause, same treatment. | -| Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | **Fixed (2026-08-01)** — stays a 1 px hairline (a 2 px trace blots at thumbnail scale), but strokes analytically at sub-pixel y instead of integer-endpoint `LICE_Line`. | +| Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | **Fixed (2026-08-01), widened (2026-08-01)** — strokes analytically at sub-pixel y instead of integer-endpoint `LICE_Line`. Initially kept as a 1 px hairline (a 2 px trace was thought to blot at thumbnail scale), but 1 px is below the opaque-core threshold; Daniel's ruling raised `kMiniTracePx` to 2 px, same as the popup trace. | | Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. **Measured cost** (Release, MSVC 14.44, real LICE, 24 stereo cards × 136 columns = 6528 columns): fill alone 0.070 ms per full-grid repaint, fill+stroke 0.48 ms — the stroke is ~0.41 ms, about 2.5% of a 60 Hz frame, and the grid repaints on hover/scroll/drag, not continuously. One-off scratchpad measurement, 2026-08-01, harness not committed — not a standing regression guard; re-measure before relying on it again. | | Preview play triangle | `editor_paint_chrome.cpp` | **Fixed** — `LICE_FillTriangle` has no `aa`; its two sloped edges are re-stroked with AA `LICE_FLine`. | | Envelope/spline node handles (squares) | `editor_paint_waveform.cpp` | Already clean — axis-aligned `LICE_FillRect`. | diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index f6209db..e28b7f9 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -103,7 +103,7 @@ L7 sub-pass, 2026-07-27): - `card_meta` — pure card-metadata formatters: bars.beats.subdivisions and seconds.milliseconds; blank when the sample is unstamped. - `stroke_aa` — analytic antialiased thick-stroke COVERAGE (the shell blends it): `StrokeCanvas`, a reusable mask holding distance-to-polyline coverage MAX-accumulated across segments, plus `strokePolyline` / `strokeBounds` / `appendArc` / `rasterRowOffset` (the row-major offset math for a possibly bottom-up raster, pulled out of the shell's LICE blend so its flipped - branch is pinned by a host-free test). An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle. **The guaranteed-opaque-core threshold is width ≥ 2 px, not any width above 1 px**: opacity needs `distance <= halfWidth - 0.5`, and the worst-case distance from a pixel centre to the centreline is 0.5, so a 1 px stroke (`halfWidth = 0.5`) has zero slack — its peak alpha modulates with the stroke's exact alignment to the pixel grid instead of pinning to 255 (the knob track arc and the mini curve-thumbnail trace are both 1 px and both live with this). Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). + branch is pinned by a host-free test). An arc is just a flattened polyline, so ONE path serves the knob arcs, the inner dial, the envelope polyline and both spline traces. Coverage is `clamp(halfWidth + 0.5 - distance, 0, 1)`, which makes perpendicular weight exactly `2·halfWidth` at every angle. **The guaranteed-opaque-core threshold is width ≥ 2 px, not any width above 1 px**: opacity needs `distance <= halfWidth - 0.5`, and the worst-case distance from a pixel centre to the centreline is 0.5, so a 1 px stroke (`halfWidth = 0.5`) has zero slack — its peak alpha modulates with the stroke's exact alignment to the pixel grid instead of pinning to 255 (Daniel's ruling, 2026-08-01: every stroker-drawn width on the editor is now >= 2 px for this reason — `testSubOpaqueCoreAtOnePixelWidth` in `tests/test_stroke_aa.cpp` still pins the 1 px case as a property of the stroker, independent of whether any surface ships at that width). Long segments are subdivided before rasterizing — EXACT, not an approximation (min-distance to a partition of a segment is min-distance to the whole), purely to keep each piece's bounding box tight, since one long diagonal's box has area O(len²). ## Gotchas diff --git a/src/shell/instrument/editor_internal.h b/src/shell/instrument/editor_internal.h index da82131..814353c 100644 --- a/src/shell/instrument/editor_internal.h +++ b/src/shell/instrument/editor_internal.h @@ -113,11 +113,14 @@ inline void drawTitleBand(LICE_IBitmap* bmp, const instrument::ui::Rect& title, } // Stroke widths for the radial faces. Every arc is ONE analytic stroke (editor_stroke.h) whose -// outer edge sits on the knob's radius, so the centerline is inset by half the width. -inline constexpr float kKnobTrackArcPx = 1.0f; +// outer edge sits on the knob's radius, so the centerline is inset by half the width. Every +// width here is >= 2 px, the stroker's guaranteed-opaque-core floor (core/ui/CLAUDE.md) — +// anything narrower modulates peak alpha with pixel-grid alignment instead of pinning solid. +inline constexpr float kKnobTrackArcPx = 2.0f; inline constexpr float kKnobValueArcPx = 3.0f; inline constexpr float kInnerDialArcPx = 2.0f; inline constexpr float kKnobNeedlePx = 2.0f; +inline constexpr float kInnerDialNeedlePx = 2.0f; // Draws one radial knob face: param_slider owns the value<->angle map; this turns it into // LICE calls. LICE takes radians, and drawing the 7->5 o'clock sweep through the top needs @@ -198,7 +201,7 @@ inline void drawInnerDial(LICE_IBitmap* bmp, const instrument::ui::Rect& innerRe toLice(ui::roleColor(arcRole))); const KnobPoint tip = instrument::ui::knobNeedlePoint(kg, arc, v); strokeLineAA(bmp, static_cast(kg.centerX), static_cast(kg.centerY), - static_cast(tip.x), static_cast(tip.y), 1.0f, + static_cast(tip.x), static_cast(tip.y), kInnerDialNeedlePx, toLice(ui::roleColor(disabled ? ui::Role::TextDim : ui::Role::AccentTertiary))); } diff --git a/src/shell/instrument/editor_paint_curve.cpp b/src/shell/instrument/editor_paint_curve.cpp index b1ac9ab..376a3af 100644 --- a/src/shell/instrument/editor_paint_curve.cpp +++ b/src/shell/instrument/editor_paint_curve.cpp @@ -19,8 +19,9 @@ using namespace reasampler::instrument::ui; // popup geometry namespace { -// The deck thumbnail traces a hairline; the full editor matches the envelope traces' weight. -constexpr float kMiniTracePx = 1.0f; +// Both at the stroker's opaque-core floor (core/ui/CLAUDE.md) — below 2 px, peak alpha +// modulates with pixel-grid alignment instead of pinning solid. +constexpr float kMiniTracePx = 2.0f; constexpr float kCurveTracePx = 2.0f; const char* curveTitle(CurveTarget target) { diff --git a/tests/test_stroke_aa.cpp b/tests/test_stroke_aa.cpp index 1594fce..7c4d792 100644 --- a/tests/test_stroke_aa.cpp +++ b/tests/test_stroke_aa.cpp @@ -91,8 +91,9 @@ static void testSubOpaqueCoreAtOnePixelWidth() { // Below the >= 2 px opaque-core threshold: a 1 px stroke (halfWidth = 0.5) has zero slack // against the 0.5 px worst-case pixel-centre distance (core/ui/CLAUDE.md), so peak alpha // tracks the stroke's alignment to the pixel grid instead of reaching 255 everywhere. Pin - // both ends of that modulation — this is the knob track arc's and the mini curve-trace's - // actual behaviour, not a hypothetical. + // both ends of that modulation — no shipping surface draws at 1 px anymore (Daniel's + // ruling, 2026-08-01), but this stays a real property of the stroker worth guarding + // against reintroducing. const StrokePoint onRowCentre[2] = {{20.0f, 100.5f}, {220.0f, 100.5f}}; // centred on row 100 StrokeCanvas aligned; strokePolyline(aligned, onRowCentre, 2, 0.5f, kBig);