diff --git a/docs/TODO.md b/docs/TODO.md index eafb197..005c8e5 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -194,6 +194,34 @@ Forward-looking follow-ups. Deferred by decision, not oversight — each entry r **Done looks like.** Not stated in the source beyond choosing one of the three placement options. +## Confirm the card name strip reads legibly at the shipping cell size (Ψ-W2-T1 DAW verification) + +**Context.** Ψ-W2-T1 (`capture-naming`) put the capture's label on the docked panel card, +across the top of the cell, drawn OVER the waveform thumbnail. Review found the strip's +text/primary was measured at ~1:1 contrast against the accent-lime waveform fill at the +shipping 140×84 cell size — a loud capture's peak reaches into the strip on 12 of its 13 +rows — and remediated it with a bg/base scrim behind the name (`kCardNameScrimAlpha`, +`core/ui/theme.h`) sized so the composite clears the WCAG 4.5:1 body floor against both the +bare fill and bare bg/cell (pinned in `test_theme.cpp`). + +**The wart.** The floor math is verified; the actual on-screen read is not. No `[verify — +DAW]` deferral was filed for this track's acceptance criterion ("the panel card shows the +name") when it landed, unlike the sibling Ψ tracks. + +**Intended fix.** N/A — no code change. Daniel views the docked panel with real captures +(quiet and loud material, long and short names) and confirms the name reads over the +waveform at the shipping cell size. + +**The constraint the fix MUST handle.** N/A — verification only. + +**Priority / risk.** Not stated. The math clears its floor with real margin (see +`testCardNameScrimClearsBodyFloorOnItsWorstBackground`), so this is a confirmation step, +not a suspected defect. + +**Done looks like.** Daniel confirms the card name reads legibly over both quiet and +loud waveform material at the shipping 140×84 cell size, or a follow-up adjusts the scrim +alpha and this entry is re-filed against the new value. + ## A realtime capture interrupted by a project switch leaves an untracked file behind **Context (found by the tracking-consolidation review, 2026-07-30).** `DriveRealtimeCapture` detects that the active project is no longer the one the in-flight capture belongs to, aborts the backend, and drops the handle. On a `Done` abort the backend has *already* moved the recorded WAV into the **original** project's bank folder (`capture_realtime_finalize`), so a file the tool created exists with no bank entry and no ledger record. diff --git a/src/core/capture/capture_name.cpp b/src/core/capture/capture_name.cpp index f35939b..c8aca5c 100644 --- a/src/core/capture/capture_name.cpp +++ b/src/core/capture/capture_name.cpp @@ -56,6 +56,10 @@ CaptureName composeCaptureName(const CaptureNameInputs& in) { if (base.empty()) base = trimmed(in.fallback); if (base.empty()) base = "capture"; base = truncateUtf8(base, kMaxSourceNameBytes); + // truncateUtf8 backs off over continuation bytes, so a name whose first kMaxSourceNameBytes + // bytes are ALL continuation bytes (0x80-0xBF) backs off to nothing — re-apply the "never an + // empty label" fallback after truncation, not just before it. + if (base.empty()) base = "capture"; CaptureName out; out.label = base; diff --git a/src/core/ui/card_meta.cpp b/src/core/ui/card_meta.cpp index 8d42d5e..eebe616 100644 --- a/src/core/ui/card_meta.cpp +++ b/src/core/ui/card_meta.cpp @@ -12,7 +12,9 @@ Rect cardNameStrip(const Rect& cell) { // is a text block, not a thumbnail. const int minHeight = 3 * kCardStripHeight; if (cell.width <= 2 * kCardStripPad || cell.height < minHeight) return Rect{}; - // Inset by 1px from the top edge so the name never sits on the selection border. + // Inset by 1px from the top edge so the name never sits on the focused-cell inner ring + // (drawn at rect.y+1, panel_render.cpp) — the selection border itself is at rect.y and is + // clear regardless. return Rect{cell.x + kCardStripPad, cell.y + 1, cell.width - 2 * kCardStripPad, kCardStripHeight}; } diff --git a/src/core/ui/card_meta.h b/src/core/ui/card_meta.h index 7bb0fcc..fbcf878 100644 --- a/src/core/ui/card_meta.h +++ b/src/core/ui/card_meta.h @@ -15,9 +15,13 @@ inline constexpr int kCardStripHeight = 12; inline constexpr int kCardStripPad = 3; // The strip the card's name line occupies: across the top of the cell, drawn OVER the -// waveform exactly as the length read-out is drawn over it at the bottom. Empty when the -// cell has no room for both strips plus a waveform worth looking at — the caller draws -// nothing rather than burying the card under text. +// waveform exactly as the length read-out is drawn over it at the bottom, rather than a +// reserved non-drawing band — kCardStripHeight (12px) is a small slice of the shipping 84px +// cell, and the overlay matches the bottom read-out's existing convention rather than +// introducing a second layout rule. The draw site scrims behind the text so it stays legible +// over the waveform's accent fill (`kCardNameScrimAlpha`, `theme.h`). Empty when the cell has +// no room for both strips plus a waveform worth looking at — the caller draws nothing rather +// than burying the card under text. Rect cardNameStrip(const Rect& cell); // Musical length inputs, taken straight off a Sample's capture-time stamp. tempoBpm 0 = unknown; diff --git a/src/core/ui/theme.h b/src/core/ui/theme.h index cfc8610..ab47446 100644 --- a/src/core/ui/theme.h +++ b/src/core/ui/theme.h @@ -97,6 +97,13 @@ KitColor compositeOver(const KitColor& over, const KitColor& under, double alpha // test composes the same value the shell draws with (see compositeOver). inline constexpr double kLoopSpanFillAlpha = 0.20; +// The card name strip's scrim: bg/base composited at this alpha UNDER the name text, so +// text/primary stays readable when a loud capture's waveform peak (accent/primary) reaches +// into the strip. Named HERE, same reason as kLoopSpanFillAlpha above — test_theme.cpp composes +// this exact value against the strip's worst-case background (accent/primary) to pin the 4.5:1 +// body floor Font::Micro answers to. +inline constexpr double kCardNameScrimAlpha = 0.75; + // Relative luminance per WCAG 2.1 (sRGB linearization + 0.2126/0.7152/0.0722 weighting). Alpha // is ignored — a translucent overlay's effective color is the caller's to compose first // (compositeOver). diff --git a/src/shell/capture/capture_batch.cpp b/src/shell/capture/capture_batch.cpp index 070c984..31714af 100644 --- a/src/shell/capture/capture_batch.cpp +++ b/src/shell/capture/capture_batch.cpp @@ -401,6 +401,10 @@ void RunRecaptureFromSource(ReaSamplerSession& session) req.sampleRate = recipe->sampleRate; req.channelCount = recipe->channelCount; req.bitDepth = WavBitDepth::Float32; + // orig->displayName is the ORIGINAL capture's label (recapture preserves identity, it + // does not re-mint it — see this function's header comment), so the regenerated file's + // stem carries the original capture's stamp, not this render's — do not read it as a + // render timestamp. req.baseName = orig->displayName.empty() ? "recapture" : orig->displayName; req.trackGuids = recipe->trackGuids; diff --git a/src/shell/capture/scope_resolve.cpp b/src/shell/capture/scope_resolve.cpp index e6930e4..5d276b0 100644 --- a/src/shell/capture/scope_resolve.cpp +++ b/src/shell/capture/scope_resolve.cpp @@ -7,6 +7,7 @@ #include "shell/capture/scope_resolve.h" +#include // strnlen — bounded read of GetTrackName's buffer #include // project-dir derivation for provenance parent resolution #include @@ -110,7 +111,9 @@ std::string trackName(MediaTrack* tr) // runs once per capture, not per frame. std::vector buf(1024, '\0'); if (!GetTrackName(tr, buf.data(), static_cast(buf.size()))) return {}; - return std::string(buf.data()); + // Bounded construction: the SDK doesn't document NUL-termination within bufOut_sz, so + // strnlen over the whole buffer (never past it) rather than trusting one. + return std::string(buf.data(), strnlen(buf.data(), buf.size())); } // Reads every track's P_RAZOREDITS (SDK header ~2899: space-separated triples of diff --git a/src/shell/capture/scope_resolve.h b/src/shell/capture/scope_resolve.h index 5adf0ba..a5ba132 100644 --- a/src/shell/capture/scope_resolve.h +++ b/src/shell/capture/scope_resolve.h @@ -58,10 +58,13 @@ bool resolveRange(double& start, double& end, std::string& why); // Collects the selected tracks (Track scope) into out.sourceTracks + GUIDs + names. bool collectSelectedTracks(ResolvedSource& out); -// The track's display name, read-only. GetTrackName (SDK header ~3626) is used rather -// than P_NAME because it already answers REAPER's own convention for an unnamed track -// ("Track N"), which is exactly the deterministic fallback a capture label wants; P_NAME -// would hand back an empty string instead. Empty only if the read itself fails. +// The track's display name. GetTrackName (SDK header ~3626) is used rather than P_NAME +// because it already answers REAPER's own convention for an unnamed track ("Track N"), +// which is exactly the deterministic fallback a capture label wants; P_NAME would hand +// back an empty string instead. `[verify]` the SDK header states neither that the read is +// non-mutating nor what a `false` return means; the call site treats it as read-only and +// treats `false` (or a `true` with an untouched buffer) the same way — an empty name, which +// falls through to the scope literal fallback either way, so both readings are safe. // Callers that build a ResolvedSource by hand (batch capture) use this directly. std::string trackName(MediaTrack* tr); diff --git a/src/shell/panel/panel_render.cpp b/src/shell/panel/panel_render.cpp index b197284..ada77d0 100644 --- a/src/shell/panel/panel_render.cpp +++ b/src/shell/panel/panel_render.cpp @@ -42,12 +42,18 @@ void drawCardMeta(LICE_IBitmap* bmp, const CellRect& rect, const Sample& s) { } // The capture's name across the top of the card. The kit clips with an end-ellipsis, so a -// long name shortens ON SCREEN only — the stored label is never truncated. An entry with -// no label (nothing writes one today, but old banks are not migrated) draws nothing. +// long name shortens ON SCREEN only — the stored label is never truncated. An entry with an +// empty label (old banks predate this track and are not migrated) draws nothing. void drawCardName(LICE_IBitmap* bmp, const CellRect& rect, const Sample& s) { if (s.displayName.empty()) return; const CellRect strip = cardNameStrip(rect); if (strip.empty()) return; + // Scrim behind the name: text/primary alone reads ~1:1 against the accent-lime waveform + // fill a loud capture's peak reaches into this strip. kCardNameScrimAlpha (core/ui/theme.h) + // is picked so bg/base composited at that alpha over accent/primary clears the 4.5:1 body + // floor Font::Micro answers to — pinned in test_theme.cpp. + LICE_FillRect(bmp, strip.x, strip.y, strip.width, strip.height, + toLice(roleColor(Role::BgBase)), static_cast(kCardNameScrimAlpha), 0); text(bmp, strip, s.displayName.c_str(), Font::Micro, Role::TextPrimary, Align::Left); } diff --git a/src/shell/panel/panel_state.h b/src/shell/panel/panel_state.h index 999f166..7abebf8 100644 --- a/src/shell/panel/panel_state.h +++ b/src/shell/panel/panel_state.h @@ -133,6 +133,7 @@ using ui::hitTestMenuButton; using ui::hitTestPruneButton; using ui::hitTestSlot; using ui::hitTestTabStrip; +using ui::kCardNameScrimAlpha; using ui::kCardStripHeight; using ui::kCardStripPad; using ui::menuButtonReserve; diff --git a/tests/test_capture_name.cpp b/tests/test_capture_name.cpp index 9f8ec5a..722c45b 100644 --- a/tests/test_capture_name.cpp +++ b/tests/test_capture_name.cpp @@ -103,6 +103,26 @@ static void testEmptyNameAndEmptyFallbackStillYieldALegalStem() { CHECK(sanitizeStem(n.stemBase) == "capture"); } +static void testAllWhitespaceNameFallsBackToTheScopeLiteral() { + // Distinct path from an empty name: trimmed() is what empties it, before truncation + // ever runs. + const CaptureName n = composeCaptureName(inputsFor({" "}, 0, "item")); + CHECK(n.label == "item 08-01 1432"); + CHECK(sanitizeStem(n.stemBase) == "item"); +} + +static void testNameThatTruncatesToNothingFallsBackToCapture() { + // Over-length and made entirely of a UTF-8 continuation byte (0x80-0xBF): not empty + // pre-truncation, so it survives trimmed()/the fallback chain as a real name — but + // truncateUtf8 backs off over continuation bytes and walks all the way to 0 (every byte + // in the first kMaxSourceNameBytes is a continuation byte), which used to leave an + // empty label. + const std::string allContinuation(kMaxSourceNameBytes + 10, '\x80'); + const CaptureName n = composeCaptureName(inputsFor({allContinuation})); + CHECK(n.label == "capture 08-01 1432"); + CHECK(sanitizeStem(n.stemBase) == "capture"); +} + static void testUnnamedTrackUsesReaperTrackNConvention() { // What GetTrackName actually hands back for an unnamed track — the deterministic // fallback rides in as an ordinary name, no special case in the composer. @@ -231,6 +251,8 @@ int main() { testEmptyNameFallsBackToTheScopeLiteral(); testNoSourceAtAllFallsBackToTheScopeLiteral(); testEmptyNameAndEmptyFallbackStillYieldALegalStem(); + testAllWhitespaceNameFallsBackToTheScopeLiteral(); + testNameThatTruncatesToNothingFallsBackToCapture(); testUnnamedTrackUsesReaperTrackNConvention(); testAllPunctuationNameKeepsTheLabelAndCollapsesTheStem(); testNonAsciiNameKeepsTheLabelAndCollapsesTheStem(); diff --git a/tests/test_theme.cpp b/tests/test_theme.cpp index 92dda3e..799f570 100644 --- a/tests/test_theme.cpp +++ b/tests/test_theme.cpp @@ -250,6 +250,25 @@ static void testTextOnHoverSurfaceClearsFloor() { CHECK(contrastRatio(roleColor(Role::TextDim), hover) >= textFloor(TextClass::Large)); } +// The card name strip (panel_render.cpp's drawCardName) draws text/primary Micro (10px, BODY +// class) over a bg/base scrim at kCardNameScrimAlpha, itself drawn over whatever the waveform +// left in that rect. Enumerate both backgrounds a loud/quiet capture can leave there: the +// accent-lime fill (a full-scale peak reaching the strip) and bare bg/cell (a quiet capture, +// nothing painted that high). Unscrimmed, text/primary reads ~1:1 against the fill — this is +// the load-bearing check that the scrim actually fixes it. +static void testCardNameScrimClearsBodyFloorOnItsWorstBackground() { + const KitColor scrim = roleColor(Role::BgBase); + const KitColor onFill = compositeOver(scrim, roleColor(Role::AccentPrimary), + kCardNameScrimAlpha); + const KitColor onCell = compositeOver(scrim, roleColor(Role::BgCell), kCardNameScrimAlpha); + CHECK(contrastRatio(roleColor(Role::TextPrimary), onFill) >= textFloor(TextClass::Body)); + CHECK(contrastRatio(roleColor(Role::TextPrimary), onCell) >= textFloor(TextClass::Body)); + // Without the scrim, text/primary on the bare accent-lime fill is UNDER floor — pins the + // defect the scrim exists to close, so a future removal of the scrim fails this first. + CHECK(contrastRatio(roleColor(Role::TextPrimary), roleColor(Role::AccentPrimary)) + < textFloor(TextClass::Body)); +} + // --- Single point of change (structural guarantee) ---------------------------- // // roleColor is the ONLY color source; there is no other public accessor that yields a @@ -346,6 +365,7 @@ int main() { testRegionTitleAccentsClearLargeFloorOnPanel(); testTextOnPastelFillClearsBodyFloor(); testTextOnHoverSurfaceClearsFloor(); + testCardNameScrimClearsBodyFloorOnItsWorstBackground(); testCurveTraceOnHoverSurfaceClearsFloor(); testSecondaryTertiaryAreDistinguishable(); testOverlayTraceClearsIndicatorFloorOnTheWaveform();