Preserve preview size#111
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a “preserve preview size” mode that decouples OBS-facing dimensions (logical size) from the internally-rendered CRT effect geometry (virtual size), while keeping legacy size-changing behavior for existing instances unless explicitly enabled.
Changes:
- Adds shared geometry/migration helpers (
c64-effect-geometry.*) to compute logical/virtual/reported/draw dimensions and to migratepreserve_sizedefaults safely. - Updates the source + effects filter rendering paths and shader uniform naming to use virtual geometry while optionally preserving OBS-facing bounds.
- Expands test coverage (unit + pipeline + E2E scenarios) and documents migration + scripting behavior, including new E2E assertions for bounds stability/variation.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
CMakeLists.txt |
Builds the new shared geometry helper into the plugin. |
README.md |
Documents preserve-size behavior, migration expectations, and script usage. |
data/effects/crt_effect.effect |
Renames/uses virtual_output_height for scanline row selection in virtual space. |
data/locale/en-US.ini |
Adds UI strings for “Preserve preview size”. |
data/scripts/preserve_size_toggle_cycle.c64script |
New demo script that toggles preserve-size at runtime. |
doc/c64script/c64script-spec.md |
Documents preserve_size as a layout flag controllable via EFFECTPARAM. |
doc/release-notes.md |
Adds release notes for the preserve preview size feature. |
doc/testing/e2e.md |
Documents new preserve-size-related scenarios/assertions. |
src/c64-source.c |
Uses shared geometry + migrate preserve_size; renders using logical vs virtual geometry split. |
src/script/vm/c64-script-vm-dispatch-effects.c |
Adds boolean updates so scripts can toggle preserve_size. |
src/ui/c64-properties.c |
Adds the preserve_size property and includes it in INI import/export + defaults. |
src/util/c64-types.h |
Stores preserve_size in the source runtime state. |
src/video/c64-effect-geometry.c |
Implements geometry calculations and preserve_size migration resolver. |
src/video/c64-effect-geometry.h |
Declares geometry struct/helpers and migration resolver. |
src/video/c64-effect.c |
Adds preset matching helper and ensures presets don’t override preserve_size. |
src/video/c64-effect.h |
Declares c64_effect_matches_preset. |
src/video/c64-stream-effects.c |
Migrates preserve_size, uses shared geometry for size reporting + render path. |
src/video/c64-stream-effects.h |
Tracks preserve_size in filter state. |
tests/e2e/assertions/__init__.py |
Exposes new assertions (bounds + effect-change/log). |
tests/e2e/assertions/__main__.py |
Passes scenario threshold overrides into assertion creation. |
tests/e2e/assertions/bounds.py |
New bounds stability/variation assertions based on sampled content bounds. |
tests/e2e/assertions/effect_change.py |
New lightweight “effect changed” assertion. |
tests/e2e/assertions/effect_cycle_log.py |
New assertion verifying effect-cycle script completion via logs. |
tests/e2e/assertions/runner.py |
Registers new assertions and supports per-assertion threshold overrides. |
tests/e2e/e2e.py |
Adds scenario-driven skip_frame_logic_validation flag wiring. |
tests/e2e/framework/orchestrator.py |
Plumbs skip-frame-logic flag into the validator. |
tests/e2e/framework/validation/results.py |
Supports skipping shared frame-logic validation per scenario. |
tests/e2e/scenarios/README.md |
Documents new thresholds and skip_frame_logic_validation fields. |
tests/e2e/scenarios/ntsc_filter_preserve_size/scenario.yaml |
New E2E scenario: filter with preserve-size enabled. |
tests/e2e/scenarios/ntsc_filter_preserve_size/overrides/basic/scenes/C64StreamTest.json |
Scenario OBS scene override for filter test. |
tests/e2e/scenarios/ntsc_legacy_size_effect_cycle/scenario.yaml |
New E2E scenario: legacy size-changing behavior coverage. |
tests/e2e/scenarios/ntsc_preserve_size_effect_cycle/scenario.yaml |
New E2E scenario: preserve-size effect cycle (source). |
tests/e2e/scenarios/ntsc_preserve_size_sharp_scanlines/scenario.yaml |
New E2E scenario: preserve-size with sharp scanlines. |
tests/e2e/scenarios/ntsc_preserve_size_sharp_scanlines/overrides/basic/scenes/C64StreamTest.json |
Scenario OBS scene override for sharp scanlines. |
tests/e2e/scenarios/ntsc_script_preserve_size_toggle/scenario.yaml |
New E2E scenario: script toggles preserve-size at runtime. |
tests/e2e/scenarios/pal_preserve_size_effect_cycle/scenario.yaml |
New E2E scenario: preserve-size effect cycle (PAL). |
tests/e2e/util/scenario_loader.py |
Loads per-assertion thresholds from scenario YAML. |
tests/e2e/util/test_av_sync.py |
Updates A/V pop ROI math to account for PAL vs NTSC logical geometry. |
tests/filter/CMakeLists.txt |
Adds geometry test subdirectory. |
tests/filter/geometry/test_c64_effect_geometry.c |
New unit tests for geometry + preserve-size migration behavior. |
tests/filter/pipeline/CMakeLists.txt |
Links geometry helper into pipeline test executable. |
tests/filter/pipeline/obs_stubs.c |
Adds stub for new c64_effect_matches_preset symbol. |
…tests for syntax validation
- Added PNG artifacts for effect preservation tests. - Implemented unit tests for OBSLogManager to verify script status and completion detection. - Created unit tests for scenario loader to ensure scenarios load correctly and handle errors appropriately.
- Make find_package(PNG) optional so Windows can resolve it transitively through OBS::libobs, matching the behavior before this PR was raised - Fix absolute module path in test_network_simulation.py: use util.generate_packets instead of tests.e2e.util.generate_packets since pytest runs from tests/e2e/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The tests/script build also had find_package(PNG REQUIRED) which fails on Windows CI. Made it optional to match the top-level fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
S_ISREG is a POSIX macro not available on MSVC. Add a compatibility definition using the Windows _S_IFMT/_S_IFREG constants so the file stability check compiles on Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ASSERT IMAGE_EQUALS feature uses libpng, but on Windows the obs-deps PNG .lib is not found by CMake's FindPNG module (headers are present but PNG_LIBRARY is missing). This caused 25 unresolved external symbol errors when linking c64stream.dll. Fix by: - Defining compile-time symbol C64_HAVE_PNG when PNG::PNG is linked - Guarding png.h include, all PNG helper structs/functions, and the OP_ASSERT_IMAGE_EQUALS case body with #ifdef C64_HAVE_PNG - Returning "PNG support not available" on platforms without PNG
- Implemented PreserveSizeCanvasMatchAssertion to validate OBS canvas composition against preserve-size semantics. - Added new assertion to the assertion runner and included it in the ntsc_preserve_compare scenario. - Created unit tests for the PreserveSizeCanvasMatch assertion covering frame index computation, source screenshot dimension validation, and canvas geometry checks. - Introduced utility functions for generating test PNGs and synthetic canvas frames. - Ensured that tests can detect regressions related to preserve-size handling in OBS.
The Python Unit Tests CI job was failing because: - validate_source_dimensions() uses Pillow (PIL) to read PNG dimensions - Pillow was not listed in tests/e2e/requirements.txt - The function correctly returns status='skip' when PIL is absent, but the unit tests asserted on ok/message without checking for skip first Fix: - Add Pillow>=10.0.0 to tests/e2e/requirements.txt - Guard TestSourceDimensionValidation tests with skipTest on skip status
Add three new validation checks to quantitative scanline mode: Phase 5b - Gap group count: when expected_gap_groups is specified in thresholds, validate the observed number of gap groups matches (±tolerance). For NTSC 240 pixel rows, expect exactly 239 gaps (one between each consecutive pair of C64 pixel rows, CRT electron-beam model). Phase 5c - Gap width uniformity: all gap bands must have the same pixel width (±1 px for OBS scaling rounding). Catches rendering artifacts that produce irregular gap sizes. Frame extraction: score by gap group count instead of BLACK row count. This naturally selects frames rendered at virtual resolution (preserve_size=false) which have the full one-gap-per-C64-pixel-row topology (239 groups), over logical-resolution frames (80 groups). Applied to ntsc_preserve_compare scenario with expected_gap_groups: 239.
…s for CRT effects
There was a problem hiding this comment.
Pull request overview
This PR introduces a “preserve preview size” mode by splitting logical vs. virtual CRT geometry, updating effect rendering to keep OBS-facing dimensions stable by default, and extending C64Script + E2E coverage to validate the new behavior.
Changes:
- Add geometry helpers (
c64-effect-geometry) and integrate them into the effects filter/source sizing and shader parameters. - Extend C64Script with new OBS control + image assertion commands and migrate
SONGNRto whitespace syntax. - Expand E2E framework/scenarios/assertions to cover preserve-size behavior, bounds stability/variation, script completion, and quantitative scanline validation.
Reviewed changes
Copilot reviewed 94 out of 121 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/script/test_c64script_parser.c | Updates parser unit tests for new syntax/commands. |
| tests/script/scripts/test_sid_playback.expected-trace.yaml | Updates expected trace for songnr whitespace syntax. |
| tests/script/scripts/test_sid_playback.c64script | Updates SID playback script syntax. |
| tests/script/scripts/test_recording.expected-trace.yaml | Updates expected trace for OBS recording commands + waits. |
| tests/script/scripts/test_recording.c64script | Migrates recording script to OBS RECORDING .... |
| tests/script/scripts/test_coverage_keyword_variants.expected-trace.yaml | Updates keyword-variant coverage expectations. |
| tests/script/scripts/test_coverage_keyword_variants.c64script | Uses new OBS recording commands in coverage script. |
| tests/script/scripts/test_c64_control.expected-trace.yaml | Updates expected traces for songnr whitespace syntax. |
| tests/script/scripts/test_c64_control.c64script | Updates playsid invocations to songnr whitespace syntax. |
| tests/script/CMakeLists.txt | Refactors test link deps; adds optional PNG support. |
| tests/script/c64script_test_stubs.h | Adds source-related stub APIs for VM tests. |
| tests/script/c64script_test_stubs.c | Implements source + filesystem stubs for VM tests. |
| tests/script/c64script_runtime_support_stubs.c | Adds runtime stubs for rest-network tests. |
| tests/filter/pipeline/obs_stubs.c | Adds stub for c64_effect_matches_preset. |
| tests/filter/pipeline/CMakeLists.txt | Includes geometry implementation in pipeline test build. |
| tests/filter/geometry/test_c64_effect_geometry.c | New unit tests for geometry + preserve_size migration logic. |
| tests/filter/geometry/CMakeLists.txt | New CMake target for geometry unit tests. |
| tests/filter/CMakeLists.txt | Enables geometry test subdirectory. |
| tests/e2e/util/udp_replay.c | Replaces fixed manifest cap with dynamic realloc growth. |
| tests/e2e/util/test_scenario_loader.py | Adds unit tests for scenario loader validation rules. |
| tests/e2e/util/test_scanline_quantitative.py | Adds unit tests for quantitative scanline validation. |
| tests/e2e/util/test_network_simulation.py | Adds tests for still pattern invariance + disable_pops behavior. |
| tests/e2e/util/test_av_sync.py | Adjusts pop-box logic for logical-vs-virtual dimensions. |
| tests/e2e/util/scenario_loader.py | Adds scenario validation + preserve_size/fixed-bounds scene logic. |
| tests/e2e/util/report_generator.py | Updates report output for skipped A/V sync scenarios. |
| tests/e2e/util/generate_packets.py | Adds still pattern + disable_pops audio behavior. |
| tests/e2e/test_script_executor.py | Ensures unittest discovery respects skip for ctypes WIP. |
| tests/e2e/test_obs_logs.py | Adds unit tests for OBS log script-completion detection. |
| tests/e2e/shell_lib/scenarios.sh | Adds scenario duration/disable_pops handling. |
| tests/e2e/shell_lib/packets.sh | Allows still packet pattern selection. |
| tests/e2e/scenarios/README.md | Documents thresholds + skip_frame_logic_validation usage. |
| tests/e2e/scenarios/pal_preserve_effect_cycle/scenario.yaml | New PAL preserve-size effect-cycle scenario. |
| tests/e2e/scenarios/ntsc_script_preserve_toggle/scenario.yaml | New script-driven preserve toggle scenario. |
| tests/e2e/scenarios/ntsc_preserve_scanlines/scenario.yaml | New preserve-size scanline validation scenario. |
| tests/e2e/scenarios/ntsc_preserve_scanlines/overrides/basic/scenes/C64StreamTest.json | New fixed scene override for preserve scanlines scenario. |
| tests/e2e/scenarios/ntsc_preserve_effect_cycle/scenario.yaml | New NTSC preserve-size effect-cycle scenario. |
| tests/e2e/scenarios/ntsc_preserve_compare/scenario.yaml | New compare scenario with still pattern + thresholds. |
| tests/e2e/scenarios/ntsc_legacy_effect_cycle/scenario.yaml | New legacy-mode effect-cycle scenario. |
| tests/e2e/scenarios/ntsc_filter_preserve_size/scenario.yaml | New filter preserve-size scenario. |
| tests/e2e/scenarios/ntsc_filter_preserve_size/overrides/basic/scenes/C64StreamTest.json | New filter-scene override enabling preserve_size. |
| tests/e2e/requirements.txt | Adds Pillow dependency. |
| tests/e2e/framework/validation/results.py | Adds skip_frame_logic_validation + disable_pops behavior. |
| tests/e2e/framework/orchestrator.py | Adds script completion wait + pass-through flags to validator. |
| tests/e2e/framework/obs/logs.py | Adds live log polling for script completion detection. |
| tests/e2e/e2e.py | Adds scenario cleanup, still-pattern handling, and new flags. |
| tests/e2e/assertions/script_status.py | New assertion for script success based on OBS log. |
| tests/e2e/assertions/runner.py | Registers new assertions + thresholds support. |
| tests/e2e/assertions/effect_cycle_log.py | New assertion verifying effect-cycle script log lines. |
| tests/e2e/assertions/effect_change.py | New lightweight visual state-change assertion. |
| tests/e2e/assertions/bounds.py | New bounds stability/variation assertions. |
| tests/e2e/assertions/main.py | Wires scenario thresholds into assertion creation. |
| tests/e2e/assertions/init.py | Exposes newly added assertions. |
| tests/e2e/artifacts/effect_preserve_size/arcade_preview_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/arcade_preview_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/arcade_source_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/arcade_source_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/classic_preview_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/classic_preview_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/classic_source_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/classic_source_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/default_preview_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/default_preview_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/default_source_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/default_source_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_preview_0.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_preview_1.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_preview_2.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_source_0.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_source_1.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/pre_source_2.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/sharp_preview_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/sharp_preview_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/sharp_source_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/sharp_source_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/vintage_preview_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/vintage_preview_preserve.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/vintage_source_legacy.png | Adds expected artifact image. |
| tests/e2e/artifacts/effect_preserve_size/vintage_source_preserve.png | Adds expected artifact image. |
| src/video/c64-stream-effects.h | Adds preserve_size field to filter state. |
| src/video/c64-stream-effects.c | Integrates geometry split + preserve_size migration. |
| src/video/c64-effect.h | Adds preset-matching API. |
| src/video/c64-effect.c | Implements c64_effect_matches_preset and refactors preset lookup. |
| src/video/c64-effect-geometry.h | Declares geometry + preserve_size migration helpers. |
| src/video/c64-effect-geometry.c | Implements geometry scaling and migration resolution. |
| src/util/c64-types.h | Adds preserve_size + script render counter to source state. |
| src/ui/c64-properties.c | Adds preserve_size UI + INI import/export support; executor signature update. |
| src/script/vm/c64-script-vm-dispatch-machine.c | Adds OBS screenshot/wait/recording opcode handling. |
| src/script/vm/c64-script-vm-dispatch-effects.c | Supports bool updates for preserve_size via EFFECTPARAM. |
| src/script/vm/c64-script-vm-dispatch-core.c | Routes new opcodes; (header comment needs cleanup). |
| src/script/vm/c64-script-bytecode.c | Compiles new OBS/ASSERT AST statements to bytecode. |
| src/script/runtime/c64-script-executor.h | Updates executor create signature to accept source_data. |
| src/script/runtime/c64-script-executor.c | Logs script completion; threads stop handling update. |
| src/script/frontend/c64-script-token.c | Adds new tokens/keywords; removes RECORDSTART/STOP tokens. |
| src/script/frontend/c64-script-parser.h | Updates PLAY_SID grammar comment for whitespace SONGNR. |
| src/script/frontend/c64-script-parser.c | Adds OBS/ASSERT parsing + rejects = clause syntax. |
| src/script/frontend/c64-script-ast.c | Frees new AST nodes. |
| src/script/c64-script.h | Adds new token/AST/opcode enums and structs. |
| src/c64-source.h | Declares script screenshot/wait hooks. |
| README.md | Documents preserve preview size behavior and scripting toggle. |
| doc/testing/e2e.md | Updates scenario/assertion documentation for new coverage. |
| doc/release-notes.md | Adds unreleased notes for preserve preview size. |
| doc/c64script/vscode/c64script.tmLanguage.json | Updates syntax highlighting keywords for new commands. |
| doc/c64script/vscode/c64script-vscode-syntax.md | Updates docs for OBS/ASSERT syntax highlighting. |
| doc/c64script/c64script-spec.md | Documents new OBS control + image assertions; updates SONGNR syntax. |
| doc/c64script/c64script-grammar.ebnf | Updates EBNF for OBS/ASSERT and SONGNR whitespace syntax. |
| data/scripts/preserve_size_toggle_cycle.c64script | New demo script toggling preserve_size at runtime. |
| data/scripts/preserve_size_compare.c64script | New script capturing screenshots + asserting equality/determinism. |
| data/scripts/demo_sid_playback_cycle.expected-trace.yaml | Updates expected trace for SONGNR whitespace syntax. |
| data/scripts/demo_sid_playback_cycle.c64script | Updates demo script syntax for SONGNR whitespace usage. |
| data/locale/en-US.ini | Adds localized strings for preserve preview size. |
| data/effects/crt_effect.effect | Switches scanline row computation to virtual output height. |
| CMakeLists.txt | Adds PNG discovery/linking and geometry source to build. |
| build | Sets PYTHONPATH for unittest runs that import util. |
| .gitignore | Ensures nested tests/**/CMakeLists.txt are not ignored. |
Summary
Verification