Repository navigation
Move Doom palette expansion to an indexed-texture shader - #9
Conversation
Upload the indexed framebuffer and palette separately so the GPU performs palette lookup while preserving the packed RGBA fallback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe DOOM example now supports indexed-texture shader rendering with an RGBA fallback. Palette tests cover all 14 palettes and 256 entries, build regeneration tracks palette changes, and performance documentation reports updated measurements. ChangesDOOM rendering and palette pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DoomFrame
participant I_FinishUpdate
participant IndexedTexture
participant PaletteTexture
participant Shader
participant RgbaTexture
DoomFrame->>I_FinishUpdate: submit indexed frame
I_FinishUpdate->>IndexedTexture: upload frame indices
I_FinishUpdate->>PaletteTexture: upload changed palette
IndexedTexture->>Shader: provide indexed pixels
PaletteTexture->>Shader: provide palette lookup
Shader-->>I_FinishUpdate: render palette-mapped frame
I_FinishUpdate->>RgbaTexture: expand and upload frame on fallback path
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/doom-gpl/src/i_video.ts`:
- Around line 133-166: Reset uploadedPaletteIndex whenever
initializeIndexedShader creates a new paletteTexture, so the first
I_FinishUpdate after graphics reinitialization uploads the current
packedPaletteBytes. Ensure this reset occurs only after successful texture
creation and preserves the existing palette-index comparison and upload
behavior.
- Around line 212-222: Update both DrawTexture calls in the indexedShaderActive
branches to use WHITE instead of RAYWHITE as the tint, preserving the existing
texture selection and rendering flow.
🪄 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: 6c440cc1-bef2-4f24-8bbb-2121fff93081
📒 Files selected for processing (6)
README.mdbenchmarks/application-performance.mdexamples/doom-gpl/Makefileexamples/doom-gpl/README.mdexamples/doom-gpl/src/i_video.tsexamples/doom-gpl/tests/palette.test.ts
Reset the GPU palette upload marker when a valid palette texture is recreated and use an identity-white texture tint so both display paths retain the framebuffer's exact palette colours.
Summary
its 256-colour palette in a small fragment shader
PLAYPALslot changes, whileretaining the current packed RGBA path as a capability fallback
the controlled current-main performance result
The five-pair steady-state comparison discarded 12 warm-up frames and measured
the next 24 frames per run. On an Apple M1 Max, the packed fallback averaged
4.97 FPS (201.22 ms/frame) and the shader averaged 6.32 FPS
(158.33 ms/frame): 1.27x higher FPS and 21.31% lower whole-frame time.
The frame-8 shader and fallback gameplay captures are byte-identical when only
the live FPS overlay is disabled.
Testing
npm cinpm run generatenpm testnpm run check:generatednpm run test:typesnpm run test:abinpm run test:ffinpm run pack:checkmake -C examples/doom-gpl verify-sourcemake -C examples/doom-gpl test-palettemake -C examples/doom-gpl smokegit diff --checkThe Docker/Xvfb Linux lane was not run locally because no Docker daemon was
available.
Linked issues
None.