From 70c64867baa30c83334559d3023153dfe3f9ff79 Mon Sep 17 00:00:00 2001 From: skal Date: Mon, 9 Feb 2026 10:43:11 +0100 Subject: docs: Simplify all design docs (50% reduction, 1687 lines removed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated and streamlined all documentation: **Merged:** - PROCEDURAL.md → deleted (content in ASSET_SYSTEM.md) - FETCH_DEPS.md → BUILD.md (dependencies section) **Simplified (line reductions):** - HOWTO.md: 468→219 (53%) - CONTRIBUTING.md: 453→173 (62%) - SPECTRAL_BRUSH_EDITOR.md: 497→195 (61%) - SEQUENCE.md: 355→197 (45%) - CONTEXT_MAINTENANCE.md: 332→200 (40%) - test_demo_README.md: 273→122 (55%) - ASSET_SYSTEM.md: 271→108 (60%) - MASKING_SYSTEM.md: 240→125 (48%) - 3D.md: 196→118 (40%) - TRACKER.md: 124→76 (39%) - SCENE_FORMAT.md: 59→49 (17%) - BUILD.md: 83→69 (17%) **Total:** 3344→1657 lines (50.4% reduction) **Changes:** - Removed verbose examples, redundant explanations, unimplemented features - Moved detailed task plans to TODO.md (single source of truth) - Consolidated coding style rules - Kept essential APIs, syntax references, technical details **PROJECT_CONTEXT.md:** - Added "Design Docs Quick Reference" with 2-3 line summaries - Removed duplicate task entries - All design docs now loaded on-demand via Read tool Result: Context memory files reduced from 31.6k to ~15k tokens. --- doc/CONTRIBUTING.md | 476 +++++++++++----------------------------------------- 1 file changed, 99 insertions(+), 377 deletions(-) (limited to 'doc/CONTRIBUTING.md') diff --git a/doc/CONTRIBUTING.md b/doc/CONTRIBUTING.md index d7ea448..3a09dbc 100644 --- a/doc/CONTRIBUTING.md +++ b/doc/CONTRIBUTING.md @@ -1,451 +1,173 @@ # Contributing Guidelines -This document outlines the conventions to follow when contributing to this project. - ## Commit Policy -### Verify Build and Tests Before Committing - -Before preparing or proposing a commit, you **must** perform the following verifications to prevent regressions: +### Verify Before Committing -**Automated Verification (Recommended):** +**Automated (Recommended):** ```bash ./scripts/check_all.sh ``` -This script automatically: -- Builds with tests and tools enabled -- Runs the full test suite (26 tests) -- Verifies all tools compile (spectool, specview, specplay) -- Cross-compiles for Windows (if mingw-w64 is installed) - -**Manual Verification:** - -1. **MacOS / Linux (Native)**: - * Build the project with tests and tools enabled: - ```bash - cmake -S . -B build -DDEMO_BUILD_TESTS=ON -DDEMO_BUILD_TOOLS=ON - cmake --build build -j8 - ``` - * Run the entire test suite: - ```bash - cd build && ctest --output-on-failure - ``` - * Verify tools compile: - ```bash - cmake --build build --target spectool specview specplay - ``` - * Ensure all tests pass and all tools build successfully. - -2. **Windows (Cross-Compilation)**: - * If `mingw-w64` is installed on your system, you **must** also verify the Windows build. - * Run `./scripts/build_win.sh`. - * Ensure the build succeeds and produces the `demo64k_packed.exe` binary. - * Check the size report to ensure no unexpected bloat. - -Refer to the "Testing" and "Windows Cross-Compilation" sections in `HOWTO.md` for detailed instructions. - -### Verify Debug Logging Compiles Before Committing - -Before any significant commit (especially those modifying audio, rendering, or asset systems), **verify that the debug logging code compiles** without errors. This ensures that the diagnostic code remains maintainable even when not actively used. - -To enable all debug logging and verify compilation: -```bash -cmake -S . -B build_debug_check -DDEMO_ENABLE_DEBUG_LOGS=ON -cmake --build build_debug_check -j4 -``` - -The build **must** succeed without errors or warnings. Debug logging is stripped from release builds but preserved for development and troubleshooting. - -#### Debug Logging Categories - -The project uses conditional debug logging macros defined in `src/util/debug.h`: -- `DEBUG_LOG_AUDIO`: Audio backend timing, callbacks, device configuration -- `DEBUG_LOG_RING_BUFFER`: Ring buffer state, underruns, bounds checks -- `DEBUG_LOG_TRACKER`: Music pattern triggers, sample caching -- `DEBUG_LOG_SYNTH`: Voice management, spectrogram registration -- `DEBUG_LOG_3D`: 3D rendering, camera, scene queries -- `DEBUG_LOG_ASSETS`: Asset loading, procedural generation -- `DEBUG_LOG_GPU`: WebGPU commands, pipeline state +Runs tests, builds tools, cross-compiles Windows. -Use these macros instead of direct `fprintf(stderr, ...)` calls in new diagnostic code. - -**Example:** -```cpp -#include "util/debug.h" +**Manual:** +```bash +# 1. Tests +cmake -S . -B build -DDEMO_BUILD_TESTS=ON -DDEMO_BUILD_TOOLS=ON +cmake --build build -j4 +cd build && ctest --output-on-failure -#if defined(DEBUG_LOG_AUDIO) - static int callback_count = 0; - DEBUG_AUDIO("[CALLBACK #%d] Processing %d frames\n", ++callback_count, frame_count); -#endif /* defined(DEBUG_LOG_AUDIO) */ +# 2. Windows (if mingw-w64 installed) +./scripts/build_win.sh ``` -### Format Code Before Committing - -All code **must** be formatted using `clang-format` before committing. This ensures a consistent coding style across the entire codebase. - -**Warning**: Never apply formatting or make any manual edits to files in the `third_party/` directory. These are external libraries and must remain unmodified. - -To format your code, run the following command from the project root: +### Debug Logging Verification ```bash -clang-format -i $(git ls-files | grep -E '\.(h|cc)$' | grep -vE '^(assets|archive|third_party)/') -``` - -Refer to the `.clang-format` file in the project root for the specific style rules. - -### Ensure Newline at End of File - -All source files (`.h`, `.cc`, `.cpp`, etc.) must end with a newline character. This prevents "No newline at end of file" errors from linters and ensures consistent file handling. - -### Source File Headers - -Every source file (`.h`, `.cc`) must begin with a concise 3-line comment header describing its purpose. - -Example: -```cpp -// This file is part of the 64k demo project. -// It implements the core audio synthesis engine. -// This is not a user-facing header, but an internal one. +cmake -S . -B build_debug_check -DDEMO_ENABLE_DEBUG_LOGS=ON +cmake --build build_debug_check -j4 ``` +Must compile without errors. -### Function and method comments - -Functions and methods, especially if they are internal non user-facing, -should at least have a 1-line comment describing what they do or their -how/when they should be called. Except if they are just 1-line function -or very very short, obvious ones. - -### '#endif' directive - -The closing #endif directive must recall the corresponding opening #ifdef -clause they are closing +**Debug macros** (`src/util/debug.h`): +- `DEBUG_LOG_AUDIO`, `DEBUG_LOG_RING_BUFFER`, `DEBUG_LOG_TRACKER` +- `DEBUG_LOG_SYNTH`, `DEBUG_LOG_3D`, `DEBUG_LOG_ASSETS`, `DEBUG_LOG_GPU` Example: ```cpp -#ifdef MY_TAG -...some code -#endif /* MY TAG */ -``` - -We must also prefer '#if defined(MY_QUITE_LONG_TAG)' over '#ifdef MY_QUITE_LONG_TAG' -especially if there's a risk of having later something like: -```cpp -#if defined(MY_TAG_1) && !defined(MY_TAG_2) -``` - -### use and abuse 'const' directives - -Especially for local variable, use 'const' qualification as much as -possible. - -As an example, don't use: -```cpp -StructA variable_name = StructA(...); +#if defined(DEBUG_LOG_AUDIO) + DEBUG_AUDIO("[CALLBACK #%d] frames=%d\n", ++count, frames); +#endif ``` -but prefer instead: -```cpp -const StructA variable_name = StructA(...); +### Code Formatting +```bash +clang-format -i $(git ls-files | grep -E '\.(h|cc)$' | grep -vE '^(assets|archive|third_party)/') ``` +Never format `third_party/`. -if variable_name is not mutated afterward. - -Also: pass parameter as "const ref" as much as possible -(```const Struct& param``` instead of pointers or non-const refs) - -In summary: use 'const variable = ...;` as much as possible. - -### put spaces around code and operators (cosmetics) +### File Requirements +- Newline at end of file +- 3-line header comment +- Max 500 lines (split if larger) -Don't compact the code to much horizontally, and prefer adding extra -spaces around code and operators. +## Coding Style -Example: -```cpp -const bool v = my_variable && (my_function() / 3. > (1. / x)); -const y = function_call(3, x, 2.); -for (int x = 0; x < 24; ++x) { ... } -``` +### Core Rules +- `const` everywhere possible +- `const T* name` not `const T *name` +- Pre-increment `++x` not `x++` +- Spaces around operators: `x = (a + b) * c;` +- No trailing whitespace +- No `auto` (except complex iterators) +- No C++ casts (`static_cast`, `reinterpret_cast`) -instead of +### Preprocessor ```cpp -const bool v=my_variable&&my_function()/3.>(1./x); -const y = function_call(3,x,2); -for(int x=0;x<24;++x){ ... } +#if defined(MY_TAG) + ... +#endif /* defined(MY_TAG) */ ``` -### prefer prefixed incrementation over suffixed - -Use pre-incrementation: +### Struct Initialization ```cpp -++x -``` - -instead of post-incrementation: - -```cpp -x++ -``` - -### use extra () for boolean operations - -Even if they are not strictly needed due to operator precedence rules, -prefer adding extra ()'s around tests for clarity, with parcimony. - -### c++ cast - -don't use reinterpret_cast<>, static_cast<> or const_cast<>. - -### pointer declaration - -prefer ```const T* name``` to ```const T *name```. - -### 'auto' type - -Don't use 'auto' as type, unless for complex iterators or very complex -types. - -### don't use trailing whitespaces - -don't. - -### no missing \newline at the end of file - -make sure each file has a final \n newline. - -### c++ keyword indentation: - -The keyword 'public', 'protected', 'private' should be intended 1 character -less than the methods. +// Good +const WGPUDescriptor desc = { + .format = g_format, + .dimension = WGPUTextureViewDimension_2D, +}; -Example: -```cpp - private: - int field_; +// Bad +WGPUDescriptor desc = {}; +desc.format = g_format; +desc.dimension = WGPUTextureViewDimension_2D; ``` -instead of: +### Class Keywords ```cpp -private: + private: // 1 space indent int field_; ``` -### try to use per-field initialized const struct - -Use the `.field = ...,` initialization pattern instead for `var.field = -...;`, especially if it means you can have the variable be declared 'const' -that way. - -Example, instead of: -```cpp -WGPUTextureViewDescriptor view_desc = {}; -view_desc.format = g_format; -view_desc.dimension = WGPUTextureViewDimension_2D; -view_desc.mipLevelCount = 1; -view_desc.arrayLayerCount = 1; -``` - -use: -```cpp -const WGPUTextureViewDescriptor view_desc = { - .format = g_format, - .dimension = WGPUTextureViewDimension_2D, - .mipLevelCount = 1, - .arrayLayerCount = 1, -}; -``` - -Make sure the `.field = ...,` initialization pattern is compatible with the compiler / c++ version used. - -### vertical space - -keep the code compact vertically. That includes shader code, too. -Use only one statement per line. - -### File size limit - -Any file larger than 500 lines should ideally be split into sub-functionalities in separate files. (Exceptions: platform code or generated files). - -### finally - -Make sure everything is reflected in clang-format. +### Comments +- 1-line comment for non-obvious functions +- 3-line header for all source files ## Development Protocols -### Adding a New Visual Effect - -1. **Implement**: Create or update a class in `src/gpu/demo_effects.cc` (and declare in `demo_effects.h`) that inherits from `Effect`. - - Implement `init()` for one-time resource setup (e.g., using the asset system). - - Implement `compute()` if you need GPU-side physics or state updates. - - Implement `render()` to record WebGPU draw commands. -2. **Register**: Add an `EFFECT` entry to `assets/demo.seq` specifying the class name, start/end times, and any constructor arguments. -3. **Update Tests** (REQUIRED): Add your effect to `src/tests/test_demo_effects.cc`: - - Add effect to the appropriate test list (`test_post_process_effects()` or `test_scene_effects()`) - - Increment `EXPECTED_POST_PROCESS_COUNT` or `EXPECTED_SCENE_COUNT` at the top of the file - - If your effect requires `Renderer3D` with full shader setup, add it to the `requires_3d` check - - The test will fail with a clear error message if you forget this step -4. **Verify**: Build with `DEMO_ALL_OPTIONS=ON` and run tests: - ```bash - cmake -S . -B build -DDEMO_BUILD_TESTS=ON - cmake --build build --target test_demo_effects - cd build && ./test_demo_effects - ``` - Then test your effect at its specific timestamp with `--seek`. - -### Audio Subsystem Initialization - -The audio subsystem uses `AudioEngine` to manage initialization order and lifecycle. - -**In production code (`main.cc` or similar):** +### Adding Visual Effect +1. Implement `Effect` subclass in `src/gpu/demo_effects.cc` +2. Add to `assets/demo.seq` +3. **Update `test_demo_effects.cc`**: + - Add to test list + - Increment `EXPECTED_*_COUNT` +4. Verify: +```bash +cmake -S . -B build -DDEMO_BUILD_TESTS=ON +cmake --build build -j4 --target test_demo_effects +cd build && ./test_demo_effects +``` +### Audio Initialization +**Production:** ```cpp -#include "audio/audio_engine.h" - -// 1. Initialize audio backend audio_init(); - -// 2. Initialize audio engine (manages synth + tracker) static AudioEngine g_audio_engine; g_audio_engine.init(); - -// 3. In main loop g_audio_engine.update(music_time); ``` -**In tests:** - +**Tests:** ```cpp -#include "audio/audio_engine.h" - -void test_audio_feature() { - AudioEngine engine; - engine.init(); - - // Test logic here - engine.update(1.0f); - - engine.shutdown(); // Always cleanup at end -} +AudioEngine engine; +engine.init(); +engine.update(1.0f); +engine.shutdown(); ``` -**Low-level usage (when AudioEngine is not needed):** - -For tests that only need synth or tracker (not both), you can still call `synth_init()` or `tracker_init()` directly. However, if you need both, always use `AudioEngine` to ensure correct initialization order. - -**Direct synth API calls are valid:** - -- `synth_register_spectrogram()` - Register spectrograms -- `synth_trigger_voice()` - Trigger audio playback -- `synth_get_output_peak()` - Get audio levels for visualization -- `synth_render()` - Low-level rendering - -These are performance-critical APIs and should be called directly, not wrapped by AudioEngine. +Direct synth APIs (`synth_register_spectrogram`, `synth_trigger_voice`, etc.) are valid for performance-critical code. ### Fatal Error Checking - -The project uses fatal error checking macros that can be stripped for final builds. These are defined in `src/util/fatal_error.h`. - -**When to use fatal error checks:** - -Use fatal error checking for conditions that indicate programming errors or corrupt state that cannot be recovered from: -- Bounds checking (array/buffer overflows) -- Null pointer checks -- Invariant violations -- Invalid state transitions - -**DO NOT use for:** -- Expected runtime errors (file not found, network errors) -- Recoverable conditions -- Input validation (use return codes instead) - -**Macro Reference:** +Use `src/util/fatal_error.h` for programming errors only: ```cpp -#include "util/fatal_error.h" +// Most common (90%) +FATAL_CHECK(pos < capacity, "overflow: %d >= %d\n", pos, capacity); -// Conditional check (most common - 90% of cases) -FATAL_CHECK(write_pos >= capacity, - "write_pos out of bounds: %d >= %d\n", - write_pos, capacity); - -// Unconditional error (should-never-happen) +// Unconditional FATAL_ERROR("Invalid state: %d\n", state); -// Unreachable code marker (switch defaults) +// Unreachable switch (type) { - case TYPE_A: return handle_a(); - case TYPE_B: return handle_b(); + case A: return handle_a(); default: FATAL_UNREACHABLE(); } -// Assertion-style (documenting invariants) -FATAL_ASSERT(buffer != nullptr); -FATAL_ASSERT(count > 0 && count < MAX_COUNT); +// Assertion +FATAL_ASSERT(ptr != nullptr); -// Complex validation blocks +// Complex validation FATAL_CODE_BEGIN - static int callback_depth = 0; - ++callback_depth; - if (callback_depth > 1) { - FATAL_ERROR("Callback re-entered! depth=%d", callback_depth); - } + if (depth > MAX) FATAL_ERROR("depth=%d\n", depth); FATAL_CODE_END ``` -**Build modes:** -- **Debug/STRIP_ALL**: All checks enabled, full error messages with file:line info -- **FINAL_STRIP**: All checks compiled out (zero runtime cost, ~500-600 bytes saved) - -**Testing requirement:** +**DO NOT** use for expected errors (file not found, network errors). -Before committing code with fatal error checks, verify both modes compile: +**Build modes:** +- Debug/STRIP_ALL: All checks enabled +- FINAL_STRIP: All checks stripped (~500-600 bytes saved) +**Test both:** ```bash -# Normal build (checks enabled) -cmake -S . -B build -cmake --build build && cd build && ctest - -# FINAL_STRIP build (checks stripped) -cmake -S . -B build_final -DDEMO_FINAL_STRIP=ON -cmake --build build_final +cmake -S . -B build && cmake --build build -j4 && cd build && ctest +./scripts/build_final.sh ``` -Or use the convenience script: +### Script Maintenance +After hierarchy changes (moving files, renaming), verify: ```bash -./scripts/build_final.sh +./scripts/check_all.sh +./scripts/gen_coverage_report.sh ``` -**Important:** Never use `FATAL_CHECK` for conditions that can legitimately occur during normal operation. These are for programming errors only. - -### Script Maintenance After Hierarchy Changes - -After any major source hierarchy change (moving, renaming, or reorganizing files), you **must** review and update all scripts in the `scripts/` directory to ensure they remain functional. - -**When to perform this review:** -- Moving source files to new directories (e.g., `src/platform.cc` → `src/platform/platform.cc`) -- Renaming source files or directories -- Reorganizing the build system (CMake changes, new subdirectories) -- Changing asset locations or formats - -**Scripts that commonly require updates:** -- `scripts/check_all.sh` - Build verification and testing -- `scripts/gen_coverage_report.sh` - Code coverage analysis -- `scripts/build_win.sh` - Windows cross-compilation -- `scripts/gen_assets.sh` - Asset generation pipeline -- Any scripts with hardcoded paths or assumptions about file locations - -**Verification steps:** -1. Run `./scripts/check_all.sh` to verify builds and tests still work -2. Run `./scripts/gen_coverage_report.sh` to ensure coverage tracking handles new paths -3. Test platform-specific scripts if applicable (`build_win.sh`, `run_win.sh`) -4. Check for error messages about missing files or incorrect paths -5. Update documentation if script behavior has changed - -**Recent example:** -When `src/platform.cc` was moved to `src/platform/platform.cc`, the coverage script (`gen_coverage_report.sh`) initially failed with "unable to open /Users/skal/demo/src/platform.cc" due to stale coverage data. The fix required: -- Adding 'source' to `LCOV_OPTS` ignore list to handle missing source files -- Enabling automatic cleanup of the `build_coverage/` directory before each run -- Testing the script to verify it handles the new file structure - -**Automation:** -The `./scripts/check_all.sh` script is designed to catch most issues automatically. Always run it before committing hierarchy changes to ensure no regressions in build or test infrastructure. - +Update scripts with hardcoded paths. -- cgit v1.2.3