fix: close Θ-W7-T1 round-2 leftovers — NaN guard placement, comment attribution, CLAUDE.md export

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.
This commit is contained in:
2026-08-01 13:59:09 -04:00
parent 3fb77027c6
commit 07628a2059
4 changed files with 51 additions and 18 deletions
+3 -1
View File
@@ -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. - `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_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. - `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 ## Gotchas
+18 -14
View File
@@ -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) { 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 // addSegment (the only caller) guarantees finite inputs before this point.
// 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 reach = halfWidth + 0.5f; // beyond this the coverage is 0
const float dx = bx - ax; const float dx = bx - ax;
const float dy = by - ay; 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) { void StrokeCanvas::addSegment(float ax, float ay, float bx, float by, float halfWidth) {
if (bounds_.empty() || halfWidth <= 0.0f) return; 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 dx = bx - ax;
const float dy = by - ay; const float dy = by - ay;
const float len2 = dx * dx + dy * dy; 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) { 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; 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) { 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); minX = (std::min)(minX, pts[i].x);
maxX = (std::max)(maxX, pts[i].x); maxX = (std::max)(maxX, pts[i].x);
minY = (std::min)(minY, pts[i].y); minY = (std::min)(minY, pts[i].y);
maxY = (std::max)(maxY, 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 float reach = halfWidth + 0.5f;
// Same tightened box as addPiece (see its comment): a pixel only takes ink when its centre // 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 // is within `reach`, so this is [ceil(min-reach-0.5), floor(max+reach+0.5)) rather than the
+1 -1
View File
@@ -88,7 +88,7 @@ void appendArc(std::vector<StrokePoint>& out, float cx, float cy, float radius,
// accounting for a possibly bottom-up (`flipped`) layout. Pulled out of the shell's LICE blend // 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 // 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 // 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) { inline std::size_t rasterRowOffset(int y, int height, int rowSpan, bool flipped) {
const int row = flipped ? height - 1 - y : y; const int row = flipped ? height - 1 - y : y;
return static_cast<std::size_t>(row) * static_cast<std::size_t>(rowSpan); return static_cast<std::size_t>(row) * static_cast<std::size_t>(rowSpan);
+29 -2
View File
@@ -289,6 +289,31 @@ static void testCoverageOutsideTheValidSpanReadsZero() {
CHECK(c.coverageAt(1000, 1000) == 0.0f); 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 ---------------------------------------------------- // --- raster row addressing ----------------------------------------------------
static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() { static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() {
@@ -296,8 +321,8 @@ static void testRasterRowOffsetMatchesUnflippedAndFlippedLayouts() {
CHECK(rasterRowOffset(0, 100, 240, false) == 0u); CHECK(rasterRowOffset(0, 100, 240, false) == 0u);
CHECK(rasterRowOffset(5, 100, 240, false) == 5u * 240u); CHECK(rasterRowOffset(5, 100, 240, false) == 5u * 240u);
CHECK(rasterRowOffset(99, 100, 240, false) == 99u * 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 // Flipped (bottom-up DIBs): row y is (height-1-y)*rowSpan — matches the free function
// accessor, `(h-1-y)*rowspan + x` (lice.cpp:2262). // LICE_GetPixel's own row math, `(h-1-y)*rowspan + x` (lice.cpp:2262).
CHECK(rasterRowOffset(0, 100, 240, true) == 99u * 240u); CHECK(rasterRowOffset(0, 100, 240, true) == 99u * 240u);
CHECK(rasterRowOffset(99, 100, 240, true) == 0u); CHECK(rasterRowOffset(99, 100, 240, true) == 0u);
CHECK(rasterRowOffset(40, 100, 240, true) == 59u * 240u); CHECK(rasterRowOffset(40, 100, 240, true) == 59u * 240u);
@@ -441,6 +466,8 @@ int main() {
testBoundsClipToTheClipRectAndCoverTheReach(); testBoundsClipToTheClipRectAndCoverTheReach();
testStrokeEntirelyOutsideTheClipDrawsNothing(); testStrokeEntirelyOutsideTheClipDrawsNothing();
testCoverageOutsideTheValidSpanReadsZero(); testCoverageOutsideTheValidSpanReadsZero();
testInteriorNonFiniteCoordinateRejectsTheWholeStroke();
testAddSegmentRejectsNonFiniteInputsDirectly();
testRasterRowOffsetMatchesUnflippedAndFlippedLayouts(); testRasterRowOffsetMatchesUnflippedAndFlippedLayouts();
testSubdivisionDoesNotChangeTheRenderedStroke(); testSubdivisionDoesNotChangeTheRenderedStroke();
testArcPointsLieOnTheCircleAndRespectTheFlatness(); testArcPointsLieOnTheCircleAndRespectTheFlatness();