feat(backends): conform raylib + null windows to the canonical window contract (#386) - #411
Merged
Merged
Conversation
…ct (#386) The pluggable-backends window contract (labelle-core/src/window_contract.zig) standardizes the window surface on `width`/`height`/`frameDuration`/`requestQuit`. bgfx + sokol already conform; raylib/sdl/wgpu/null still use legacy names (`getScreenWidth`/`windowShouldClose`/`getFrameTime`). This conforms the two where it's a clean, dependency-free win: - raylib: canonical decls alias the existing getters (`width`→getScreenWidth etc.); `frameDuration` widens getFrameTime to f64; `requestQuit` latches a flag `windowShouldClose` ORs in (raylib has no native programmatic close — additive, no behavior change unless called). Renamed initWindow's `width`/`height` params to `*_px` so they no longer shadow the new decls. - null: mirrors the conformed nullfixture backend — `width`/`height` return the `initWindow` dims, `frameDuration` is 1/60, `requestQuit` is a no-op. Additive only: the legacy names stay (the generated raylib/null run-loop templates still call them), so generated output is byte-identical and the golden suites are unchanged. This is the prerequisite for sharing one canonical run-loop template across backends and for extending the #386 Phase-6b contract guard to built-ins. Deferred (separate slices): sdl/wgpu lack the underlying size/timing getters (real per-backend work, not aliases); per-backend `comptime core.assertWindow` self-assertion needs a labelle-core dep on each backend → the known CI-only core-unification path, kept out of this low-risk change. Verified: null backend `zig build test` green (incl. a new canonical-contract test); raylib backend compiles and a generated raylib game gets past window.zig (the prior param-shadow error is gone; the remaining DecodedImage.compressed error is pre-existing local-pin drift, reproduces on clean main). Assembler `zig build test` green.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a canonical window contract API ( ChangesCanonical window contract for null and raylib backends
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the canonical window contract (width, height, frameDuration, and requestQuit) for both the null and raylib backends to satisfy the core window assertion. Feedback is provided for the raylib backend, where the global quit_requested state should be reset to false inside initWindow to prevent issues during sequential window initializations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+16
to
19
| pub fn initWindow(width_px: i32, height_px: i32, title: [:0]const u8) void { | ||
| rl.initWindow(width_px, height_px, title); | ||
| rl.setExitKey(.escape); | ||
| } |
There was a problem hiding this comment.
If the window is closed and subsequently re-initialized (for example, during integration tests or multiple sequential runs), quit_requested will persist as true. This will cause the newly initialized window to immediately signal that it should close. Resetting quit_requested to false inside initWindow ensures a clean state for each window session.
pub fn initWindow(width_px: i32, height_px: i32, title: [:0]const u8) void {
quit_requested = false;
rl.initWindow(width_px, height_px, title);
rl.setExitKey(.escape);
}
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in 8bc5bd7. initWindow now clears quit_requested = false so a re-init (sequential runs / integration tests) starts fresh and doesn't inherit a prior requestQuit() latch.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backends/raylib/src/window.zig`:
- Around line 16-19: The window lifecycle in initWindow and related close/reopen
paths is not resetting the quit_requested latch, so a prior requestQuit() call
can make later windowShouldClose() checks fail immediately. Update the window
backend to clear quit_requested whenever a new window is initialized and ensure
the state is also reset across the shutdown/reopen flow in the functions that
manage lifecycle (including requestQuit and windowShouldClose) so the backend is
reentrant for repeated initWindow calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 030ef0d0-e937-42ad-b702-422cac76d659
📒 Files selected for processing (2)
backends/null/src/window.zigbackends/raylib/src/window.zig
Both bot reviewers flagged that the requestQuit latch persisted across window re-inits (sequential runs / integration tests would stay 'should close' once requestQuit fired). Clear it in initWindow so each lifecycle starts fresh.
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
… (#418) Pluggable-backends Phases 5+6 (epic #386): the assembler can now FETCH, verify, and build out-of-tree backend packages, and bgfx is extracted to its own repo. - #409 6a: remote .backend_package cache-fetch (fetched like a plugin) - #410 6b: contract-verify an external backend (assertBackend/Window/Input) - #411: conform raylib + null windows to the canonical window contract - #412 Phase 5: enum-as-shorthand resolution (a built-in tag can resolve to a package) - #413 6c: bgfx extracted -> github.com/labelle-toolkit/labelle-bgfx; opt-in CI-verified - #414: select the GUI bridge by backend name, not the enum (external backends) - #415: labelle-bgfx v0.2.0 — gamepad sources extracted to their own packages - #416/#417: the two flip-blockers (callback-external guard; android enum-fallthrough) Opt-in today via .backend_package; built-in .backend = .bgfx still ships bundled (the default-flip is a follow-up gated on this release). Built-in backends are byte-identical. External bgfx validated on-device (Galaxy Tab A7): builds, runs crash-free, behaves identically to bundled.
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
…raction step 1) (#424) * docs(ci): refresh stale bgfx comments (jobs removed; bgfx is in labelle-bgfx) The examples-integration preambles still described the removed bgfx-build / bgfx-android-build jobs + the old bgfx external-fetch step. Point them at labelle-bgfx (which owns bgfx's build/test now) and describe the agnostic nullfixture external step. (CodeRabbit on #422.) * feat(wgpu): conform window to the canonical contract (width/height/frameDuration/requestQuit) First step of extracting wgpu out-of-tree (#386), mirroring the in-tree raylib/null conformance (#411): the wgpu window now satisfies labelle-core's `assertWindow` contract. Adds canonical `width()`/`height()` (physical framebuffer px), `frameDuration()` (GLFW monotonic clock, first-call baseline), `requestQuit()` (latches a flag OR'd into windowShouldClose), and the loop-style `shouldQuit()` alias. Additive — the desktop template still calls the legacy names + a fixed 0.016 dt, so generated output is byte-identical; these exist for the contract guard + the manifest-driven template the extraction will use. initWindow's width/height params renamed to `*_px` to avoid shadowing the new module-level decls (same fix #411 made for raylib).
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
…step 2) (#426) * fix(wgpu): reset quit/timing state in initWindow (close→reopen) quit_requested + last_frame_time persisted across a window close→reopen: a prior requestQuit would close the new window immediately, and the first frameDuration would be a huge time-since-old-baseline. Reset both at initWindow (same fix raylib got in #411). CodeRabbit/gemini on #424. * feat(wgpu): add backend.manifest.zon — manifest-splice codegen (#386 step 2) wgpu step 2 of out-of-tree extraction (after window conformance #424). Presence of backends/wgpu/backend.manifest.zon opts the wgpu DESKTOP build into the manifest-splice path (manifest_splice.zig) instead of the enum `switch (cfg.backend)` sections in build_zig.txt — exactly as bgfx did in #396. Loop-style, desktop-only (wgpu has no wasm/android target). No params: the fragments take no gamepad/gui toggles and wgpu pulls no shared gamepad sub-package, so unlike bgfx there's no core-diamond override. build_fragments/{backend_dep,link}.txt are the verbatim .backend_wgpu / .link_wgpu section bodies. Output is BYTE-IDENTICAL to the enum path (diffed a generated baseline: 0 differences). `zig build test` green (golden suites unchanged). This is the step that lets wgpu's build sections travel WITH the package on extraction.
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
…step 2) (#430) * chore(slim): remove the bundled wgpu backend — it's extracted (#386 Phase 6c) wgpu is resolved out-of-tree now (`.backend = .wgpu` → the labelle-wgpu provider, flipped in #428), so the in-tree backends/wgpu/ slot is dead weight. Delete it + the assembler example + the coverage that built them in-tree: - Remove backends/wgpu/ (src/templates/build_fragments/manifest/build + example). - Remove examples/wgpu (the assembler-generated wgpu project) — its assembler-builds-a-wgpu-project coverage (incl. the Foundation/QuartzCore/Metal link-fragment regression guard) moved to labelle-wgpu's CI (PR #1). - CI: drop the `wgpu backend WAV parser tests` + `wgpu backend demo build (macOS)` steps and the `Generate + build the wgpu example` examples-integration step. - Tests: drop the desktop wgpu-artifacts unit test (no in-tree package to resolve) and the wgpu case from the in-tree-template preview regression list. Two backends now fully out of the assembler bundle (bgfx + wgpu). The agnostic external-fetch path stays covered by the `external-null` (nullfixture) step. * feat(null): add backend.manifest.zon — manifest-splice codegen (#386 step 2) null extraction step 2 (window conformance was done in #411). Presence of backends/null/backend.manifest.zon opts the null DESKTOP build into the manifest-splice path instead of the enum `switch (cfg.backend)` sections. Loop-style (the headless main drives a fixed-frame tick loop), pure-Zig, zero deps. The LINK fragment is EMPTY — null has no native artifact (the enum path's `.null => {}` emitted nothing). backend_dep.txt is the verbatim .backend_null section body. Output BYTE-IDENTICAL to the enum path (diffed a generated baseline → 0 diff), including the is_tests_target path (which forces .backend=.null on host). `zig build test` green.
apotema
added a commit
that referenced
this pull request
Jun 30, 2026
… (#438) * feat(sdl): conform window to the canonical contract (#386) First step of extracting the SDL backend out-of-tree (epic #386): make backends/sdl/src/window.zig satisfy labelle-core's canonical window contract (core.assertWindow), mirroring the raylib backend's prior conformance. This is purely ADDITIVE and byte-identical for generated output: the generated SDL run-loop templates still call the legacy names (windowShouldClose, getScreenWidth via gfx, beginDrawing, ...). The new decls are thin aliases/wrappers over SDL's existing internals: - width()/height() -> gfx.getScreenWidth()/getScreenHeight() - frameDuration() -> seconds since last call from a dedicated SDL_GetPerformanceCounter baseline (SDL stores no frame-time; the FPS limiter discards its delta) - requestQuit() -> latches the existing should_close flag - shouldQuit() -> alias of windowShouldClose() (its presence marks SDL as a loop-model backend) Also resets should_close and the frameDuration baseline at the top of initWindow so a close->reopen starts clean, and renames initWindow's width/height params to width_px/height_px to avoid shadowing the new module-level width()/height() decls. Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw * feat(raylib): add backend.manifest.zon — manifest-splice codegen (#386) raylib extraction step toward out-of-tree (window already conformed in #411). Presence of backends/raylib/backend.manifest.zon opts the raylib DESKTOP build into the manifest-splice path instead of the enum sections in build_zig.txt. Loop-style. Params gamepad_enabled/gamepad_hidapi (the backend_dep fragment forwards them to b.dependency, as the enum renderSection did) + the fragment carries the core-diamond overrides (input + the transitive sdl_gamepad sub-package). WASM keeps the enum path (link_raylib_wasm) — the splice is desktop-only. Output BYTE-IDENTICAL to the enum path (diffed a generated baseline → 0 diff). zig build test green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Conform raylib + null to the canonical window contract
Follow-up to Phase 6b (#410). The pluggable-backends window contract (
labelle-core/src/window_contract.zig) standardizes the window surface onwidth/height/frameDuration/requestQuit. Survey of the in-tree backends:Changes
width→getScreenWidth, etc.);frameDurationwidensgetFrameTimetof64;requestQuitlatches a flag thatwindowShouldCloseORs in (raylib has no native programmatic close — additive, no behavior change unless called). RenamedinitWindow'swidth/heightparams to*_pxso they no longer shadow the new module-level decls.nullfixturebackend —width/heightreturn theinitWindowdims,frameDurationis1/60,requestQuitis a no-op.Safe by construction
Additive only — the legacy names stay (the generated raylib/null run-loop templates still call them), so generated output is byte-identical and the golden suites are unchanged. This is the prerequisite for (a) sharing one canonical run-loop template across backends and (b) extending the #386 Phase-6b contract guard to built-ins.
Deferred (honest scope — separate slices)
getScreenWidth/getFrameTimearen't there) — conforming them is real per-backend work (query SDL window size / wgpu surface dims + frame timing), not aliases.comptime core.assertWindow(@This())self-assertion per backend: needs alabelle-coredependency on each backend module → the known CI-only-reproducing core-unification (overrideImport) path. Kept out of this low-risk change.Verification
zig build testgreen, incl. a newcanonical window contracttest.window.zig(the param-shadow error this surfaced is fixed). The remainingDecodedImage.compressederror is pre-existing local-pin drift (reproduces on cleanmainwith this PR stashed) — CI, with correct pins, builds the raylib example end-to-end.zig build testgreen.Summary by CodeRabbit
New Features
width,height,frameDuration, andrequestQuit.Bug Fixes
Tests