diff --git a/AGENT.md b/AGENT.md new file mode 100644 index 0000000..ce89950 --- /dev/null +++ b/AGENT.md @@ -0,0 +1,83 @@ +# SerumAlt agent guidance + +## Project + +SerumAlt is a JUCE 7 wavetable synthesiser using C++17. Project metadata is in +`CMakeLists.txt`. Production formats are VST3 and Standalone on Linux, VST3/AU/Standalone +on macOS, and VST3 on Windows. First-party classes use `namespace serum`; executable +entry points and JUCE's `createPluginFilter` are outside that namespace. + +## Build and verification + +Read README.md's Building section before changing scripts, dependencies, or CMake. +It is the maintained source for prerequisites, options and artifact paths. + +- Linux production: `./build_linux.sh`. +- macOS production, on a Mac: `./build_macos.sh`. +- Windows x64 VST3, cross-compiled on Linux: `./build_windows.sh`. +- Native QA build and execution: `./build_harness.sh --run`. +- Shell syntax: `for script in build_*.sh; do bash -n "$script"; done`. + +Production scripts explicitly enable `SERUMALT_BUILD_PLUGIN` and disable +`SERUMALT_BUILD_TESTS`. The harness script does the reverse in a separate directory. +Building `SerumAltTest` alone does not execute it; `--run` invokes CTest. +When changing CMake, verify production has no `SerumAltTest` target and the +harness-only build has no `SerumAlt`, VST3, AU, or Standalone target. + +Keep existing build trees and local changes intact. Use a new `BUILD_DIR` for +clean-build verification. Use a fresh Windows `OUTPUT_DIR` for distribution checks, +since staging does not remove extra files. Never edit generated CMakeCache.txt files. +The checked-in MinGW toolchain is the single source of cross-compiler configuration. + +## Current source layout + +`CMakeLists.txt` owns the explicit `SERUMALT_SOURCES` list and shared target setup. +`cmake/Plugin.cmake` owns production formats and MinGW link flags; +`cmake/Harness.cmake` owns the console harness and CTest registration. +The harness compiles the same processor/editor sources independently of the plugin. + +Most C++ files still live directly under `Source/`. Existing implementation +subdirectories are `EffectUnits/`, `GUI/`, `Presets/`, `Resources/`, and `Tests/`. +`FXProcessor`, `PluginEditor`, `Resources`, and `RAVEButton` remain at the source root. +`RAVEButton.{h,cpp}` defines `RaveController`. + +The lowercase `Source/engine`, `modulation`, `params`, `plugin`, and `synth` +directories currently hold guidance, not relocated implementations. Consult their +AGENT.md files when editing the related root-level sources. Consult the matching +AGENT.md when editing an existing implementation subdirectory. +`AUDIT_REPORT.md` is a historical audit and proposed refactor, not a pending +instruction to move files. Update the explicit source list and includes only when +an actual source move is part of the requested task. + +## Style and ownership + +Follow the surrounding file. Existing C++ generally uses four-space indentation, +`#pragma once`, PascalCase types, camelCase members without prefixes, `k`-prefixed +constants, scoped enums, and `noexcept` on cheap getters and DSP paths. Use `Count` +only for enums that need a size or iteration sentinel. Headers include JuceHeader; +implementation files normally include their own header first. + +Prefer value members or `std::unique_ptr` for owned objects and attachments. +`addAndMakeVisible` does not transfer ownership or delete children. Raw allocations +returned by `createEditor` and `createPluginFilter` transfer ownership to JUCE callers. +Guard parameter lookups and invalid state data. Use `jassert` sparingly for programmer +errors and return `bool` from bounded inserts to report failure. + +## Real-time and concurrency requirements + +For new or changed audio paths, allocate buffers during preparation and avoid heap +allocation, blocking locks and I/O in `processBlock` or per-sample DSP. Account for +variable host block sizes without allocating in the callback. Keep GUI updates on +the JUCE message thread. Exchange GUI/audio state through a safe snapshot or bounded +handoff; a timer or a writer-only mutex does not make concurrent access safe. + +These are requirements, not claims that the current code already satisfies them. +The historical audit records lazy wavetable allocation, per-block buffer resizing, +and unsynchronised modulation/LFO edits. When changing those paths, inspect the +current implementation and verify the fix separately. The QA smoke test does not +prove real-time safety or absence of data races. + +## Scope of guidance + +Module guidance refines this file for its related sources. Preserve the root +real-time and ownership requirements if an example conflicts with them. diff --git a/AUDIT_REPORT.md b/AUDIT_REPORT.md new file mode 100644 index 0000000..251f34e --- /dev/null +++ b/AUDIT_REPORT.md @@ -0,0 +1,500 @@ +# SerumAlt codebase audit (Phase 1) + +Historical snapshot, taken before the platform/harness build separation. Build +commands, defaults, target locations and line numbers below describe that earlier +state. Use README.md's Building section and the current CMake files for builds. +The proposed source moves are suggestions, not an approved implementation plan. + +Scope: the SerumAlt C++/JUCE VST3 + Standalone synthesiser. This report records the +implicit standards, anti-patterns, memory choices, error handling, concurrency model and +module boundaries found by reading the real repository files. No source, build or +documentation files were modified. All findings are cited with `path:line` references read +during this task. + +Repository note: at the start of this audit the working tree already had uncommitted +changes to `CMakeLists.txt`, several `Source/` files and several `third_party/JUCE/` files, +plus untracked `build/`, `build_windows/` and `SerumAlt_Windows/` directories. Those +changes predate this audit and are not analysed here except where the current on-disk +content is the source of truth. + +--- + +## 1. Build & test setup + +### Project metadata and targets + +- Project name and version: `project(SerumAlt VERSION 1.0.0 LANGUAGES CXX)` at `CMakeLists.txt:3`. +- C++ standard: C++17, required, extensions off at `CMakeLists.txt:8-10` + (`CMAKE_CXX_STANDARD 17`, `CMAKE_CXX_STANDARD_REQUIRED ON`, `CMAKE_CXX_EXTENSIONS OFF`). +- The plugin target is `SerumAlt`, declared with `juce_add_plugin(...)` at + `CMakeLists.txt:33-46`. Formats are `VST3 AU Standalone` at `CMakeLists.txt:44`. JUCE + generates the concrete per-format targets (`SerumAlt_VST3`, `SerumAlt_Standalone`, and + `SerumAlt_AU` on macOS). There is no separate `PluginProcessor`/`PluginEditor` target; + those are ordinary classes compiled into `SerumAlt`. +- A console test target `SerumAltTest` is added at `CMakeLists.txt:126-140` via + `juce_add_console_app`. It compiles the entire `${SERUMALT_SOURCES}` list plus + `Source/Tests/TestMain.cpp` (`CMakeLists.txt:128`). + +### Source list + +Sources are an explicit list, not a glob. `set(SERUMALT_SOURCES ...)` spans +`CMakeLists.txt:50-87` and is consumed by `target_sources(SerumAlt PRIVATE +${SERUMALT_SOURCES})` at `CMakeLists.txt:89`. The list enumerates every `.cpp` used by the +plugin, including the subdirectory files `Source/EffectUnits/*.cpp`, `Source/GUI/*.cpp` +and `Source/Presets/FactoryPresets.cpp`. + +Two header-only files are intentionally absent from the list because they have no +translation unit: `Source/EffectUnits/Biquad.h` and `Source/GUI/SerumLookAndFeel.h`. + +Consequence for later phases: any file move or rename must update this list, because there +is no glob to pick files up automatically. + +### Custom flags and linkage + +- Compile definitions at `CMakeLists.txt:91-95`: `JUCE_WEB_BROWSER=0`, `JUCE_USE_CURL=0`, + `JUCE_VST3_CAN_REPLACE_VST2=0`. +- Link libraries at `CMakeLists.txt:97-103`: `juce::juce_audio_utils` and `juce::juce_dsp` + (private), `juce::juce_recommended_config_flags` and `juce::juce_recommended_warning_flags` + (public). +- Warning flags at `CMakeLists.txt:142-146`: `/W4` under MSVC, `-Wall -Wextra` otherwise. +- MinGW cross-build static runtime at `CMakeLists.txt:113-118`: when + `CMAKE_CROSSCOMPILING`, `SerumAlt_VST3` gets `-static-libgcc -static-libstdc++ -static` + so the DLL is self-contained. +- VST3 manifest handling at `CMakeLists.txt:23-31`: `SERUMALT_VST3_AUTO_MANIFEST` is `FALSE` + when cross-compiling (because JUCE 7.0.12 builds the manifest helper with the same + toolchain, producing a Windows `.exe` that cannot run on the Linux host). The manifest is + instead injected by `build_windows.sh`. + +### Windows cross-build (build_windows.sh) + +`build_windows.sh` is the canonical Windows build path. Exact steps, in order: + +1. Verify `x86_64-w64-mingw32-g++` exists, else exit with an error (`build_windows.sh:16-21`). +2. Regenerate `mingw64-toolchain.cmake` from a heredoc inside the script + (`build_windows.sh:26-42`). This is identical in content to the checked-in + `mingw64-toolchain.cmake` (see below), so the toolchain file is defined in two places. +3. Configure: `cmake -S . -B build_windows -G Ninja -DCMAKE_BUILD_TYPE=Release + -DCMAKE_TOOLCHAIN_FILE=mingw64-toolchain.cmake -DSERUMALT_BUILD_TESTS=OFF` + (`build_windows.sh:47-50`). Note the test harness is explicitly disabled here. +4. Build: `cmake --build build_windows --target SerumAlt_VST3` (`build_windows.sh:53`). +5. Inject `Contents/Resources/moduleinfo.json` into the `.vst3` bundle by writing the file + directly, because the automatic manifest step was disabled (`build_windows.sh:63-126`). +6. Copy the bundle to `SerumAlt_Windows/SerumAlt.vst3` (`build_windows.sh:129-131`). + +The toolchain (`mingw64-toolchain.cmake:1-15`) sets `CMAKE_SYSTEM_NAME Windows`, +`CMAKE_SYSTEM_PROCESSOR x86_64`, the `x86_64-w64-mingw32-*` compilers/tools, and +`CMAKE_FIND_ROOT_PATH /usr/x86_64-w64-mingw32` with `PROGRAM NEVER` and the other +`..._MODE_*` values `ONLY`. + +### Native Linux build (README only) + +`README.md:76-78` documents a native build: + +``` +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release +cmake --build build +``` + +Outputs listed at `README.md:80-84` are +`build/SerumAlt_artefacts/Release/VST3/SerumAlt.vst3` and the standalone app. + +### Tests + +Tests exist. `Source/Tests/TestMain.cpp` is a console harness that instantiates the +processor, plays notes through every factory preset, checks for NaN/inf and silence, and +toggles RAVE (`TestMain.cpp:37-135`). It returns a non-zero exit code on failure +(`TestMain.cpp:134`). + +Tests are run through the native build, documented at `README.md:96-98`: + +``` +cmake --build build --target SerumAltTest +./build/SerumAltTest_artefacts/Release/SerumAltTest +``` + +The Windows cross-build does not build or run tests: `build_windows.sh:50` passes +`-DSERUMALT_BUILD_TESTS=OFF`. + +### Clean build and test command + +There is no clean step in `build_windows.sh`. The script configures into `build_windows/` +and builds without removing or cleaning it first. A clean rebuild is not scripted; it would +require manually removing `build_windows/` (or invoking Ninja's clean target, which the +script does not do). A combined "clean build + run tests" command does not exist: the +Windows script disables tests, and the README test instructions assume an already-built +native `build/` tree. + +--- + +## 2. Proposed module boundaries + +The codebase already has three real subdirectory boundaries (`EffectUnits/`, `GUI/`, +`Presets/`) plus `Resources/` (assets only) and `Tests/`. The flat files at `Source/` root +mix four distinct concerns, and three root files clearly belong inside existing +subdirectories. The proposal below is based on the include graph actually present, not on +generic synth architecture. + +### Dependency facts that drive the split + +- `Params.h`/`Params.cpp` depend only on `` (`Params.h:3`). It declares the + `ids`, `maps` namespaces, all enums, `ModSource`/`ModTarget`, and the wavetable name + table. It is the shared foundation: nearly every other file includes it. +- `Wavetable`, `Oscillator`, `SubOscillator`, `NoiseOscillator`, `Envelope`, `LFO`, + `Filter`, `FilterBank`, `SynthVoice` depend only on `Params`, `JuceHeader`, and each + other (`Oscillator.h:4-5`, `FilterBank.h:4-5`, `SynthVoice.h:4-12`). +- `ModulationMatrix` and `MacroControls` depend only on `Params` (`ModulationMatrix.h:4`, + `MacroControls.h:4`). +- `RAVEButton.h`/`RaveController` depends only on `Params` (`RAVEButton.h:4`) but manipulates + the APVTS, so it sits between modulation and plugin plumbing. +- `FXProcessor.h` depends only on `Params` (`FXProcessor.h:4`); every file in + `EffectUnits/` includes `../FXProcessor.h` (`Hyper.h:3`, `Chorus.h:3`, etc.), so + `FXProcessor` is the base the effect units build on. +- `Engine` aggregates the DSP: `Engine.h:4-10` includes `Wavetable`, `SynthVoice`, `LFO`, + `FXProcessor`, `ModulationMatrix`, `MacroControls`. +- `PluginProcessor.h` includes `Params`, `Engine`, `RAVEButton` (`PluginProcessor.h:4-6`); + it is the JUCE `AudioProcessor` boundary plus state/preset/RAVE glue. +- `PluginEditor.h` includes `PluginProcessor` and every `GUI/` header + (`PluginEditor.h:4-14`); it is pure GUI. +- `Resources.h` depends on `JuceHeader` plus `Params` for the format helpers + (`Resources.cpp:2`); it holds the theme and SVG strings. The SVG asset files live in + `Source/Resources/`, but the `Resources.{h,cpp}` code lives at root, splitting one + concept across two locations. +- `Presets/FactoryPresets` depends on `Params` and `ModulationMatrix` + (`FactoryPresets.h:4-5`). +- `Tests/TestMain.cpp` depends only on `PluginProcessor` (`TestMain.cpp:4`). + +### Proposed modules + +| Module | Files (current location) | Depends on | +| --- | --- | --- | +| params (foundation) | `Params.{h,cpp}` | JuceHeader only | +| synth (DSP building blocks + voice) | `Wavetable.{h,cpp}`, `Oscillator.{h,cpp}`, `SubOscillator.{h,cpp}`, `NoiseOscillator.{h,cpp}`, `Envelope.{h,cpp}`, `LFO.{h,cpp}`, `Filter.{h,cpp}`, `FilterBank.{h,cpp}`, `SynthVoice.{h,cpp}` | params | +| modulation | `ModulationMatrix.{h,cpp}`, `MacroControls.{h,cpp}`, `RAVEButton.{h,cpp}` | params | +| effects | `FXProcessor.{h,cpp}` plus all of `EffectUnits/` (including `Biquad.h`) | params | +| engine | `Engine.{h,cpp}` | synth, effects, modulation | +| gui | `PluginEditor.{h,cpp}` plus all of `GUI/` (including `SerumLookAndFeel.h`) | params, synth (for wavetable display), plugin | +| resources | `Resources.{h,cpp}` plus `Resources/*.svg` | params | +| presets | `Presets/FactoryPresets.{h,cpp}` | params, modulation | +| plugin (JUCE plumbing) | `PluginProcessor.{h,cpp}` | engine, modulation, presets, params | +| tests | `Tests/TestMain.cpp` | plugin | + +### Where the current layout violates the boundaries + +- `FXProcessor.{h,cpp}` sits at `Source/` root but is the base class and owner of the + `EffectUnits/` classes; `EffectUnits/*.h` include it as `../FXProcessor.h`. It belongs in + `EffectUnits/` (or in a renamed `FX/` directory with the units). +- `PluginEditor.{h,cpp}` sits at root but is pure GUI and includes every `GUI/` header. It + belongs in `GUI/`. +- `Resources.{h,cpp}` holds the theme and SVG strings, while the `.svg` assets live in + `Source/Resources/`. The code and assets should live together under one `resources` + module. +- `RAVEButton.{h,cpp}` is orphaned at root under a filename that does not match its class + (`RaveController`). It is a macro-like boost over the APVTS, so it fits the `modulation` + module, though its APVTS manipulation also makes `plugin` a defensible home. +- The remaining root files (`Params`, `Wavetable`, `Oscillator`, `SubOscillator`, + `NoiseOscillator`, `Envelope`, `LFO`, `Filter`, `FilterBank`, `SynthVoice`, + `ModulationMatrix`, `MacroControls`, `Engine`, `PluginProcessor`) are the synthesis core + and JUCE boundary. Grouping them into `synth/` and `modulation/` subdirectories would + match the dependencies above, but the flat layout is at least internally consistent: + they are the only files that legitimately belong at root today. + +--- + +## 3. Memory management standards + +### Dominant idiom: value semantics plus `std::unique_ptr` for ownership + +- Owned sub-objects are held by value, not by pointer: `juce::AudioProcessorValueTreeState + parameters; Engine engine; RaveController rave;` at `PluginProcessor.h:56-58`; the + `Engine` holds `std::array voices`, `std::array + lfos`, and `WavetableLibrary`, `FXProcessor`, `ModulationMatrix`, `MacroControls` by value + at `Engine.h:45-50`. +- Owned polymorphic objects use `std::unique_ptr`: `std::array, 9> + units` at `FXProcessor.h:50`, populated with `std::make_unique` at `FXProcessor.cpp:15-26`. +- GUI attachments are owned by `std::unique_ptr`: `std::vector>` and `...ComboBoxAttachment` at `PluginEditor.h:103-104`, filled at + `PluginEditor.cpp:169-170` and `183-184`. +- Transient GUI objects are `std::unique_ptr`: `std::unique_ptr valuePopup` + at `Knob.h:38`, created at `Knob.cpp:61` and released with `valuePopup.reset()` at + `Knob.cpp:83`. +- JUCE-managed objects are returned as raw `new` pointers because JUCE takes ownership + through `addAndMakeVisible` or the plugin API: `createEditor()` returns + `new PluginEditor (*this)` at `PluginProcessor.cpp:195`; the plugin entry point returns + `new serum::SerumAltAudioProcessor()` at `PluginProcessor.cpp:380`; GUI children are + created with `new` at `PluginEditor.cpp:166,178,193,602,606`. These raw pointers are never + deleted by the code, which is correct only because JUCE's component tree and plugin host + own them. +- Non-owning cross-object references are raw pointers or references: `juce::RangedAudioParameter* + param` at `ToggleButton.h:38`, `const WavetableLibrary* wtLib` at `WaveformDisplay.h:24`, + and the `RenderContext` raw `const` pointers to the library/matrix/macros at + `SynthVoice.h:24,32-33`. + +### Containers and buffers + +- `std::vector` is the standard delay line / table buffer: `Wavetable.h:24` + (`frames`), `Filter.h:39` (`combLine`), `Chorus.h:21`, `Delay.h:21`, `Reverb.h:22-29`. +- `std::array` is used for fixed-size DSP state: `Oscillator.h:62` + (`std::array`), `Filter.h:36,44`, `Reverb.h:22-27`. +- `juce::AudioBuffer` is used for block buffers: `Engine.h:59` (`mixBuffer`), + `SynthVoice.h:95` (`scratch`), `FXProcessor.h:51` (`dry`, `wet`). These are sized once in + `prepare()` (for example `Engine.cpp:68`, `SynthVoice.cpp:23`, `FXProcessor.cpp:34-35`) + and cleared per block. + +### Realtime vs non-realtime allocation + +The code intends to allocate only at prepare time, but it does not fully achieve this: + +- Prepare-time allocation is the norm: delay lines and buffers are sized in `prepare()` + (`Filter.cpp:24`, `Chorus.cpp:9-11`, `Reverb.cpp:17-33`, `Engine.cpp:68`). +- Violation 1: `Engine::processBlock` calls `mixBuffer.setSize (2, n, false, false, true)` + on every block at `Engine.cpp:211`. `AudioBuffer::setSize` is a no-op when the size is + unchanged, but it is still a per-callback reconfiguration call on the audio thread. +- Violation 2: wavetables are built lazily. `WavetableLibrary::getTable` builds the table + on first access (`Wavetable.cpp:126-132`) and `getTable` is reached from the audio path at + `SynthVoice.cpp:177-178`. The `prebuild()` method exists at `Wavetable.cpp:120-124` but is + never called (it is not referenced anywhere else, and `Engine::prepare` at + `Engine.cpp:58-70` does not call it). The first rendered block for a wavetable therefore + allocates `256 x 2048` floats plus harmonic work buffers on the audio thread. + +--- + +## 4. Error handling + +There is no exception handling and no user-facing alerting. Failures are handled with a +mix of asserts, null guards, and return codes. + +- `jassert` is used exactly once in the entire codebase: `jassert (freqHz > 0.0)` at + `Oscillator.cpp:20`. There are no `throw`, `try` or `catch` statements (verified by grep). +- Null guards with silent fallback are the dominant pattern. The APVTS helper `v()` returns + `0.0f` when a parameter is missing: `Engine.cpp:8-13`. State restore silently returns on + malformed input: `PluginProcessor.cpp:313-315` returns when the XML is null and + `PluginProcessor.cpp:318-319` returns when the `ValueTree` is invalid. Preset loads skip + missing parameters with `if (auto* param = ...)` at `PluginProcessor.cpp:248-250`. +- Return codes carry failure for the two bounded collections: + `ModulationMatrix::addConnection` returns `false` when the target is `None` or the + connection limit is reached (`ModulationMatrix.cpp:6-12`), and + `MacroControls::addAssignment` returns `false` similarly (`MacroControls.cpp:6-13`). + These return values are ignored by their callers (`PluginEditor.cpp:536`, `PluginEditor.cpp:687`, + `PluginProcessor.cpp:254`, `PluginProcessor.cpp:258`). +- Out-of-range lookups degrade to empty/default values: `getProgramName` returns an empty + string for a bad index (`PluginProcessor.cpp:218-224`); the enum-string converters fall + through to `return {}` (`Params.cpp:27,44,91`). +- There are no `juce::AlertWindow`, no `MessageManager` calls, and no logging in the plugin + itself. The only reporting is the test harness printing to `std::cout` and returning an + exit code (`TestMain.cpp:73,103,129,132-134`). + +Net style: programmer errors use `jassert` (very sparingly), runtime/state errors use null +guards or default values, and bounded-insertion failures use `bool` return codes that +callers currently ignore. + +--- + +## 5. Concurrency + +The codebase is single-threaded and has no explicit synchronisation. A grep for threads, +mutexes, locks, atomics, `CriticalSection`, `SpinLock` and `ScopedLock` returns nothing +outside JUCE's own headers. + +- The only non-audio execution context is the JUCE message thread, driven by a `juce::Timer`: + `PluginEditor` inherits `juce::Timer` (`PluginEditor.h:24`), starts it at 30 Hz + (`PluginEditor.cpp:135`), and refreshes visuals in `timerCallback` (`PluginEditor.cpp:875-880`). +- The audio thread is `SerumAltAudioProcessor::processBlock` (`PluginProcessor.cpp:187-191`) + delegating to `Engine::processBlock` (`Engine.cpp:176-318`). It reads the playhead tempo at + `Engine.cpp:184-187`. + +The significant issue is unsynchronised sharing between the message thread and the audio +thread: + +- The GUI mutates `ModulationMatrix::connections` via `addConnection`/`removeConnection`/ + `clear` (`PluginEditor.cpp:536,549,559`) while the audio thread iterates the same vector in + `SynthVoice::render` at `SynthVoice.cpp:114`. `connections` is a public `std::vector` + (`ModulationMatrix.h:30`). +- The GUI mutates `MacroControls::assignments` (`PluginEditor.cpp:687,699`) while the audio + thread iterates it at `SynthVoice.cpp:125`. `assignments` is a public `std::array` of + vectors (`MacroControls.h:28`). +- The GUI writes LFO shape data through `engine.setLfoShapeData` (`PluginEditor.cpp:483-486`) + which calls `LFO::setShapeData` (`Engine.cpp:84-88`, `LFO.cpp:51-59`), while the audio + thread reads the same `shapeBuffer` in `LFO::shapeValue` (`LFO.cpp:82-91`). + +None of these shared collections are locked or double-buffered. This is a real data race +today: resizing `connections` while the audio thread iterates it is undefined behaviour. + +The processor header acknowledges GUI-thread access but does not address the race: the +comment "Public DSP state (read/write from the GUI thread)" at `PluginProcessor.h:55` only +describes intent. + +--- + +## 6. Naming & style conventions + +- Namespace: every file is inside `namespace serum` (`PluginProcessor.h:8`, `Engine.h:12`, + etc.). File-local helpers use an anonymous namespace (`Engine.cpp:6`, `SynthVoice.cpp:6`, + `PluginEditor.cpp:6`, `FactoryPresets.cpp:6`). +- Include guard: `#pragma once` in every header (for example `PluginProcessor.h:1`, + `Params.h:1`, `Biquad.h:1`). No `#ifndef` guards exist. +- Include order: `` first, then local headers (`PluginProcessor.h:3-6`). + Subdirectory files include root files with a `../` relative path (`Hyper.h:3`, + `Knob.h:5`, `FactoryPresets.h:4`). +- File/header pairs: one primary class per `.h`/`.cpp` pair, with the pair named after the + class (`Oscillator.h`/`Oscillator.cpp`, `FilterBank.h`/`FilterBank.cpp`). Header-only + helpers exist where they have no state logic worth a TU (`Biquad.h`, + `SerumLookAndFeel.h`). +- Class and struct names: PascalCase, no prefix (`SynthVoice`, `RenderContext`, + `FilterBankParams`, `ModConnection`). +- Members: no `m_` prefix and no trailing underscore. Plain camelCase fields such as + `note`, `velocity`, `baseFreq`, `active`, `released`, `noteId` at `SynthVoice.h:83-88`, + and `sr`, `blockSize`, `bpm`, `pitchBend` at `Engine.h:52-57`. +- Constants: `k` prefix for compile-time constants (`kNumVoices`, `kNumLfos` at + `Params.h:221-227`; `kFrames`, `kTableSize` at `Wavetable.h:18-19`; `kMaxConnections` at + `ModulationMatrix.h:28`; `kMaxAssignments` at `MacroControls.h:26`). Local constant arrays + in anonymous namespaces also use `k` (`kEnvAttack` at `Engine.cpp:30`, `kFxType` at + `PluginEditor.cpp:97`). +- Enums: `enum class` (scoped) with PascalCase enumerators (`WarpMode::BendPlus`, + `ModSource::Lfo1`, `ModTarget::Filter1Cutoff`) at `Params.h:213-219` and `Params.h:232-256`. +- Namespaces for related constants: `ids::` for parameter ID strings (`Params.h:18`), + `theme::` for colours (`Resources.h:12`), `maps::` for unit mappings (`Params.h:309`). +- Method naming: camelCase with a leading verb (`noteOn`, `processAdd`, `getValue`). + JUCE overrides keep JUCE's spelling (`processBlock`, `prepareToPlay`, `resized`, + `timerCallback`). +- `noexcept` is applied consistently to the per-sample DSP hot paths (`Oscillator.cpp:93`, + `Filter.cpp:181,236`, `Wavetable.cpp:81`, `Envelope.cpp:54`, `SynthVoice.cpp:72`, + `LFO.cpp:98`) and to cheap getters (`SynthVoice.h:71-74`). +- Formatting: two-space indentation; a space before the opening parenthesis in function + calls and definitions (`f (x)` at `PluginProcessor.cpp:12`, `juce::jlimit (...)`; + `if (...)`, `for (...)`). Aligned member initialiser lists (`PluginEditor.cpp:113-115`). +- Section dividers: `// ===...` banners at class/file tops and `// ---...` dividers within + files (`PluginProcessor.cpp:17,198,261`, `PluginEditor.cpp:161`). +- Casting idiom: `(size_t)` casts on int loop indices when indexing `std::vector`/`std::array` + (`Engine.cpp:220`, `SynthVoice.cpp:79`, `FactoryPresets.cpp` throughout), and `(int)` + casts on `size()` results. + +--- + +## 7. Anti-pattern catalog + +Each entry lists the location, the problem, and the standard that a later AGENT.md phase +should encode. + +1. Parameter ID tables duplicated three times. + - `Params.h:18-208` (canonical `ids::`), `Engine.cpp:30-55` (`kEnv*`, `kLfo*`, `kFx*`), + `PluginEditor.cpp:97-108,433-437,458-464,820-825,833` (the same arrays again), plus the + display names and defaults in `PluginProcessor.cpp:31-171`. + - Standard: one source of truth in `Params.h` (or a single generated table); engine and + editor must reference it rather than re-declaring ID arrays. + +2. Wavetable library allocates on the audio thread. + - `WavetableLibrary::getTable` builds lazily (`Wavetable.cpp:126-132`), reached from + `SynthVoice::render` (`SynthVoice.cpp:177-178`); `prebuild()` is defined + (`Wavetable.cpp:120-124`) but never called. + - Standard: call `prebuild()` (or build all tables) in `prepareToPlay`, never in the + render path. + +3. Per-block buffer resize in the audio callback. + - `Engine::processBlock` calls `mixBuffer.setSize(...)` every block at `Engine.cpp:211`. + - Standard: size the buffer once in `prepare()` and only `clear()` per block. + +4. Unsynchronised GUI/audio sharing of container state. + - `ModulationMatrix::connections` is public (`ModulationMatrix.h:30`), written by the GUI + (`PluginEditor.cpp:536,549,559`) and read on the audio thread (`SynthVoice.cpp:114`). + `MacroControls::assignments` is public (`MacroControls.h:28`), written + (`PluginEditor.cpp:687,699`) and read (`SynthVoice.cpp:125`). LFO `shapeBuffer` is + written via `setShapeData` (`LFO.cpp:51-59`) and read (`LFO.cpp:82-91`). + - Standard: either protect these collections with a lock, or use atomic/double-buffered + exchange between the message and audio threads. + +5. Class name does not match its file. + - `Source/RAVEButton.h` declares `RaveController` (`RAVEButton.h:14`), and + `RAVEButton.cpp` implements it. + - Standard: file pair name matches the class (`RaveController.h/.cpp`), or the class is + renamed to match the file. + +6. Dead code in `SubOscillator::noteOn`. + - `SubOscillator.cpp:9-12` computes `mult` and then discards it with `(void) freqHz; + (void) mult;` because the octave shift is already baked into the frequency by the voice. + - Standard: remove the unused parameters or make the signature reflect what is actually + used (the octave shift is applied at `SynthVoice.cpp:179`). + +7. No-op override. + - `changeProgramName` is an empty override at `PluginProcessor.cpp:226-228`. + - Standard: implement it or do not override it. + +8. Ignored prepare-time parameter. + - `Filter::prepare` accepts `maxBlockSize` and discards it with `(void) maxBlockSize;` + (`Filter.cpp:21-29`); effect units name the parameter `int` but never use it + (`Hyper.cpp:6`, `Chorus.cpp:6`, `Flanger.cpp:6`, etc.). + - Standard: keep the `FXUnit` interface uniform but do not add unused parameters to + concrete `prepare()` implementations. + +9. Duplicated envelope curve constants. + - The attack/decay shape mapping `0.3 + curve * 2.7` / `3.0 - curve * 2.7` appears in + `Envelope.cpp:44-45` and again in the display `EnvelopeDisplay.cpp:19-20`. + - Standard: expose one shared helper so the preview cannot drift from the DSP. + +10. Filter response reimplemented for display. + - `FilterDisplay::magnitude` (`FilterDisplay.cpp:6-36`) hardcodes a parallel set of + transfer-function approximations that duplicate the real DSP in `Filter.cpp` + (`Filter.cpp:87-234`). + - Standard: keep the display response derived from one documented source, or mark it as + an approximation with a comment that the two are not the same code. + +11. Inconsistent pi constants. + - `kTwoPi` is defined at `Oscillator.h:67` and `SubOscillator.h:26`; `twoPi` at + `Wavetable.cpp:21`; `kPi` at `Wavetable.cpp:156` and `Filter.cpp:8`; raw literals + `6.28318530717958647692` appear at `Chorus.cpp:40-41`, `Flanger.cpp:38-39`, + `Phaser.cpp:30-31`, and `6.2831853f` at `LFO.cpp:67` and `LFODisplay.cpp:19`. + - Standard: use `juce::MathConstants::pi` / `twoPi` everywhere. + +12. Public mutable state exposes internals. + - `PluginProcessor.h:56-58` exposes `parameters`, `engine` and `rave` as public fields; + `ModulationMatrix.h:30` and `MacroControls.h:28` expose their collections; + `Engine.h:36-38` returns non-const references from `getMatrix()`, `getMacros()` and + `getWavetables()`. + - Standard: return `const&` from accessors and route mutation through member functions, + except where a public field is an explicit, documented design choice. + +13. Unchecked raw parameter dereference. + - `PluginEditor.cpp:794` dereferences `getRawParameterValue(id)->load()` without a null + check, while the equivalent helper in `Engine.cpp:8-13` guards the pointer. + - Standard: always guard `getRawParameterValue`/`getParameter` results before use. + +14. Mixed ownership for GUI children. + - `makeKnob`/`makeCombo`/`makeToggle` return raw pointers to `new`-allocated children + that JUCE owns via `addAndMakeVisible` (`PluginEditor.cpp:162-199`), and `fxUp`/ + `fxDown` use raw `new` at `PluginEditor.cpp:602,606`. This is correct only because of + JUCE's component-tree ownership, but it is easy to misread as a leak. + - Standard: document that JUCE-owned children are raw pointers and never deleted, and + keep `std::unique_ptr` for everything the plugin owns directly. + +--- + +## 8. Hard constraints worth codifying + +Only rules that the current code and build actually support are listed. + +1. Do not allocate or resize containers in the audio callback. Evidence that this is the + intent: all DSP state is sized in `prepare()` (`Engine.cpp:68`, `SynthVoice.cpp:23`, + `FXProcessor.cpp:34-35`) and the code uses pre-sized `juce::AudioBuffer` and + `std::array`. The current violations (`Engine.cpp:211` and the lazy wavetable build) are + exceptions to fix, not the rule. +2. All audio parameters are normalised to 0..1 in the APVTS, and physical units are mapped + only through `maps::` (`Params.h:309-342`). DSP must consume the normalised values, not + physical units. +3. Parameter IDs live in the `ids` namespace in `Params.h` (`Params.h:18-208`). Engine, + editor and preset code must reference `ids::`, not re-declare string literals. (This is + the stated intent at `Params.h:5-9`; the current duplication in `Engine.cpp` and + `PluginEditor.cpp` is a violation to eliminate.) +4. The CMake source list is explicit (`CMakeLists.txt:50-87`), not a glob. Every file move + or rename must update `SERUMALT_SOURCES`. +5. All code is inside `namespace serum`, and every header uses `#pragma once` (verified + across all headers). +6. No exceptions and no user-facing alert dialogs. Use `jassert` for programmer errors, + null guards for runtime lookups, and `bool` returns for bounded inserts (see + `Oscillator.cpp:20`, `Engine.cpp:8-13`, `ModulationMatrix.cpp:6-12`). +7. When cross-compiling for Windows, keep the VST3 manifest step disabled and let + `build_windows.sh` inject `moduleinfo.json`, because JUCE's manifest helper must not be + built as a Windows executable on a Linux host (`CMakeLists.txt:23-31`, + `build_windows.sh:55-126`). Keep the injected metadata in sync with + `PLUGIN_CODE`/`PLUGIN_MANUFACTURER_CODE`/`VERSION` (`build_windows.sh:60-62`). +8. The project is single-threaded by design. Any future thread, mutex or atomic must come + with an explicit plan for the GUI-to-audio boundary, because the current matrix/macro/LFO + state is shared without synchronisation (section 5). diff --git a/Source/EffectUnits/AGENT.md b/Source/EffectUnits/AGENT.md new file mode 100644 index 0000000..553737d --- /dev/null +++ b/Source/EffectUnits/AGENT.md @@ -0,0 +1,53 @@ +# effects module + +## Owned files + +- Related rack implementation: `../FXProcessor.h`, `../FXProcessor.cpp` +- Biquad.h +- Hyper.h, Hyper.cpp +- Chorus.h, Chorus.cpp +- Flanger.h, Flanger.cpp +- Phaser.h, Phaser.cpp +- Distortion.h, Distortion.cpp +- EQ.h, EQ.cpp +- Compressor.h, Compressor.cpp +- Delay.h, Delay.cpp +- Reverb.h, Reverb.cpp + +## Rules + +- Implement the `FXUnit` interface with `prepare`, `reset` and `process (float* l, float* r, int numSamples, const float p[4])`. +- Process full-wet in place; FXProcessor handles the dry/wet mix. +- Document what p[0] through p[3] mean for each unit in the header comment. +- Use `std::vector` for delay lines and size them in `prepare()`. +- Use `juce::MathConstants::pi` and `twoPi` in delay LFOs, not raw literals. +- Never allocate or resize a delay line inside `process`. +- Guard empty delay lines with an early `if (len < 4) return;`. + +## IF-THEN + +- IF a unit needs a delay line THEN allocate it in `prepare()` at the full required length and only read and write it in `process`. +- Include the existing rack interface via `../FXProcessor.h`; no source relocation is implied by this guidance. + +## Examples + +```cpp +// BAD: reallocates a delay line on the audio thread +void process (float* l, float* r, int n, const float p[4]) override +{ + std::vector delay ((size_t) (sr * 0.05), 0.0f); + ... +} +``` + +```cpp +// GOOD: sized once at prepare time, reused per block +void prepare (double sampleRate, int) override +{ + sr = sampleRate; + delayL.assign ((size_t) (sr * 0.05), 0.0f); + delayR.assign ((size_t) (sr * 0.05), 0.0f); +} +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/GUI/AGENT.md b/Source/GUI/AGENT.md new file mode 100644 index 0000000..1e6972d --- /dev/null +++ b/Source/GUI/AGENT.md @@ -0,0 +1,47 @@ +# gui module + +## Owned files + +- Related editor implementation: `../PluginEditor.h`, `../PluginEditor.cpp` +- SerumLookAndFeel.h +- Knob.h, Knob.cpp +- Slider.h, Slider.cpp +- ToggleButton.h, ToggleButton.cpp +- Display.h, Display.cpp +- Panel.h, Panel.cpp +- WaveformDisplay.h, WaveformDisplay.cpp +- FilterDisplay.h, FilterDisplay.cpp +- EnvelopeDisplay.h, EnvelopeDisplay.cpp +- LFODisplay.h, LFODisplay.cpp + +## Rules + +- Draw all controls through `SerumLookAndFeel`. +- Use `theme::` colours from Resources.h instead of hardcoded colour values. +- Own GUI attachments with `std::unique_ptr`. +- Own component children by value or with `std::unique_ptr`; `addAndMakeVisible` only attaches and shows them, and does not manage their lifetime. +- Refresh visuals from a `juce::Timer` callback on the message thread, not from the audio thread. +- Keep layout constants in the `layout` namespace and derive every position from them. +- Use `std::function` formatters for value readouts. +- Reference `ids::` for APVTS parameter IDs. + +## IF-THEN + +- IF a control reads a DSP value THEN refresh it on the message thread from a thread-safe snapshot or atomic value. A `timerCallback` alone does not make a shared DSP read safe. +- IF a raw parameter pointer is dereferenced THEN null-guard it first. + +## Examples + +```cpp +// BAD: unchecked raw parameter dereference +const float v = processor.parameters.getRawParameterValue (ids::f1Cutoff)->load(); +``` + +```cpp +// GOOD: null guard with fallback +const float v = processor.parameters.getRawParameterValue (ids::f1Cutoff) + ? processor.parameters.getRawParameterValue (ids::f1Cutoff)->load() + : 0.0f; +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/Presets/AGENT.md b/Source/Presets/AGENT.md new file mode 100644 index 0000000..52e425c --- /dev/null +++ b/Source/Presets/AGENT.md @@ -0,0 +1,33 @@ +# presets module + +## Owned files + +- FactoryPresets.h +- FactoryPresets.cpp + +## Rules + +- Build presets as sparse maps; unset parameters keep their APVTS defaults. +- Use `ids::` for every parameter ID, never raw strings. +- Use named helpers (coarse, unison, slope, fx, lfoShape, mod) to build normalized values. +- Return the preset list as `const std::vector&` from a static local. +- Reference `ModConnection` and macro assignment structs instead of ad hoc tuples. + +## IF-THEN + +- IF a preset omits a parameter THEN leave it at its default; do not write an explicit zero. +- IF a preset sets a physical value THEN encode it through the same `maps::` helpers the DSP uses. + +## Examples + +```cpp +// BAD: raw string ID and an opaque normalized number +pr.params = { { "oscAUnison", 0.4f } }; +``` + +```cpp +// GOOD: ids:: and a named helper +pr.params = { p (ids::oscAUnison, unison (7)) }; +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/Resources/AGENT.md b/Source/Resources/AGENT.md new file mode 100644 index 0000000..4a4e474 --- /dev/null +++ b/Source/Resources/AGENT.md @@ -0,0 +1,34 @@ +# resources module + +## Owned files + +- Related embedded assets and helpers: `../Resources.h`, `../Resources.cpp` +- logo.svg +- background.svg + +## Rules + +- Keep every theme colour in `namespace theme` in Resources.h. +- Embed SVG assets as raw string literals. +- Keep formatting helpers (formatPercent, formatHz, formatSeconds, formatSemis, formatCents, formatPan, formatInt) here. +- Use `maps::` from Params for unit conversions in formatters. +- Never hardcode a theme colour in a widget; reference `theme::`. + +## IF-THEN + +- IF a widget needs a colour THEN reference a `theme::` entry rather than a literal Colour. +- IF an SVG asset ships under Source/Resources THEN keep its embedded string in Resources.cpp consistent with the file. + +## Examples + +```cpp +// BAD: hardcoded colour in a widget +g.setColour (juce::Colour (0xff4fc3f7)); +``` + +```cpp +// GOOD: theme is the single source +g.setColour (theme::accent); +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/Tests/AGENT.md b/Source/Tests/AGENT.md new file mode 100644 index 0000000..53ebe48 --- /dev/null +++ b/Source/Tests/AGENT.md @@ -0,0 +1,40 @@ +# tests module + +## Owned files + +- TestMain.cpp + +## Rules + +- Keep this a headless console harness that drives the real SerumAltAudioProcessor. +- Check rendered blocks for NaN and inf. Check non-silence over the full preset render, not each block, because note release can legitimately produce silent blocks. +- Print PASS or FAIL per preset to `std::cout`. +- Return a non-zero exit code when any check fails. +- Never show a JUCE alert window. +- Do not add GUI interaction beyond `juce::ScopedJuceInitialiser_GUI`. + +## IF-THEN + +- IF a check fails THEN increment the failure count and print the preset name. +- Build and run natively from the repository root with `./build_harness.sh --run`. Building the `SerumAltTest` target alone does not run checks. +- The harness-only configuration sets `SERUMALT_BUILD_PLUGIN=OFF` and `SERUMALT_BUILD_TESTS=ON`. Production scripts disable the harness. +- The harness links the real processor and editor sources but creates no plugin bundles. Keep that build separation when adding checks. + +## Examples + +```cpp +// BAD: a failing check does not fail the run +bool ok = ! hasNaN (buffer) && peak (buffer) > 0.001f; // result ignored +return 0; +``` + +```cpp +// GOOD: accumulate failures and exit non-zero +int failures = 0; +const bool ok = ! hasNaN (buffer) && peak (buffer) > 0.001f; +if (! ok) ++failures; +... +return failures == 0 ? 0 : 1; +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/engine/AGENT.md b/Source/engine/AGENT.md new file mode 100644 index 0000000..234846c --- /dev/null +++ b/Source/engine/AGENT.md @@ -0,0 +1,48 @@ +# engine module + +## Related source files + +This directory contains guidance; the implementation remains in `../Engine.h` +and `../Engine.cpp`. The preparation rules below are requirements for changes; +current buffer resizing and lazy wavetable issues are recorded in the root AGENT.md. + +## Rules + +- Own the voice pool, LFOs, wavetable library, FX rack, matrix and macros by value. +- Read every APVTS value through the null-guarding `v()` helper. +- Build the `RenderContext` once per block and pass it to voices as `const&`. +- Size `mixBuffer` once in `prepare()` and only `clear()` it per block. +- Prebuild wavetables in `prepare()` before the first render. +- Clamp the final output with the soft limiter. +- Never call `setSize` on any buffer inside `processBlock`. + +## IF-THEN + +- IF a per-block buffer is needed THEN allocate it in `prepare()` and clear it per block. +- IF a module needs the matrix, macros or wavetables THEN go through Engine accessors rather than reaching into another module. + +## Examples + +```cpp +// BAD: per-callback buffer reconfiguration on the audio thread +void processBlock (...) +{ + mixBuffer.setSize (2, n, false, false, true); + ... +} +``` + +```cpp +// GOOD: size once, clear per block +void prepare (double sr, int blockSize) +{ + mixBuffer.setSize (2, blockSize, false, false, true); +} +void processBlock (...) +{ + mixBuffer.clear(); + ... +} +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/modulation/AGENT.md b/Source/modulation/AGENT.md new file mode 100644 index 0000000..54cad59 --- /dev/null +++ b/Source/modulation/AGENT.md @@ -0,0 +1,34 @@ +# modulation module + +## Related source files + +This directory contains guidance; implementations remain in the parent `Source/` +directory: + +- `../ModulationMatrix.h`, `../ModulationMatrix.cpp` +- `../MacroControls.h`, `../MacroControls.cpp` +- `../RAVEButton.h`, `../RAVEButton.cpp`, which define `RaveController` + +## Rules + +- Use `bool` return codes for bounded inserts (`addConnection`, `addAssignment`). +- Require a cap on connection and assignment counts (`kMaxConnections`, `kMaxAssignments`). +- Serialise and restore state through `juce::ValueTree` so presets survive save and load. +- Never accept a `ModTarget::None` connection; reject it and return false. +- Use `ids::` for every APVTS parameter reference in RaveController. +- Keep the existing RAVEButton filenames unless a rename is explicitly in scope. + +## GUI/audio state + +The current public connection and assignment vectors are shared mutable state. +GUI writes can race with audio-thread iteration. A mutex taken only by the writer +does not protect an unlocked reader, and taking a blocking mutex in the audio +callback conflicts with the real-time requirements. + +When fixing this path, publish a stable snapshot or use a bounded thread-safe +handoff. Keep allocation and snapshot reclamation off the audio thread. Validate +reader and writer lifetimes together, including preset and state restore. + +If `addConnection` or `addAssignment` returns false, handle the failed insert. + +This file refines the repository-root AGENT.md for the related sources. diff --git a/Source/params/AGENT.md b/Source/params/AGENT.md new file mode 100644 index 0000000..3991f67 --- /dev/null +++ b/Source/params/AGENT.md @@ -0,0 +1,39 @@ +# params module + +## Related source files + +This directory contains guidance; the implementation remains in `../Params.h` +and `../Params.cpp`. + +## Rules + +- Use the `ids::` namespace for every APVTS parameter ID string. +- Require one `ids::` entry per audio parameter, and one entry only. +- Use `enum class` for every enum. Give each a trailing `Count` enumerator. +- Use `inline constexpr` for parameter ID strings and `k`-prefixed integer constants. +- Keep all 0..1 to physical unit conversions in the `maps::` namespace. +- Keep the wavetable name table (`kWavetableNames`) in sync with `kNumWavetables`. +- Return an empty `juce::String` from enum-to-string switches for unknown values. +- Never re-declare a parameter ID string literal outside Params.h. +- Never add a physical unit mapping anywhere but `maps::`. + +## IF-THEN + +- IF you add an audio parameter THEN add its ID to `ids::` and, when it has physical units, add its mapping to `maps::` before any engine or GUI code reads it. +- IF a switch converts an enum to a string THEN include a default case that returns an empty string. + +## Examples + +```cpp +// BAD: re-declares the ID and drifts from Params.h +if (auto* p = apvts.getParameter ("f1Cutoff")) + return p->getValue(); +``` + +```cpp +// GOOD: one source of truth +if (auto* p = apvts.getParameter (ids::f1Cutoff)) + return p->getValue(); +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/plugin/AGENT.md b/Source/plugin/AGENT.md new file mode 100644 index 0000000..a0a10af --- /dev/null +++ b/Source/plugin/AGENT.md @@ -0,0 +1,41 @@ +# plugin module + +## Related source files + +This directory contains guidance; the implementation remains in +`../PluginProcessor.h` and `../PluginProcessor.cpp`. + +## Rules + +- Keep this the JUCE `AudioProcessor` boundary: APVTS, engine, RAVE, presets and state. +- Declare the full parameter layout in `createParameterLayout()`. +- Use `juce::ScopedNoDenormals` in `processBlock`. +- Guard `getRawParameterValue` and `getParameter` results before dereferencing them. +- Return silently on null or invalid state XML and invalid ValueTrees. +- Use raw `new` only for JUCE-owned objects (`createEditor`, `createPluginFilter`). +- Prefer const accessors over the public `parameters`, `engine` and `rave` fields for new code. + +## IF-THEN + +- IF state XML is null or invalid THEN return without touching the APVTS. +- IF a parameter is missing from a preset load THEN skip it with an `if (auto* param = ...)` guard. + +## Examples + +```cpp +// BAD: dereferences a possibly null XML result +auto xml = getXmlFromBinary (data, size); +auto state = juce::ValueTree::fromXml (*xml); +``` + +```cpp +// GOOD: null guard, silent return +std::unique_ptr xml (getXmlFromBinary (data, size)); +if (xml == nullptr) + return; +juce::ValueTree state = juce::ValueTree::fromXml (*xml); +if (! state.isValid()) + return; +``` + +This file overrides /AGENT.md where they conflict. diff --git a/Source/synth/AGENT.md b/Source/synth/AGENT.md new file mode 100644 index 0000000..1876b9d --- /dev/null +++ b/Source/synth/AGENT.md @@ -0,0 +1,58 @@ +# synth module + +## Related source files + +This directory contains guidance. All files listed below remain in the parent +`Source/` directory. The preparation rules below are requirements for changes, +not a claim that lazy wavetable allocation has already been fixed. + +- Wavetable.h, Wavetable.cpp +- Oscillator.h, Oscillator.cpp +- SubOscillator.h, SubOscillator.cpp +- NoiseOscillator.h, NoiseOscillator.cpp +- Envelope.h, Envelope.cpp +- LFO.h, LFO.cpp +- Filter.h, Filter.cpp +- FilterBank.h, FilterBank.cpp +- SynthVoice.h, SynthVoice.cpp + +## Rules + +- Use `noexcept` on every per-sample DSP path and cheap getter. +- Use `std::array` for fixed-size state and `std::vector` for delay lines and table buffers. +- Size every buffer and delay line in `prepare()`, never in `process` or `render`. +- Clamp filter and envelope state so extreme settings cannot produce NaN or inf. +- Use `juce::MathConstants::pi` and `twoPi` instead of raw pi literals. +- Use `(size_t)` casts when indexing containers with int loop variables. +- Pass global per-block state into voices as a `const RenderContext&`. +- Never allocate memory in the audio path. + +## IF-THEN + +- IF a wavetable is read in the render path THEN build all tables in `prepare()` by calling `WavetableLibrary::prebuild()` before the first block. +- IF a parameter is read every block THEN snapshot it into a struct (`OscParams`, `RenderContext`) instead of reading the APVTS per sample. + +## Examples + +```cpp +// BAD: lazy table build reached from the render path allocates on the audio thread +const Wavetable& wt = library.getTable (wave); +``` + +```cpp +// GOOD: build once at prepare time, then only read const references +library.prebuild(); // in prepareToPlay +const Wavetable& wt = library.getTable (wave); // lookup, no allocation +``` + +```cpp +// BAD: raw pi literal duplicated across files +const double step = 6.28318530717958647692 * freq / sr; +``` + +```cpp +// GOOD: single source +const double step = juce::MathConstants::twoPi * freq / sr; +``` + +This file overrides /AGENT.md where they conflict.