Repository navigation
Batch Bunnymark sprites with public raylib instancing - #10
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe Bunnymark implementation now uses compact typed-array state and an optional custom-shader instanced renderer. Runtime configuration supports deterministic benchmarks, screenshots, reporting, and draw-path selection. New tests compare direct and instanced screenshots byte-for-byte, with benchmark documentation and CI integration. ChangesBunnymark instanced rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VisualTest
participant Fixture
participant runBunnymark
participant Renderer
participant PNGFiles
VisualTest->>Fixture: run direct and instanced fixtures
Fixture->>runBunnymark: configure draw_path and capture settings
runBunnymark->>Renderer: render selected Bunnymark path
Renderer-->>runBunnymark: capture configured frame
runBunnymark->>PNGFiles: write screenshot
VisualTest->>PNGFiles: read direct and instanced PNGs
VisualTest->>VisualTest: compare bytes and compute SHA-256
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
frostney
marked this pull request as ready for review
July 24, 2026 13:27
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/integration/bunnymark-visual.test.mjs (1)
40-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the shader-failure fallback path.
These cases validate successful direct and instanced rendering only. Add a controlled fixture or runtime switch that forces instanced shader creation to fail, then assert that the requested instanced run reports
draw_path=directand produces the same screenshot as the direct reference.🤖 Prompt for 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. In `@tests/integration/bunnymark-visual.test.mjs` around lines 40 - 49, The visual integration test currently covers only successful direct and instanced rendering. Extend the fixture or runtime configuration used by bunnymark-instanced-visual to deliberately fail instanced shader creation, then verify the requested instanced execution reports draw_path=direct and its screenshot matches the direct reference, while preserving the existing successful comparison.examples/lib/bunny-instancing.ts (1)
73-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-obvious mesh construction could use a short comment.
Building the quad via
GenMeshHeightmap+ fullUpdateMeshBufferoverwrite works but isn't self-explanatory; a one-line comment on why this API is (ab)used to obtain a ready 6-vertex mesh would help future maintainers.🤖 Prompt for 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. In `@examples/lib/bunny-instancing.ts` around lines 73 - 80, Add a concise comment immediately before the GenMeshHeightmap call explaining that it is used to create a ready 6-vertex quad mesh whose vertex and texture-coordinate buffers are fully overwritten by the subsequent UpdateMeshBuffer calls.
🤖 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 `@examples/lib/bunny-instancing.ts`:
- Around line 21-33: The vertex shader in examples/lib/bunny-instancing.ts lines
21-33 must extract packed transform data correctly: update fragColor to use the
.x component of each transform column and compute position from the .w
components of columns 0 and 1. Then re-run the direct-versus-instanced
visual-equivalence test and update or re-confirm the reported SHA-256 checksums
in benchmarks/bunnymark-instancing.md lines 41-57.
In `@tests/integration/bunnymark-visual.test.mjs`:
- Around line 43-49: Validate the buffers returned by readFile for directPath
and instancedPath before the assert.deepEqual comparison, ensuring each is a
non-empty, valid PNG capture. Reject either capture when validation fails, then
preserve the existing pixel-buffer equality assertion for valid files.
---
Nitpick comments:
In `@examples/lib/bunny-instancing.ts`:
- Around line 73-80: Add a concise comment immediately before the
GenMeshHeightmap call explaining that it is used to create a ready 6-vertex quad
mesh whose vertex and texture-coordinate buffers are fully overwritten by the
subsequent UpdateMeshBuffer calls.
In `@tests/integration/bunnymark-visual.test.mjs`:
- Around line 40-49: The visual integration test currently covers only
successful direct and instanced rendering. Extend the fixture or runtime
configuration used by bunnymark-instanced-visual to deliberately fail instanced
shader creation, then verify the requested instanced execution reports
draw_path=direct and its screenshot matches the direct reference, while
preserving the existing successful comparison.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c653a78-efaa-427e-b7ed-bc63d2958b8e
📒 Files selected for processing (16)
.github/workflows/ci.ymlREADME.mdbenchmarks/bunnymark-instancing.mdexamples/lib/bunny-instancing.tsexamples/lib/bunny-state.tsexamples/lib/run-bunnymark.tspackage.jsonscripts/validate-package.mjstests/fixtures/bunnymark-benchmark.tstests/fixtures/bunnymark-direct-visual.tstests/fixtures/bunnymark-instanced-visual.tstests/fixtures/bunnymark-visual.tstests/integration/bunnymark-visual.test.mjstests/linux/Dockerfiletests/unit/bunny-state.test.tstests/unit/syntax.test.ts
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.
Summary
DrawTextureVcrossings with one public-raylibDrawMeshInstancedsubmission wrapped byBeginShaderMode/EndShaderModeMatrixinstance layout in compact typed arraysDrawTextureVfallback if the desktop shader cannot be createdNo companion C/native bridge is included, and this does not change the GocciaScript runtime, generated binding surface, or WASM work.
Why
Raylib already batches GPU quads internally, but the Bunnymark example still crossed GocciaScript's FFI boundary once per sprite per frame. The existing Raylib 6 bindings already expose
DrawMeshInstanced, shader loading, mesh updates, and material APIs, so the example can submit the whole sprite set through public Raylib calls.Performance
Five alternating-order pairs used the production GocciaScript import-binding fast path at
3b19fdbe, seed1234, 10,000 sprites, an uncapped target, and 25 frames per run:origin/maindirect path: 1925.2 ms / 12.99 FPS meanThe complete method, individual trials, and tradeoffs are recorded in
benchmarks/bunnymark-instancing.md.Validation
npm run generatenpm test- interpreter and bytecode, 10 tests / 90 assertions eachnpm run check:generatednpm run test:typesnpm run test:abi- all 35 native layoutsnpm run test:ffinpm run pack:check- exact 22-file allowlistnpm run test:bunnymark:visual- valid, byte-identical direct/instanced PNG captures on macOStests/linux/Dockerfilegate on Linux/arm64 with Xvfb/Mesa, including byte-identical visual proof and existing DOOM smokeorigin/mainscreenshot also byte-matches the macOS instanced imagegit diff --checkReview status
Current automated findings were independently evaluated. Capture integrity validation is included in
650cf2f; the packedMatrixshader mapping was revalidated against Raylib'sMatrixToFloatVimplementation and retained.