Fix audit citations, counts, and the L2-07/L2-09 mechanisms after code-review verification
This commit is contained in:
@@ -65,10 +65,11 @@ is direct evidence the pure layer was written with a non-Windows target in mind.
|
||||
### 1.2 `tests/` — portable.
|
||||
|
||||
91 test TUs. Only `tests/test_capture_paths.cpp` forks on `_WIN32`, and it asserts the
|
||||
non-Windows behaviour explicitly (lines 28–33, 50–51). Six other files matched a
|
||||
`C:\`/`C:/` grep — every hit is opaque *string data* fed to a pure function
|
||||
non-Windows behaviour explicitly (lines 28–33, 50–51). A `C:\`/`C:/` grep over `tests/` hits
|
||||
six files total, one of which is `test_capture_paths.cpp` itself; the five OTHER files — every
|
||||
hit opaque *string data* fed to a pure function
|
||||
(`test_bank_model.cpp:186–187,243–245`, `test_bake_wire.cpp:77`, `test_drag_out.cpp:291–293,388`,
|
||||
`test_origin_ledger.cpp:56`, `test_sample_usage.cpp:502`), not platform behaviour. They pass
|
||||
`test_origin_ledger.cpp:56`, `test_sample_usage.cpp:502`) — are not platform behaviour. They pass
|
||||
identically on Linux.
|
||||
|
||||
### 1.3 `app/` — portable.
|
||||
@@ -85,37 +86,64 @@ type, no conditional. The one hazard is behavioural, not structural — see **L2
|
||||
`_WIN32|WIN32|__APPLE__|_MSC_VER|__linux__|__GNUC__|_WIN64` over `src/**/*.{cpp,h}`). Their
|
||||
distribution is the story:
|
||||
|
||||
| Area | Guarded blocks | Shape |
|
||||
| Area | Lines | Shape |
|
||||
|---|---|---|
|
||||
| `shell/instrument/` (VST3 editor + embed) | 44 of 65 | Whole-TU `#ifdef _WIN32` … `#endif` — Windows-only by design |
|
||||
| `shell/panel/` | 9 | Include-selection + two small `#else` bodies |
|
||||
| `shell/persist/`, `shell/capture/`, `shell/actions/` | 6 | Trash fallback, `localtime_r`, the SWELL drag-out branch |
|
||||
| `core/` | 1 | The case-fold |
|
||||
| `src/resource.h`, `src/ext_keys.h` | comments only | — |
|
||||
| `shell/instrument/` (VST3 editor + embed) | 51 of 65 | Whole-TU `#ifdef _WIN32` … `#endif` — Windows-only by design |
|
||||
| `shell/panel/` | 8 | Include-selection + two small `#else` bodies |
|
||||
| `shell/persist/`, `shell/capture/`, `shell/actions/` | 4 | Trash fallback, `localtime_r`, the SWELL drag-out branch |
|
||||
| `core/` | 2 | The case-fold |
|
||||
|
||||
Two `shell/` directories are absent from this table because they were swept and came back
|
||||
completely clean, not because they went unchecked: `shell/view/` and `shell/bank_ops/` both
|
||||
return **zero** hits on the same grep (confirmed). Naming them here, since the table above is
|
||||
by hit-count and gives a reader no way to tell a zero-hit sweep from an unswept directory.
|
||||
|
||||
`src/resource.h` and `src/ext_keys.h` mention platform names only in comments — zero hits on
|
||||
this specific grep (re-run and confirmed), so they are not a row of this distribution; see
|
||||
`resource.h`/`resource.rc` under §7 for their actual (build-system) relevance.
|
||||
|
||||
The extension's guards are all *complete*: every `#ifdef _WIN32` that gates a **definition**
|
||||
has an `#else` (`draw_kit.cpp:12`, `capture.cpp:218`, `panel_audition.cpp:36,58`,
|
||||
`panel_state.h:20`, `prune_fs.cpp:169`, `drag_out_win.cpp:8`); the ones without an `#else`
|
||||
gate only an `#include` or a single statement (`panel_window.cpp:19,148`, `prune_fs.cpp:35`,
|
||||
`draw_kit.h:27`, `capture_paths.cpp:18`). **There is no undefined-symbol gap in the extension.**
|
||||
`draw_kit.h:27`, `capture_paths.cpp:18`). **There is no undefined-symbol gap from this guard
|
||||
structure** — every `#ifdef` that needs an `#else` has one. This is narrower than "no
|
||||
undefined-symbol gap in the extension" full stop: L2-02's `GetCurrentProcessId`
|
||||
(`instrument_drop_win.cpp:59`) is an undefined symbol on Linux precisely because it carries
|
||||
**no** `#ifdef` at all, so it falls outside what this guard-completeness check measures.
|
||||
|
||||
The instrument's guards are complete in the opposite direction: `reasampler_editor.h:156–269`
|
||||
wraps the entire paint/input/timer/`wndProc`/`invalidate` family in `#ifdef _WIN32`, and
|
||||
`editor_platform.cpp:298–306` stubs only the three IPlugView overrides that must exist. Every
|
||||
`invalidate()` call site is inside a guard (verified: 7 calls in `editor_session.cpp` at
|
||||
119, 126, 139, 141, 150, 156, 174 — all within the `#ifdef _WIN32` regions opened at 105 and
|
||||
173; `editor_controls.cpp`, `editor_models.cpp`, `instrument_bake.cpp` have zero). So the
|
||||
editor *links* off Windows — it just does nothing.
|
||||
`invalidate()` call site is inside a guard (verified exhaustively over `src/shell/instrument/`:
|
||||
53 call sites across eight files — `editor_input.cpp` (5), `editor_input_browse.cpp` (11),
|
||||
`editor_input_chrome.cpp` (10), `editor_input_curve.cpp` (4), `editor_input_deck.cpp` (9),
|
||||
`editor_input_waveform.cpp` (4), `editor_platform.cpp` (3, excluding the definition itself at
|
||||
`:65`), and `editor_session.cpp` (7, at 119, 126, 139, 141, 150, 156, 174, excluding a
|
||||
same-named mention inside a comment at `:106`) — each file whole-file- or whole-region-guarded
|
||||
by `#ifdef _WIN32` (e.g. `editor_input_browse.cpp:8` … `:177`); `reasampler_editor.h:266` is
|
||||
the declaration, not a call site. `editor_controls.cpp`, `editor_models.cpp`,
|
||||
`instrument_bake.cpp` have zero call sites, confirmed). So the editor *links* off Windows — it
|
||||
just does nothing.
|
||||
|
||||
### 1.5 What the platform layer actually provides (vendor-verified)
|
||||
|
||||
The extension's non-Windows path is SWELL-provided-by-host
|
||||
(`SWELL_PROVIDED_BY_APP` → function pointers resolved through `SWELL_dllMain`,
|
||||
`vendor/WDL/WDL/swell/swell-modstub-generic.cpp:135–145`). On Linux REAPER's own
|
||||
`libSwell.so` is built with `swell-gdi-lice.o`, `-DSWELL_LICE_GDI`, `-DSWELL_FREETYPE` and
|
||||
`-lfontconfig` (`vendor/WDL/WDL/swell/Makefile:100–107,141–151`) — i.e. the GDI SWELL
|
||||
answers with is the **LICE-backed, freetype/fontconfig** one, not the null stub in
|
||||
`swell-gdi-generic.cpp` (that file is `#ifndef SWELL_PROVIDED_BY_APP`, line 24).
|
||||
`vendor/WDL/WDL/swell/swell-modstub-generic.cpp:135–145`). `[verify — Linux]` **What is stated
|
||||
here about REAPER's own `libSwell.so` is only evidence about WDL's own Makefile, not about what
|
||||
Cockos actually ships** — `vendor/WDL/WDL/swell/Makefile:100–107,141–151` shows
|
||||
`swell-gdi-lice.o`/`-DSWELL_LICE_GDI` built whenever GDK is enabled (`ifndef NOGDK`, `:111`),
|
||||
with `-DSWELL_FREETYPE` gated behind `ifndef NOFREETYPE` (`:144`) and `-lfontconfig` gated a
|
||||
level deeper behind `ifndef NOFONTCONFIG` (`:149`) — i.e. the Makefile makes the LICE-backed,
|
||||
freetype/fontconfig SWELL the *default* build, not a certainty about the binary REAPER
|
||||
distributes. **Every downstream claim built on "REAPER's Linux SWELL is the fontconfig-backed
|
||||
GDK build" inherits this same unstated assumption** — that includes L2-09's entire mechanism,
|
||||
the GDK-backend evidence cited for L2-07, and the cursor (`GDK_*`) / modifier-key
|
||||
(`GetAsyncKeyState`) claims below, none of which are re-flagged individually; this paragraph is
|
||||
their one shared source of doubt. Contrast with the null stub in `swell-gdi-generic.cpp`
|
||||
(`#ifndef SWELL_PROVIDED_BY_APP`, line 24), which is not in play for a REAPER-hosted extension
|
||||
either way.
|
||||
|
||||
Everything the panel layer calls was checked by name against
|
||||
`vendor/WDL/WDL/swell/swell-functions.h` and `swell-types.h`. Present and real:
|
||||
@@ -143,11 +171,18 @@ Everything the panel layer calls was checked by name against
|
||||
(`swell-generic-gdk.cpp:2439–2441`), so `panel_state.h:409–411` works verbatim.
|
||||
- `GetTickCount` is SWELL-provided (`swell-functions.h:81`), so `panel_drag.cpp:308,324` — the
|
||||
only unguarded Win32-looking calls in the panel — are fine.
|
||||
- `SWELL_InitiateDragDropOfFileList` **does exist on Linux** and is a real implementation, not a
|
||||
stub: `swell-generic-gdk.cpp:3563–3592` (`gdk_drag_begin` + a nested `SWELL_RunMessageLoop`
|
||||
until capture drops, with a 500 ms no-motion timeout). The headless backend
|
||||
(`swell-generic-headless.cpp:246`) and macOS (`swell-dlg.mm:3534`) also define it; it is
|
||||
declared at `swell-functions.h:1011`. **The doc's claim holds.**
|
||||
- `SWELL_InitiateDragDropOfFileList` **does exist on Linux**, and is a GDK implementation on
|
||||
the GDK backend: `swell-generic-gdk.cpp:3563–3592` sets up a hidden drop-source window and
|
||||
spins a nested `SWELL_RunMessageLoop` until capture drops, with a 500 ms no-motion timeout;
|
||||
the actual `gdk_drag_begin` call is inside the shared `dropSourceWndProc` helper it invokes
|
||||
(`:3446`), not textually inside the `3563–3592` range itself. The headless backend defines
|
||||
the **same-named function as an empty no-op stub** —
|
||||
`swell-generic-headless.cpp:246–248` is `void SWELL_InitiateDragDropOfFileList(...) { }` — so
|
||||
the "not a stub" framing only holds for the GDK backend specifically. The two backends are
|
||||
mutually exclusive (`swell-generic-headless.cpp:28` is `#ifndef SWELL_TARGET_GDK`), so nothing
|
||||
breaks in a GDK build, but a reader should not take "does exist on Linux" to mean "is
|
||||
implemented in every Linux SWELL backend." macOS (`swell-dlg.mm:3534`) also defines it; it is
|
||||
declared at `swell-functions.h:1011`.
|
||||
- LICE itself is portable: `lice.h:31` includes `swell-types.h` off Windows, `lice.cpp:22`
|
||||
includes `swell.h`, `LICE_SysBitmap::__resize` has a SWELL framebuffer path
|
||||
(`lice.cpp:179–182`), and `LICE_CachedFont::DrawTextImpl` carries real non-Windows branches
|
||||
@@ -216,8 +251,20 @@ here says a gap exists — but nothing rules it out either, and the failure mode
|
||||
opaque.
|
||||
|
||||
**Severity: Blocker (conditional)** `[verify — Linux]` — a silent load refusal with no
|
||||
diagnostic is the worst possible first-run experience for a port.
|
||||
**Effort: S.**
|
||||
diagnostic is the worst possible first-run experience for a port. **Scope note:** the
|
||||
all-or-nothing mechanism itself is identical on Windows too
|
||||
(`reaper_plugin_functions.h:45–46` — "an older version of REAPER may not succeed in loading",
|
||||
no platform fork in the loader) — nothing about the *mechanism* is Linux-specific, only whether
|
||||
*this particular* build happens to be missing an entry is unknown. Graded here as a Linux
|
||||
Blocker on failure-mode quality alone (per the Mechanism above), the same axis L2-04 is
|
||||
graded on below, despite L2-04's mechanism being certain rather than speculative — see L2-04's
|
||||
calculus note. Arguably this belongs as a cross-platform robustness note rather than a
|
||||
Linux-exclusive finding; left here because the fix is trivial and worth doing regardless of
|
||||
platform.
|
||||
**Effort: S** — confirmed against the two Direction options below: the `ShowConsoleMsg`
|
||||
diagnostic is one new line on the existing failure branch (`main.cpp:293`), and the
|
||||
`REAPERAPI_MINIMAL` swap follows a pattern already used elsewhere in this codebase
|
||||
(`panel_window.cpp:26–31`, `panel_audition.cpp:12–16`) rather than inventing a new one.
|
||||
**Direction.** Either switch `main.cpp` to `REAPERAPI_MINIMAL` + an explicit `WANT` list (the
|
||||
other TUs already do this — e.g. `panel_window.cpp:26–31`, `panel_audition.cpp:12–16`), or keep
|
||||
the full load but print the failure count via `rec->GetFunc("ShowConsoleMsg")` before returning
|
||||
@@ -226,8 +273,11 @@ the full load but print the failure count via `rec->GetFunc("ShowConsoleMsg")` b
|
||||
### L2-04 — every persisted floating-point number is `LC_NUMERIC`-dependent, in both directions
|
||||
**Location (writers):** `src/core/json/json.cpp:39–43` (`%.17g` — the bank index, view model and
|
||||
tracking ledger all serialize through it), `src/core/model/provenance.cpp:40` (`%.17g` — the
|
||||
provenance blob inside the bank JSON), `src/core/capture/tail_control.cpp:74` (`%.17g` — the
|
||||
per-project tail setting).
|
||||
provenance blob inside the bank JSON). Two writers, not three: `tail_control.cpp:74` is a
|
||||
comment describing the format ("Byte-identical to the former snprintf writer:
|
||||
`{"mode":%d,"manualMs":%.17g}`"), not a write site — the actual per-project tail-setting write
|
||||
is `tail_control.cpp:78–79`, which already goes through `json::numToStr` (the same
|
||||
`json.cpp:39–43` codec cited above), so it is the same writer, not a third one.
|
||||
**Location (readers):** `src/core/json/json.cpp:190–196` (`std::strtod`),
|
||||
`src/core/wire/wire.cpp:121–130` (`Cursor::fieldDouble`, `std::strtod`),
|
||||
`src/core/capture/render_settings.cpp:178–186` (`std::stod` over REAPER's own `P_RAZOREDITS`).
|
||||
@@ -236,7 +286,7 @@ per-project tail setting).
|
||||
comma-decimal locale the writers emit `1,5`, which makes the bank JSON *structurally invalid*
|
||||
(an extra separator inside an object) — the whole index fails to parse on the next load. The
|
||||
readers are honestly fail-closed (all three require whole-token consumption:
|
||||
`json.cpp:194`, `wire.cpp:127`, `render_settings.cpp:183`), so they degrade to "malformed"
|
||||
`json.cpp:194`, `wire.cpp:127`, `render_settings.cpp:184`), so they degrade to "malformed"
|
||||
rather than silently truncating — but that means a razor range or a bank field simply
|
||||
disappears. On Windows the CRT's start-up locale is `"C"` and nothing in this codebase calls
|
||||
`setlocale`, which is why this has never fired. On Linux the process locale is far more likely
|
||||
@@ -247,11 +297,19 @@ when built with `SWELL_SUPPORT_GTK` (`swell-generic-gdk.cpp:366`; the `#else` br
|
||||
process can do the same.
|
||||
|
||||
**Severity: Major** — builds and runs; under a non-C `LC_NUMERIC` the bank index is written
|
||||
unparseable and the project's whole bank is lost on reload.
|
||||
**Effort: M** — three writers and three readers, all in `core/`, all unit-testable; the fix is
|
||||
unparseable and the project's whole bank is lost on reload. **Calculus note:** unlike L2-03
|
||||
(graded Blocker above on zero direct evidence, purely on failure-mode quality), this finding's
|
||||
mechanism is confirmed by reading the actual writer/reader call sites, and its failure mode —
|
||||
the entire bank index becomes unparseable — is at least as severe as L2-03's. It is graded only
|
||||
Major here because it additionally requires a non-C `LC_NUMERIC` in the hosting process, a
|
||||
likelihood factor L2-03 does not apply to its own claim. Reading both on the same axis (either
|
||||
both by failure-mode quality, or both by likelihood) would put them closer together than
|
||||
Blocker/Major suggests; resolving that is a scope call for whoever prioritizes the two, not a
|
||||
fact this audit can settle statically.
|
||||
**Effort: M** — two writers and three readers, all in `core/`, all unit-testable; the fix is
|
||||
a locale-independent path (`std::to_chars`/`std::from_chars`, C++17, or an explicit
|
||||
`std::locale::classic()`-bound stream), not a `setlocale` call in a plugin.
|
||||
**Direction.** Make the number codec locale-independent at its three writers and three readers;
|
||||
**Direction.** Make the number codec locale-independent at its two writers and three readers;
|
||||
add a pure test that pins the emitted text for a fractional value. Do **not** "fix" this by
|
||||
calling `setlocale` — an extension must not mutate the host's locale.
|
||||
`[verify — Linux]` — read `LC_NUMERIC` inside a running REAPER-Linux process (e.g.
|
||||
@@ -316,25 +374,43 @@ having on Windows too.
|
||||
instead. Reading the GDK backend: on a URI-list selection-notify, SWELL resolves the top-level
|
||||
window, walks down with `ChildWindowFromPoint` (which **is** descending — `for(;;)` loop,
|
||||
`swell-wnd-generic.cpp:6956–6981`), and `SendMessage(cw, WM_DROPFILES, (WPARAM)gobj, 0)`
|
||||
(`swell-generic-gdk.cpp:1622`). The default child proc forwards `WM_DROPFILES` up to the parent
|
||||
only when the window *lacks* `WS_EX_ACCEPTFILES` (`swell-wnd-generic.cpp:7680–7688`) — i.e. the
|
||||
opt-in on SWELL is an ex-style bit, not an API call, and the message reaches the window under
|
||||
the pointer either way. So the panel's own `dlgProc` plausibly receives the drop **without any
|
||||
opt-in at all**. Two supporting details check out: SWELL's `DragQueryFile` reads the same
|
||||
`DROPFILES` layout the handler assumes (`swell-wnd-generic.cpp:7730+`), and SWELL's `DragFinish`
|
||||
is a documented no-op ("caller will free hdrops", `:7725–7728`) while SWELL itself
|
||||
`GlobalFree`s the handle right after `SendMessage` (`swell-generic-gdk.cpp:1623–1624`) — so
|
||||
`panel_window.cpp:59` calling `DragFinish` is safe, not a double-free.
|
||||
(`swell-generic-gdk.cpp:1622`). The default child proc forwards an *unhandled* `WM_DROPFILES`
|
||||
up to the parent only when the window *lacks* `WS_EX_ACCEPTFILES`
|
||||
(`swell-wnd-generic.cpp:7681`) — but that bit's only effect is to **suppress** the up-forward;
|
||||
it does not enable acceptance, and the message already reaches the window under the pointer via
|
||||
the `ChildWindowFromPoint` descent regardless of the bit's state. So the panel's own `dlgProc`
|
||||
plausibly receives the drop **without any opt-in at all**, but the earlier "ex-style bit, not
|
||||
an API call" framing was backwards about what the bit does. The real SWELL opt-in surface for
|
||||
`WS_EX_ACCEPTFILES` is the dialog *resource*, not a runtime call: `swell_resgen.pl:10` and
|
||||
`swell_resgen.php:204` translate a dialog's `WS_EX_ACCEPTFILES` style to
|
||||
`SWELL_DLG_WS_DROPTARGET`, and `swell-dlg-generic.cpp:320–321` sets
|
||||
`h->m_exstyle |= WS_EX_ACCEPTFILES` from that flag at dialog-creation time. `src/resource.rc:19`
|
||||
declares `STYLE WS_CHILD` only — no `WS_EX_ACCEPTFILES` — so the bit is off on Linux regardless
|
||||
of anything `panel_window.cpp` does at runtime. That makes L2-07's real seam `src/resource.rc` +
|
||||
resgen, the SAME work as L2-06 and §7 bullet 1, not an independent one-liner: a runtime
|
||||
`SetWindowLong(..., GWL_EXSTYLE, ... | WS_EX_ACCEPTFILES)` would only suppress the
|
||||
unhandled-drop forward-to-parent — it would not change whether the drop reaches `dlgProc` in
|
||||
the first place, which already happens via the `ChildWindowFromPoint` descent independent of
|
||||
the bit. Note also the doc's own conclusion is better-supported than it knew: `src/resource.rc:21–22`
|
||||
is `BEGIN`/`END` — zero child controls — so the GDK `ChildWindowFromPoint` descent has nowhere
|
||||
to land but the panel HWND itself. Two supporting details still check out: SWELL's
|
||||
`DragQueryFile` reads the same `DROPFILES` layout the handler assumes
|
||||
(`swell-wnd-generic.cpp:7730+`), and SWELL's `DragFinish` is a documented no-op ("caller will
|
||||
free hdrops", `:7725–7728`) while SWELL itself `GlobalFree`s the handle right after
|
||||
`SendMessage` (`swell-generic-gdk.cpp:1623–1624`) — so `panel_window.cpp:59` calling
|
||||
`DragFinish` is safe, not a double-free.
|
||||
|
||||
**Severity: Major `[verify — Linux]`** — if the routing does *not* reach a docked child dialog,
|
||||
one of the three ingest surfaces (file drop onto the bank panel) is silently dead with no error.
|
||||
The evidence above says it probably works; it is not proof.
|
||||
**Effort: S** — if the verify fails, the fix is
|
||||
`SetWindowLong(hwnd, GWL_EXSTYLE, GetWindowLong(hwnd, GWL_EXSTYLE) | WS_EX_ACCEPTFILES)`
|
||||
(both symbols exist: `swell-functions.h:319–320`, `swell-types.h`).
|
||||
**Direction.** Verify first, then either do nothing or add the ex-style set in the `#else`.
|
||||
`[verify — Linux]` drop a WAV onto the docked panel and check whether `WM_DROPFILES` reaches
|
||||
`dlgProc`.
|
||||
The evidence above says it probably works; it is not proof, and it cannot be exercised until
|
||||
the panel itself renders (T1's resgen wiring — see L2-06).
|
||||
**Effort:** no independent source-side fix exists — this is the same resgen work already
|
||||
costed under L2-06 / §7 bullet 1. Do not add an `Effort: S` line here; adding the
|
||||
`SetWindowLong` call would compile, do nothing observable, and send an implementer looking for
|
||||
a bug that isn't where they'd look.
|
||||
**Direction.** No separate fix. Once the resgen work lands the panel dialog with
|
||||
`WS_EX_ACCEPTFILES` set from the resource, verify: `[verify — Linux]` drop a WAV onto the
|
||||
docked panel and confirm `WM_DROPFILES` reaches `dlgProc`.
|
||||
|
||||
### L2-08 — the VST3 instrument has no Linux editor, and no automatable parameters to fall back to
|
||||
**Location:** `src/shell/instrument/editor_platform.cpp:37–42` (`isPlatformTypeSupported`
|
||||
@@ -345,8 +421,8 @@ off Windows). `reasampler_processor.cpp:360–365` still hands the host a `ReaSa
|
||||
|
||||
**Mechanism.** The VST3 SDK *does* support Linux — `kPlatformTypeX11EmbedWindowID`
|
||||
(`vendor/vst3sdk/pluginterfaces/gui/iplugview.h:79`), `Linux::IRunLoop` with
|
||||
`registerEventHandler`/`registerTimer` (`:267–279`), `Linux::IEventHandler` (`:224`),
|
||||
`Linux::ITimerHandler` (`:238`), and a `linuxmain.cpp` module entry
|
||||
`registerEventHandler`/`registerTimer` (`:267–279`), `Linux::IEventHandler` (`:223`),
|
||||
`Linux::ITimerHandler` (`:239`), and a `linuxmain.cpp` module entry
|
||||
(`vendor/vst3sdk/public.sdk/source/main/linuxmain.cpp`). The instrument simply does not
|
||||
implement any of it. The consequence compounds: with `isPlatformTypeSupported` false for every
|
||||
type, the host falls back to a **generic parameter UI** — and a grep for
|
||||
@@ -381,25 +457,41 @@ already *is* that seam; it needs a sibling, not a rewrite.
|
||||
**Location:** `src/shell/panel/draw_kit.cpp:70–77` (`loadFont`), and the two literal faces it is
|
||||
called with — Segoe UI and Consolas, per `draw_kit.h:63–64`.
|
||||
|
||||
**Mechanism.** On Linux `CreateFont` goes through fontconfig: `FcPatternAddString(pat, FC_FAMILY,
|
||||
lfFaceName)` → `FcFontMatch` (`swell-gdi-lice.cpp:450–487`). `FcFontMatch` never fails — it
|
||||
returns the best available substitute. So `loadFont`'s failure branch (`draw_kit.cpp:74`,
|
||||
"if (!hf) return") will not fire, and the kit will draw in *some* font with different metrics.
|
||||
Consequences: the kit's end-ellipsis clipping (`draw_kit.h:70–74`) is tuned against Segoe's
|
||||
advance widths, so labels will clip at different points; `Font::ValueMono` is only tabular if
|
||||
fontconfig's substitution for "Consolas" happens to be monospaced, which is a distro-config
|
||||
property, not a guarantee. The two WCAG `static_assert`s (`draw_kit.cpp:53–54`) are on **pixel
|
||||
height and weight**, not on the face, so they still hold — the contrast entitlement is not at
|
||||
risk.
|
||||
**Mechanism.** On Linux `CreateFont` goes through fontconfig when built with
|
||||
`-DSWELL_FONTCONFIG` (gated per the `[verify — Linux]` note in §1.5):
|
||||
`FcPatternAddString(pat, FC_FAMILY, lfFaceName)` → `FcConfigSubstitute`/`FcDefaultSubstitute` →
|
||||
`FcFontMatch` (`swell-gdi-lice.cpp:450–487`). Whether `FcFontMatch` itself can return failure is
|
||||
a claim about fontconfig's own internals — fontconfig is not vendored here, so it is dropped
|
||||
rather than asserted uncited (this doc's own evidence-discipline rule). What the vendored code
|
||||
does show: even a successful match doesn't guarantee a loadable face —
|
||||
`swell-gdi-lice.cpp:480–484` only sets `face` when `FcPatternGetString(hit, FC_FILE, …)`
|
||||
resolves to a nonempty string AND the following `FT_New_Face` succeeds; either step can fail,
|
||||
leaving `face` NULL, and in this build path there is no further fallback — the
|
||||
`MatchFont`/LiberationSans/DejaVuSans list at `:493–539` compiles only in the `#else`
|
||||
(`#ifndef SWELL_FONTCONFIG`) branch. However `swell-gdi-lice.cpp:400–401,561,564` show
|
||||
`CreateFont` always allocates and returns a non-null `HGDIOBJ__*` regardless of whether `face`
|
||||
resolved — the failure is recorded internally (`font->typedata = NULL`), not as a null return —
|
||||
so `draw_kit.cpp:74`'s own `if (!hf) return` guard does **not** catch this failure mode; `hf`
|
||||
comes back non-null either way. Whatever degraded outcome exists here comes from
|
||||
`LICE_CachedFont`'s own handling of a null-`typedata` font (§1.5 already cites real
|
||||
non-Windows branches in `lice_textnew.cpp:236–249,829,1027–1028,1040`), not from `loadFont`'s
|
||||
stated guard — `[verify — Linux]`. The two WCAG `static_assert`s (`draw_kit.cpp:53–54`) are on
|
||||
**pixel height and weight**, not on the face, so they still hold regardless.
|
||||
|
||||
**Severity: Minor** — cosmetic; nothing breaks, layout gets uglier and possibly clipped.
|
||||
**Effort: S.**
|
||||
**Direction.** Add a platform fallback face list at the one place the faces are chosen
|
||||
(`draw_kit.cpp`'s `loadFont` call sites — the header already promises "the face is chosen here so
|
||||
a change is one line", `:69`). DejaVu Sans / DejaVu Sans Mono are the safe Linux defaults; note
|
||||
that SWELL's own no-fontconfig fallback list names LiberationSans/DejaVuSans and
|
||||
LiberationMono/DejaVuSansMono (`swell-gdi-lice.cpp:505–507`), which is a reasonable precedent to
|
||||
copy. This same change fixes macOS (San Francisco / Menlo), one code path.
|
||||
**Severity: Minor** `[verify — Linux]` — cosmetic at best; whether it degrades further than
|
||||
wrong metrics (e.g. to no text at all) rests on the unconfirmed downstream null-face handling
|
||||
above, not on anything this audit can read statically.
|
||||
**Effort: S** — confirmed against the actual call sites: five total (`draw_kit.cpp:154–158`,
|
||||
four using "Segoe UI", one "Consolas"), each a single string-literal argument. A platform
|
||||
fallback is a `#ifdef`-guarded literal swap at those five sites, not a new mechanism —
|
||||
`draw_kit.cpp:69`'s comment ("the face is chosen here so a change is one line") describes one
|
||||
call's literal, not all five; the S estimate rests on there being only five call sites, not on
|
||||
that comment's wording.
|
||||
**Direction.** Add a platform fallback face list at the five `loadFont` call sites
|
||||
(`draw_kit.cpp:154–158`). DejaVu Sans / DejaVu Sans Mono are the safe Linux defaults; SWELL's
|
||||
own no-fontconfig fallback list names LiberationSans/DejaVuSans and
|
||||
LiberationMono/DejaVuSansMono (`swell-gdi-lice.cpp:505–507`), a reasonable precedent to copy.
|
||||
This same change fixes macOS (San Francisco / Menlo), one code path.
|
||||
|
||||
### L2-10 — OS drag-out on SWELL loses the copy-only mask, the readiness probe, and the outcome
|
||||
**Location:** `src/shell/actions/drag_out_win.cpp:249–282` (the `#else` branch);
|
||||
@@ -417,7 +509,10 @@ so (c) is inert today. Note the call path is already Linux-correct in one import
|
||||
`handOffToOs` releases capture and resets drag state (`panel_drag.cpp:240–241`) *before*
|
||||
invoking the drag, which matters because SWELL's implementation takes capture on its own hidden
|
||||
window and spins a nested `SWELL_RunMessageLoop` until capture drops
|
||||
(`swell-generic-gdk.cpp:3543–3552`) — with a 500 ms no-motion timeout that has no Windows analog.
|
||||
(`swell-generic-gdk.cpp:3575–3584`, inside `SWELL_InitiateDragDropOfFileList` — the function
|
||||
`drag_out_win.cpp:270` actually calls; the body is identical to the sibling
|
||||
`SWELL_InitiateDragDrop` at `:3531–3560`, which is where an earlier draft of this citation
|
||||
pointed) — with a 500 ms no-motion timeout that has no Windows analog.
|
||||
|
||||
**Severity: Minor** — the feature works; one safety property becomes conventional rather than
|
||||
structural.
|
||||
@@ -435,7 +530,9 @@ structurally impossible" as a Windows-only claim in any future spec text.
|
||||
case-**insensitive**, so the same `#ifdef _WIN32` under-folds there — a pre-existing macOS
|
||||
defect this audit surfaces but does not own.
|
||||
|
||||
**Severity: Minor** (no Linux defect). **Effort: S.**
|
||||
**Severity: Minor** (no Linux defect). **Effort: S** — confirmed: the one platform conditional
|
||||
in `core/` is a single `#ifdef _WIN32` block (`capture_paths.cpp:18–20`) with no other call
|
||||
site depending on the token; a predicate swap is a one-block change.
|
||||
**Direction.** No Linux action. If macOS is ever targeted, the predicate wants to be
|
||||
"case-insensitive filesystem", not "Windows".
|
||||
|
||||
@@ -466,7 +563,18 @@ Recorded so the sweep's negative results are as auditable as its findings.
|
||||
four were the symbol name appearing in a *comment* (`render_settings.h:53`,
|
||||
`ext_state_read.h:20`, `action_registry.h:7`) or a correct `<cmath>` `std::abs`
|
||||
(`velocity_curve.cpp:229`, `<cmath>` at `:6`), and `processor_reload.cpp`'s `std::unique_ptr`
|
||||
comes from `reasampler_processor.h:12`. **No missing include found.**
|
||||
comes from `reasampler_processor.h:12`. **No missing include found — but this only covers the
|
||||
ten headers above.** The two headers most likely to produce the classic
|
||||
MSVC-transitively-provides failure under libstdc++, `<vector>` and `<string>`, were excluded
|
||||
from that sweep; re-run including them: **45 files use `std::vector` without `#include
|
||||
<vector>`, and 52 use `std::string` without `#include <string>`** (raw grep-diff counts,
|
||||
unread). `<map>` and `<set>` add 2 and 3 more respectively. These 102 raw hits were **not**
|
||||
individually verified the way the five above were (that would mean reading 102 files); the
|
||||
claim this sweep actually supports is "up to 102 files may rely on transitive inclusion for
|
||||
`<vector>`/`<string>`/`<map>`/`<set>`, unconfirmed one by one" — not "no missing include
|
||||
found." The two header cases that matter most for the extension's own build
|
||||
(`shell/capture/capture_orchestrator.h`, `src/ext_keys.h`) are confirmed satisfied
|
||||
transitively.
|
||||
- **Templates / two-phase lookup.** Exactly 7 templates in the tree
|
||||
(`core/wire/bytes.h:25,61`, `core/wire/ext_state_read.h:37`,
|
||||
`core/instrument/engine/play_params.h:191,203,208,218`). None derives from a dependent base,
|
||||
@@ -564,11 +672,12 @@ changed behaviour; **AT RISK** = depends on an unverified assumption.
|
||||
| TCP/MCP embed strip | **GONE** | `REAPER_FXEMBED_WM_IS_SUPPORTED` returns 0 | `reasampler_embed.cpp:135–141` |
|
||||
| Kit fonts (Segoe UI / Consolas) | **DIFFERENT** | fontconfig substitutes silently; metrics and ellipsis points shift; ValueMono may lose tabular alignment | `swell-gdi-lice.cpp:450–487` |
|
||||
| OS drag-out copy-only guarantee | **DIFFERENT** | No effect mask; copy-only is conventional not structural | `drag_out_win.cpp:257–270` |
|
||||
| OS drag-out readiness/outcome | **DIFFERENT** | Probe degrades to "non-empty"; return is always `true`; 500 ms no-motion timeout has no Windows analog | `drag_out_win.cpp:276–278`, `swell-generic-gdk.cpp:3547–3548` |
|
||||
| OS drag-out readiness/outcome | **DIFFERENT** | Probe degrades to "non-empty"; return is always `true`; 500 ms no-motion timeout has no Windows analog | `drag_out_win.cpp:276–278`, `swell-generic-gdk.cpp:3579–3580` |
|
||||
| Panel file-drop ingest | **AT RISK** | Depends on SWELL delivering `WM_DROPFILES` without the Win32 opt-in | `panel_window.cpp:145–150` |
|
||||
| Bank index float round-trip | **AT RISK** | Unparseable under a non-C `LC_NUMERIC` | `json.cpp:39–43,190–196` |
|
||||
| Extension load | **AT RISK** | Any one unresolved API name in ~869 = silent refusal | `main.cpp:292–293` |
|
||||
| Path case sensitivity | **DIFFERENT (correct)** | No case-fold; correct for Linux, and the tests already assert it | `capture_paths.cpp:18–20` |
|
||||
| Prune reclaim on a symlinked bank file | **AT RISK** | `fs::directory_iterator` + `is_regular_file()` follows symlinks (C++17); size is read from the target via `file_size()` but `fs::remove` deletes the link, not the target — prune reports N bytes reclaimed and reclaims zero. Symlinked media folders are far more idiomatic on Linux than Windows. | `prune_fs.cpp:113–124` |
|
||||
| Audition / preview | **INTACT** | `preview_register_t` fork matches the SDK; `PlayPreview`/`StopPreview` are REAPER API | `panel_audition.cpp:36–63` |
|
||||
| Docking, menus, cursors, keyboard, wheel, capture rollback, tooltips | **INTACT** | All SWELL-provided; verified call-by-call | §1.5 |
|
||||
| Capture pillar (offline + realtime), persist, tracking, prune *computation* | **INTACT** | Pure `core/` + REAPER API only | §1.1 |
|
||||
@@ -594,7 +703,7 @@ changed behaviour; **AT RISK** = depends on an unverified assumption.
|
||||
distro decides whether L2-09 is cosmetic or a real readability regression on numeric readouts.
|
||||
6. **`SWELL_InitiateDragDropOfFileList` acceptance semantics.** Whether a GDK-initiated file drag
|
||||
is accepted as a copy by common targets (a file manager, another DAW), and whether the 500 ms
|
||||
no-motion timeout (`swell-generic-gdk.cpp:3547–3548`) cancels a slow user gesture.
|
||||
no-motion timeout (`swell-generic-gdk.cpp:3579–3580`) cancels a slow user gesture.
|
||||
7. **Prune against an in-use file.** Play an item from a bank file, prune it, confirm the audio
|
||||
survives until the fd closes and the file is then unrecoverable — the concrete demonstration
|
||||
behind L2-05(b).
|
||||
@@ -624,12 +733,18 @@ Dependency order only — this is not a plan, and it assigns no waves.
|
||||
3. **Make the panel appear.** L2-06 (the missing-dialog diagnostic) alongside T1's resgen wiring.
|
||||
The diagnostic should land first so the resgen step can be confirmed rather than assumed.
|
||||
4. **Then, and only then, verify.** `[verify — Linux]` items 1, 3, 5, 6, 7 all need a running
|
||||
panel. Item 2 is already answered by step 2.
|
||||
panel. Item 2 is already answered by step 2. **The critical path for this step runs through
|
||||
the parallel build-system track, not this one:** "a running panel" means T1's resgen wiring
|
||||
(§7 bullet 1) has landed — this doc doesn't own that work and doesn't know its schedule, so
|
||||
step 3's "alongside T1's resgen wiring" is doing real scheduling work, not just sequencing
|
||||
flavor. Anyone driving this ordering sketch needs to coordinate with T1 before step 4 can
|
||||
start, not just after step 3 finishes on this track's own items.
|
||||
5. **Data integrity.** L2-04 (locale-independent number codec) — pure `core/` work, unit-testable
|
||||
on Windows, and it should land before any Linux user saves a project. Sequenced after the
|
||||
verify only so its urgency is known; the *work* is not blocked.
|
||||
6. **Deletion safety.** L2-05 (XDG trash or a platform-aware confirm) — gated on `[Daniel]` (2).
|
||||
7. **Ingest opt-in, if needed.** L2-07 — strictly gated on `[verify — Linux]` (3).
|
||||
7. **Ingest opt-in, if needed.** L2-07 — no independent fix; it is the resgen work already
|
||||
sequenced in step 3, strictly gated on `[verify — Linux]` (3) to confirm it's needed at all.
|
||||
8. **Cosmetics.** L2-09 (fonts), L2-10 (drag-out doc). Independent of everything above.
|
||||
9. **Separately, if ruled in.** L2-08 — the instrument's X11 editor. No dependency in either
|
||||
direction on 1–8 except L2-01, which the shared `draw_kit` needs regardless.
|
||||
@@ -669,7 +784,7 @@ Dependency order only — this is not a plan, and it assigns no waves.
|
||||
| L2-04 | Persisted floats are `LC_NUMERIC`-dependent, both directions | Major | M | both |
|
||||
| L2-05 | Prune loses trash AND the locked-file backstop | Major | M | extension |
|
||||
| L2-06 | Panel dialog never created; silent no-op | Major | S | extension |
|
||||
| L2-07 | No drop-accept opt-in on SWELL; ingest-by-drop unverified | Major `[verify]` | S | extension |
|
||||
| L2-07 | No drop-accept opt-in on SWELL; real seam is resgen, same as L2-06 | Major `[verify]` | — (see L2-06) | extension |
|
||||
| L2-08 | No Linux VST3 editor; no parameters to fall back to | Major | L | instrument |
|
||||
| L2-09 | Segoe UI / Consolas substitute silently via fontconfig | Minor | S | both |
|
||||
| L2-10 | SWELL drag-out: no copy mask, no probe, no outcome | Minor | S | extension |
|
||||
|
||||
Reference in New Issue
Block a user