fix: close Θ-W7-T1 review — scaling guard, opacity claims, two vacuous test fixes

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.
This commit is contained in:
2026-08-01 13:45:43 -04:00
parent 2e09776342
commit 3fb77027c6
11 changed files with 181 additions and 30 deletions
+1 -1
View File
@@ -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
+27 -8
View File
@@ -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<int> 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<int>(std::floor((std::min)(ax, bx) - reach));
int x1 = static_cast<int>(std::ceil((std::max)(ax, bx) + reach)) + 1;
int y0 = static_cast<int>(std::floor((std::min)(ay, by) - reach));
int y1 = static_cast<int>(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<int>(std::ceil((std::min)(ax, bx) - reach - 0.5f));
int x1 = static_cast<int>(std::floor((std::max)(ax, bx) + reach + 0.5f));
int y0 = static_cast<int>(std::ceil((std::min)(ay, by) - reach - 0.5f));
int y1 = static_cast<int>(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<int> 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<int>(std::floor(minX - reach)));
const int y0 = (std::max)(clip.y, static_cast<int>(std::floor(minY - reach)));
const int x1 = (std::min)(clip.right(), static_cast<int>(std::ceil(maxX + reach)) + 1);
const int y1 = (std::min)(clip.bottom(), static_cast<int>(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<int>(std::ceil(minX - reach - 0.5f)));
const int y0 = (std::max)(clip.y, static_cast<int>(std::ceil(minY - reach - 0.5f)));
const int x1 = (std::min)(clip.right(), static_cast<int>(std::floor(maxX + reach + 0.5f)));
const int y1 = (std::min)(clip.bottom(), static_cast<int>(std::floor(maxY + reach + 0.5f)));
if (x0 >= x1 || y0 >= y1) return Rect{};
return Rect::ltrb(x0, y0, x1, y1);
}
+10
View File
@@ -84,4 +84,14 @@ void strokePolyline(StrokeCanvas& canvas, const StrokePoint* pts, std::size_t co
void appendArc(std::vector<StrokePoint>& 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<std::size_t>(row) * static_cast<std::size_t>(rowSpan);
}
} // namespace reasampler::ui
+3 -3
View File
@@ -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<float>(ix), static_cast<float>(iy),
static_cast<float>(tip.x), static_cast<float>(tip.y),
static_cast<float>(kKnobNeedlePx), toLice(ui::roleColor(needleRole)));
static_cast<float>(tip.x), static_cast<float>(tip.y), kKnobNeedlePx,
toLice(ui::roleColor(needleRole)));
}
// The concentric INNER dial: a second value on the same cell, drawn in the categorical
+2 -2
View File
@@ -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<ui::StrokePoint> pts;
std::vector<ui::StrokePoint>& 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<ui::StrokePoint> pts;
std::vector<ui::StrokePoint>& pts = scratchPoints();
pts.clear();
for (int px = 0; px <= box.width; ++px) {
const int cx = box.left + px;
@@ -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<ui::StrokePoint> trace;
std::vector<ui::StrokePoint>& 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<ui::StrokePoint> trace;
std::vector<ui::StrokePoint>& 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<float>(vx), static_cast<float>(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
+38 -3
View File
@@ -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<ui::StrokePoint> g_arcPoints;
thread_local std::vector<ui::StrokePoint> 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<std::size_t>(flipped ? lastRow - y : y) *
static_cast<std::size_t>(span) + static_cast<std::size_t>(b.x);
bits + ui::rasterRowOffset(y, height, span, flipped) + static_cast<std::size_t>(b.x);
for (int i = lo; i < hi; ++i) {
const int ia = static_cast<int>(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<ui::StrokePoint>& scratchPoints() { return g_scratchPoints; }
} // namespace reasampler::vst
#endif // _WIN32
+9
View File
@@ -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<ui::StrokePoint>& scratchPoints();
} // namespace reasampler::vst
#endif // _WIN32