Skip to content

perf(wgpu): wire vsync config, add frame phase timing instrumentation - #662

Merged
aram-devdocs merged 8 commits into
mainfrom
codex/issue-660-frame-pacing
Apr 3, 2026
Merged

perf(wgpu): wire vsync config, add frame phase timing instrumentation#662
aram-devdocs merged 8 commits into
mainfrom
codex/issue-660-frame-pacing

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Overview

Type: perf

Summary:
Wire the existing vsync config flag to wgpu PresentMode (was hardcoded to AutoVsync), add per-phase frame timing instrumentation across the render pipeline, and upgrade the character_sandbox profiling to match Throne's pattern. Profiling confirms the vsync fix eliminates ~16ms of overhead per frame.

Related Issues: Fixes #660


Changes Made

Engine Core (goud_engine/src/)

  • Added libs/graphics/frame_timing.rs — thread-local per-frame phase timing cache with 8 measured phases
  • Modified libs/graphics/backend/wgpu_backend/init.rs — accept vsync: bool, use AutoNoVsync when disabled, reduce desired_maximum_frame_latency from 2 to 1
  • Modified libs/graphics/backend/wgpu_backend/xbox_init.rs — same vsync parameter for Xbox GDK path
  • Modified libs/graphics/backend/wgpu_backend/frame.rs — instrumented begin_frame (surface_acquire) and end_frame (uniform_upload, render_pass, gpu_submit, readback_stall, surface_present)
  • Modified libs/graphics/renderer3d/render/mod.rs — instrumented shadow_build and render3d_scene phases
  • Modified libs/platform/native_runtime.rs — pass window_config.vsync to WgpuBackend

FFI Layer (goud_engine/src/ffi/)

  • Added FfiFramePhaseTimings struct to ffi/types.rs
  • Added goud_renderer_get_frame_phase_timings() function to ffi/renderer/metrics.rs

C# SDK (sdks/csharp/)

  • Generated FfiFramePhaseTimings struct and P/Invoke declaration

Python SDK (sdks/python/)

  • Generated FfiFramePhaseTimings ctypes struct and FFI binding

TypeScript SDK (sdks/typescript/)

No changes

Codegen Pipeline (codegen/)

  • Schema changes — added FramePhaseTimings type
  • ffi_mapping changes — added type mappings and function entry

Proc Macros (goud_engine_macros/)

No changes

Tools (tools/)

No changes

WASM (goud_engine/src/wasm/)

No changes

Examples (examples/)

  • Upgraded character_sandbox profiling: duration-based sampling, per-frame metrics, markdown/JSON reports, shadow config, VSync toggle
  • Added Makefile with profile (200 NPCs + 80 animals) and profile-stress (400+ entities) targets

Documentation

No changes


Architectural Compliance

  • Rust-first: All logic lives in Rust; SDKs are thin wrappers
  • FFI boundary: New export uses #[no_mangle] extern "C" and #[repr(C)]
  • Dependency flow: frame_timing.rs at libs/graphics level, accessible from both wgpu_backend and renderer3d
  • SDK parity: FFI function exposed; typed GoudGame wrapper deferred (function has no context_id parameter)
  • Unsafe discipline: // SAFETY: comment on FFI function

Testing

  • cargo test passes (pre-existing native_main_thread test failure unrelated)
  • cargo clippy -- -D warnings is clean
  • cargo fmt --all -- --check passes
  • 4 unit tests for frame_timing module (record, reset, unknown field, defaults)

Code Quality

  • No todo!() or unimplemented!() in production code
  • Error handling uses Result, not unwrap()/expect() in library code
  • Public items have doc comments

Breaking Changes

None


Version Bump

Bump type: patch
Justification: Performance improvement, no API breaking changes


Security

  • New unsafe block has a // SAFETY: comment (FFI metrics function)
  • New FFI pointer parameter has null check

Performance

Profiling results with 280 entities (200 NPCs + 80 animals):

Config FPS Frame Time
Shadows + VSync ON (before fix) 13.2 71ms
Shadows + VSync OFF (after fix) 17.3 55ms
No shadows + VSync OFF 42.4 22ms
84 agents + VSync ON 55.5 17ms

VSync fix eliminates ~16ms overhead (one 60Hz vsync period).
Remaining render bottleneck is CPU shadow rasterization (out of scope).


Reviewer Notes

  • The goud_renderer_get_frame_phase_timings FFI function does not take a context_id parameter (reads from thread-local cache). A typed GoudGame.GetFramePhaseTimings() SDK wrapper is deferred until the function signature is finalized with context support.
  • GC pressure investigation (issue req Set FPS on engine init #4) is a .NET runtime concern best addressed in the Throne game layer, not the Rust engine.
  • The native_main_thread integration test failure is pre-existing on main.

- Wire existing GameConfig.vsync flag to wgpu PresentMode (was hardcoded
  to AutoVsync). Use AutoNoVsync when vsync is disabled.
- Reduce desired_maximum_frame_latency from 2 to 1 for lower input latency.
- Add per-phase frame timing instrumentation in begin_frame/end_frame
  (surface_acquire, uniform_upload, render_pass, gpu_submit,
  readback_stall, surface_present) and Renderer3D::render()
  (shadow_build, render3d_scene).
- Store timings in thread-local cache (libs/graphics/frame_timing.rs)
  accessible without debugger.
- Expose FfiFramePhaseTimings struct and goud_renderer_get_frame_phase_timings
  FFI function. Codegen updated for all SDKs.
- Upgrade character_sandbox profiling to Throne parity: duration-based
  sampling, per-frame metrics, markdown/JSON reports, shadow config,
  Makefile with profile/profile-stress targets.

Profiling confirms vsync fix eliminates ~16ms overhead per frame (one
60Hz period). With shadows disabled, 280 entities achieve 42 FPS.

Fixes #660

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the size/L label Apr 3, 2026
@claude

claude Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @aram-devdocs's task in 2m 18s —— View job


PR #662 Review: perf(wgpu): wire vsync config, add frame phase timing instrumentation

Verdict: APPROVED with warnings


Phase 1–3: Architecture & Anti-Pattern Scan

Check Result
Dependency flow (downward-only) frame_timing.rs at libs layer; ffi imports libs correctly
#[no_mangle] extern "C" on FFI export goud_renderer_get_frame_phase_timings
#[repr(C)] on FFI struct FfiFramePhaseTimings
Null check on pointer param ✅ present in metrics.rs:88
// SAFETY: comment ✅ present in metrics.rs:96
No raw GPU calls outside backend/
No todo!()/unimplemented!() in prod
No unwrap()/expect() in library code
SDK parity (C#, Python, Go, Kotlin, Swift) ✅ all updated

BLOCKERS

None.


WARNINGS (non-blocking)

1. String-key dispatch in record_timing is silently lossyframe_timing.rs:38–49

match field {
    "surface_acquire" => t.surface_acquire_us = value,
    // ...
    _ => {}  // silent drop
}

A typo at any call site (e.g. "surface_acuire") produces zero for that phase with no error or warning. The unknown_field_is_ignored unit test even documents this silent-failure behavior. At a minimum the call sites should be validated (all 8 callers use literal strings, so this is low risk today, but it's a maintenance trap as new phases are added). Consider an enum-based setter in a follow-up. Fix this →

2. Thread-local without a context guardmetrics.rs:85, frame_timing.rs:30

goud_renderer_get_frame_phase_timings takes no context_id and reads a bare thread_local!. If the engine is ever extended with multiple contexts on separate threads, the wrong context's timings are returned without any error. The PR notes mention this as deferred work — a // TODO: comment in both the FFI function and FRAME_TIMINGS declaration would make this visible to future contributors before they add multi-context support.

3. Missing GoudGame wrapper for C#sdks/csharp/

NativeMethods.g.cs:2477 exposes the raw P/Invoke, but there's no GoudGame.GetFramePhaseTimings() method. Consumers must call NativeMethods.goud_renderer_get_frame_phase_timings(ref timings) directly, leaking FFI details. The PR description acknowledges this is deferred — please track it as an issue so it doesn't get lost.

4. desired_maximum_frame_latency change is undocumentedinit.rs:80

The value was silently reduced from 2 → 1. This is a meaningful behavioral change: lower frame latency but higher risk of GPU pipeline stalls on slower hardware. The PR description doesn't mention it. Worth a brief comment at the call site explaining the trade-off (e.g. "reduced to 1 to cut acquisition latency at the cost of one pipeline slot; revisit if stutter appears on integrated GPUs").

5. Xbox vsync path has no coveragexbox_init.rs:31

The new_from_raw_handle now accepts vsync: bool and wires it to the surface config — good. But the existing TODO(xbox-gdk) comment on the file notes it already duplicates significant init logic. With two init paths that must stay in sync, the vsync logic is now duplicated across init.rs:73–77 and xbox_init.rs (assumed same pattern). If only one path is tested, a divergence could go unnoticed.


Positive Callouts

  • Clean module design: frame_timing.rs is a minimal, focused thread-local cache with clear public API and no external dependencies.
  • Good test coverage: 4 unit tests cover record, reset, unknown-field, and defaults — exactly the right surface for this module.
  • Full SDK sweep: Go, Kotlin, Swift, C#, Python all updated in the same commit. The generated headers are consistent across all SDK include/ copies.
  • vsync wiring is correct: AutoNoVsync for the disabled path is the right wgpu enum; AutoVsync retains backward-compatible behavior.
  • Profiling structure in character_sandbox: duration-based sampling with markdown/JSON output is production-quality tooling.
  • FFI discipline: null check, SAFETY: comment, #[repr(C)], #[no_mangle] — all present and correct.

- Move TextureOps and ShaderOps impls to frame_trait_impls.rs to keep
  frame.rs under the 500-line CI limit (520 -> 403 lines).
- Add render3d_scene timing around the full Renderer3D::render() call.
- Add unit tests for frame_timing module (record, reset, unknown field,
  defaults).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/L labels Apr 3, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
Incorporate PR #661 (SDL windowing/Switch PoC) changes.
No conflicts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
Apply the same vsync parameter pattern to new_from_sdl_handle() and
new_from_switch_handle() introduced by PR #661, ensuring vsync config
is respected across all platform backends.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
Replace the software triangle rasterizer in shadow.rs with a GPU
depth-only render pass. This is the primary fix for the 14 FPS frame
pacing issue — shadow generation drops from ~50ms to <2ms.

Changes:
- Add shadow_pass.rs: GPU shadow resource management, depth-only
  pipeline building, and shadow render pass execution
- Extend end_frame() with two-pass rendering: shadow pass to offscreen
  Depth32Float texture, then main pass sampling it via comparison sampler
- Add shadow sampling to ALL WGSL shaders (previously only GLSL had it),
  including PCF 3x3 with hardware depth comparison
- Add depth-only WGSL shader for shadow geometry pass
- Expand pipeline layout to 4 bind groups (uniforms/texture/storage/shadow)
- Extract compute_light_space_matrix() from CPU rasterizer for reuse
- Gate frame readback: only prepare when post-processing/FXAA requests it
- Keep CPU rasterizer as fallback for OpenGL legacy backend

Profiling results at 280 entities with shadows + 2048 shadow map:
- Before: 13.2 FPS (71ms/frame, 65ms render)
- After:  62.0 FPS (15ms/frame, 1.8ms render) — 36x render speedup

At 500 entities with shadows: 50.6 FPS (18ms/frame)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
CI Fixes:
- Add 12 missing shadow/readback fields to SDL, Switch, Xbox init paths
- Extract shared shadow resource helpers to avoid code quadruplication
- Split render/mod.rs (537→474 lines) by extracting shadow_render.rs

Frame Pacing:
- Restore desired_maximum_frame_latency from 1 to 2 (wgpu default)
  for proper double-buffering on high-refresh displays

Critical Quality:
- Fix shadow_pass timing silently dropped (missing field in match)
- Fix depth_only_shader GPU resource leak on Renderer3D drop
- Fix Xbox vsync hardcoded to true instead of window_config.vsync
- Dedup light-space matrix computation (build_directional_shadow_map
  now calls compute_light_space_matrix internally)
- Remove dead _has_storage variable in pipeline.rs

Dead Code Cleanup:
- Delete orphan sdk/game/instance/runtime.rs (not in any mod.rs)
- Remove stale "demonstrate the API" comment
- Replace magic 10000 with MAX_HEADLESS_FRAMES constant
- Update MAX_SHADOW_VERTICES comment for CPU fallback context
- Fix stale doc on depth_only_vertex_layout
- Remove redundant draw_commands.clear() in end_frame

Robustness:
- Add shadow_strength to ShadowConfig (replaces hardcoded 0.65)
- Compute shadow camera distance from scene bounds (was hardcoded 20.0)
- Derive shadow ortho padding from scene radius (was hardcoded 10.0)
- Add record_phase() helper unifying frame_timing + debugger calls
- Log warning and skip shadow pass on shader bind failure

Profiling: 63.9 FPS at 280 entities with shadows (no vsync),
48-55 FPS with vsync on 120Hz ProMotion, 54 FPS at 500 entities.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL and removed size/XL labels Apr 3, 2026
@aram-devdocs
aram-devdocs merged commit 13007c1 into main Apr 3, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: investigate 14 FPS frame pacing — 41ms unexplained overhead in game loop

1 participant