Dual-language ShaderEffect (GLSL + WGSL) and post effects on the WebGPU renderer — 20.0.0 - #1563
Open
obiot wants to merge 24 commits into
Open
Dual-language ShaderEffect (GLSL + WGSL) and post effects on the WebGPU renderer — 20.0.0#1563obiot wants to merge 24 commits into
obiot wants to merge 24 commits into
Conversation
Checkpoint 1 of the dual-language effect arc: ShaderEffect and the 19 built-in effect classes move from src/video/webgl/ to src/video/effects/, and the pure GLSL source assembly (builtin parsing + vertex/fragment boilerplate) is extracted verbatim into glsl_realization.js so the class can dispatch per backend in the next step. Zero behavior change, proven two ways: every existing shader/effect spec passes unmodified, and a new generated-GLSL golden spec pins the assembled sources byte-identical across the move (snapshots generated against the pre-refactor assembly). Only mechanical path updates outside the moved files (index.ts, loader parser, drawLight import, camera2d, one dynamic-import path in lights.spec). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Checkpoint 2 of the dual-language effect arc. ShaderEffect's body
argument now accepts `{glsl, wgsl}` alongside the historical GLSL string
(which keeps meaning GLSL everywhere); the constructor picks the body
matching `renderer.shaderLanguage` and generalizes the Canvas inert-stub
contract to any missing language: warn once, `enabled = false`, every
method no-ops.
The WGSL side (WebGPU has no uniform reflection, and preloaded assets
are pure text, so everything derives from the source):
- wgsl/parse.js — declaration-only parser: `fn apply`, one uniform
struct at @group(3) @binding(0) whose members ARE the setUniform
names, texture/sampler pairs at explicit consecutive bindings,
builtins activated on reference; every malformed shape refuses with
a reason (warn + inert, never a guessed offset)
- wgsl/layout.js — uniform-address-space offset calculator (vec3
align-16, 16-multiple array strides, struct tail rounding)
- wgsl/scaffold.js — deterministic module assembly around the verbatim
body: frozen quad vertex layout, clip-z remap, .bgr premultiply,
y-down screen_uv, builtin bindings assigned above user bindings
- wgsl_realization.js — CPU uniform mirror + values map (clone replay,
device-loss-proof); GPU objects deferred to the renderer's effect path
- pipeline cache: registerShader (module-text dedup → shared pipelines
for clones), signature-cached effect layouts, shared empty group for
the reserved lights slot, and a device epoch for lazy invalidation
VignetteEffect carries the first WGSL twin as the reference body (its
GLSL string is byte-identical — the golden spec proves it).
61 new unit tests: layout offsets hand-computed, every parser refusal,
scaffold snapshots + frozen-convention assertions, the full dispatch
matrix on mock renderers, mirror byte placement, clone/destroy, and the
pipeline-cache registration surfaces.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
…me capture
Checkpoint 3 of the dual-language effect arc — the offscreen foundations
the post-effect chain builds on:
- WebGPURenderTarget: color texture in the canvas format (renderable +
sampleable + copyable), generation-keyed lazy material bind group for
blits, deferred clear via colorLoadOp, async readPixels() (sync
getImageData throws with guidance). The depth-stencil attachment is
SHARED (renderer-owned, sized per pass) — targets in the 2D flow are
canvas-sized, and sharing the stencil means a surrounding mask keeps
clipping offscreen content
- pass-target parametrization: setRenderTarget(target, {clear}) is the
retarget primitive (flush + pass break; next pass opens on the
target's view with an optional clearing load); beginPass/viewport/
scissor clamp against the active target's size; clear()/abandonFrame
always return to the canvas
- retireTexture() centralizes mid-frame texture disposal (recorded
draws must outlive their resources until submit) — the texture store,
targets, depth recreation and capture all route through it
- captureFrame(): encoder-ordered copyTextureToTexture of the active
destination into the shared WebGPUFrameTexture (the screen_texture
builtin's backing); canvas configured with COPY_SRC usage; capture is
copy-only so the next pass samples it hazard-free
Unit specs: target lifecycle incl. mid-frame retire semantics,
generation-keyed bind groups, pool composition with the WebGPU factory,
capture reallocation. Full suite green; WebGPU platformer re-verified
headed after the hot-path changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
…bGPU Checkpoint 4 of the dual-language effect arc: beginPostEffect / endPostEffect / blitEffect on the WebGPU renderer, mirroring the WebGL control flow on the recording model — pooled offscreen capture (camera clears with background color + fresh stencil, sprites clear transparent), screen_texture capture points (camera-before / sprite- after retarget), camera-viewport scissor bounding, ping-pong chains, final blit with keepBlend semantics, per-depth projection stack restored through fresh frame-globals slots. The effect draw itself (effect_binding.js): device state built lazily per pipeline-cache epoch (module registered once per body text — clones share pipelines), group-3 layout from the parsed shape, and uniform values SNAPSHOT-PER-BIND into a dedicated dynamic-offset arena — the uniform twin of the vertex arena's queue-write-before-draws rationale, so a shared effect bound twice per frame with different values stays correct. Declared-but-unset textures and never-captured screen_texture bind a 1×1 stub so bind groups stay valid; an effect without a WGSL realization composites as a plain blit (content never lost). quad batcher blitTexture records the screen-space quad with UNFLIPPED UVs (WebGPU texture row 0 is the top — the GL flip exists because GL FBOs are bottom-up; apply()'s uv orientation matches across backends). Headed gate: the platformer minimap renders VIGNETTED under WebGPU — closing the phase-1 known degradation. 11 new mock-renderer tests pin the snapshot offsets, epoch rebuilds, stub/capture keying and the blit's pipeline + bind-group recording. Full suite 5351 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Checkpoint 5 of the dual-language effect arc. A single enabled effect on a non-managed renderable draws the sprite's own quad through the effect's pipeline — live compositing against the backdrop with blending KEPT, the semantic that distinguishes the fast path from the pooled blit (apply()'s uv is the sprite's atlas region; discard/edge effects composite differently through a target). WebGPUQuadBatcher adopts renderer.customShader like a material: pending vertices drain under THEIR pipeline before the state changes, and each effect sprite is its own draw with its own uniform snapshot, per-sprite noise_uv frame rect (min-normalized UVs, GL parity) and — for screen_texture effects — a fresh backdrop capture before the draw. Mock-renderer tests pin the adoption drain, per-quad draws with per-draw snapshots, capture-before-draw ordering, the ME rect values, and the return to plain batching when customShader clears. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Checkpoint 6 of the dual-language effect arc: every built-in effect now carries a WGSL body beside its GLSL one (same logic, same uniform names — one setUniform serves both backends). Desaturate/Invert/Sepia inherit ColorMatrix's twin; RadialGradientEffect stays GLSL-only (drawLight is WebGL-internal; lights on WebGPU are a later phase). Porting notes encoded in the bodies: - new `vColor` builtin (reference-gated like the others): the interpolated tint under its GLSL varying name, for bodies that re-sample the source texture (blur, chromatic, pixelate, wave, glow, outline, drop shadow) - conditional/post-return sampling uses textureSampleLevel(…, 0.0) (WGSL uniform-control-flow rule; sprites are single-level textures so output is identical) - `discard` ports as-is (dissolve, scanline); swizzle-assignment and ternaries become vec4f reconstruction and select() The GLSL literals were wrapped, not touched — the generated-GLSL golden stays byte-identical. Headed gate: the shader-effects showcase renders side-by-side matching between WebGL and WebGPU across all 15 per-sprite effects + the viewport vignette, zero console/validation errors on a real adapter. A device-gated spec compiles every twin's scaffolded module via getCompilationInfo (skips visibly without WebGPU). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Checkpoint 7 of the dual-language effect arc:
- shader assets accept the dual shape — src: {glsl: url, wgsl: url} or
inline via data, either language omittable — fetched with the
established pair Promise.all pattern and compiled at load time into a
shared ShaderEffect carrying both bodies. A body for a language the
active renderer doesn't speak never fails the load: the preload
succeeds with the inert stub (the Canvas-fallback contract), so mixed
manifests stay portable across backends
- the ShaderEffect class JSDoc documents the full dual-body contract
and WGSL authoring convention (apply signature, the group-3 uniform
struct whose member names are the setUniform names, texture/sampler
pairs, builtins, the textureSampleLevel porting note)
- CHANGELOG: effects-on-WebGPU entry; the WebGPU renderer entry's
coverage claims updated (full 2D contract; post effects no longer in
the not-yet list)
New loader specs (existing describes untouched): dual inline + dual URL
fetch on the WebGL renderer, and the wgsl-only-on-GLSL inert-stub
mirror case with safe unload.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes ShaderEffect backend-neutral by supporting dual-language effect bodies (GLSL + WGSL) and implements the full post-effect pipeline on the WebGPU renderer, including frame capture (screen_texture) and multi-pass ping-pong composition.
Changes:
- Moved/centralized shader effects under
src/video/effects/and introduced WGSL realization (parser, layout calculator, scaffold/module builder). - Added WebGPU post-effect rendering: offscreen render targets + pooling, encoder-ordered frame capture, effect uniform snapshot arena, and effect binding/pipeline caching.
- Added extensive Vitest coverage and golden snapshots to pin GLSL/WGSL assembly and WebGPU effect behavior.
Reviewed changes
Copilot reviewed 50 out of 53 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/melonjs/tests/wgsl_scaffold.spec.js | Tests WGSL scaffold/module assembly + frozen conventions. |
| packages/melonjs/tests/wgsl_parse.spec.js | Tests WGSL declaration-only body parser accept/refuse cases. |
| packages/melonjs/tests/wgsl_layout.spec.js | Tests WGSL uniform layout/offset calculator rules. |
| packages/melonjs/tests/webgpu_texture_store.spec.js | Updates mock renderer retire behavior for recording model. |
| packages/melonjs/tests/webgpu_render_target.spec.js | Adds WebGPURenderTarget + capture lifecycle/pool composition tests. |
| packages/melonjs/tests/webgpu_post_effect.spec.js | Adds WebGPU effect binding + blit + fast-path behavior tests. |
| packages/melonjs/tests/webgpu_pipeline.spec.js | Extends pipeline cache tests for registered shader families/layout caching. |
| packages/melonjs/tests/webgpu_effects_validate.spec.js | Device-gated compilation validation for built-in WGSL effect twins. |
| packages/melonjs/tests/shadereffect_dual_body.spec.js | Tests dual-language ShaderEffect dispatch + inert stub behavior + WGSL mirror. |
| packages/melonjs/tests/shader-loader.spec.js | Tests dual-language shader assets and inert preload behavior. |
| packages/melonjs/tests/lights.spec.js | Updates RadialGradientEffect import path after effects move. |
| packages/melonjs/tests/helpers/webgpu-mock-renderer.js | Enhances WebGPU mock to support effect binding/capture/layout caching tests. |
| packages/melonjs/tests/effects_golden_glsl.spec.js | Adds golden GLSL snapshots to pin byte-identical assembly. |
| packages/melonjs/tests/snapshots/wgsl_scaffold.spec.js.snap | Snapshots for WGSL scaffolded module text. |
| packages/melonjs/tests/snapshots/effects_golden_glsl.spec.js.snap | Snapshots for generated GLSL sources. |
| packages/melonjs/src/video/webgpu/webgpu_renderer.js | Implements WebGPU render targets, captureFrame, post-effect begin/end, and effect uniform arena integration. |
| packages/melonjs/src/video/webgpu/texture/store.js | Routes texture retirement through renderer-level retire API. |
| packages/melonjs/src/video/webgpu/texture/frametexture.js | Introduces WebGPU frame capture texture for screen_texture. |
| packages/melonjs/src/video/webgpu/pipeline/cache.js | Adds registered shader family support, effect-layout caching, empty group-2, and epoch tracking. |
| packages/melonjs/src/video/webgpu/effect_binding.js | Implements WGSL effect bind-group creation + per-bind uniform snapshotting. |
| packages/melonjs/src/video/webgpu/buffer/arena.js | Adds alignment support to arena allocations for dynamic uniform offsets. |
| packages/melonjs/src/video/webgpu/batchers/quad_batcher.js | Adds WebGPU customShader fast path and effect-aware blitting. |
| packages/melonjs/src/video/webgl/webgl_renderer.js | Updates RadialGradientEffect import to new backend-neutral effects location. |
| packages/melonjs/src/video/rendertarget/webgpurendertarget.js | Adds WebGPU offscreen render target implementation for post effects. |
| packages/melonjs/src/video/effects/wgsl/scaffold.js | Builds the full WGSL module scaffold around an effect body. |
| packages/melonjs/src/video/effects/wgsl/parse.js | Parses WGSL declarations for uniforms/textures/builtins. |
| packages/melonjs/src/video/effects/wgsl/layout.js | Computes WGSL uniform buffer member offsets/sizes. |
| packages/melonjs/src/video/effects/wgsl_realization.js | Represents a parsed/assembled WGSL effect realization + CPU mirrors. |
| packages/melonjs/src/video/effects/wave.js | Converts WaveEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/vignette.js | Converts VignetteEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/tintPulse.js | Converts TintPulseEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/shine.js | Converts ShineEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/shadereffect.js | Adds dual-language body support + WGSL realization dispatch + inert stub behavior. |
| packages/melonjs/src/video/effects/sepia.js | Adds SepiaEffect (ColorMatrix-based) under backend-neutral effects. |
| packages/melonjs/src/video/effects/scanline.js | Converts ScanlineEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/radialGradient.js | Updates RadialGradientEffect import paths to new effects location. |
| packages/melonjs/src/video/effects/pixelate.js | Converts PixelateEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/outline.js | Converts OutlineEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/invert.js | Adds InvertEffect (ColorMatrix-based) under backend-neutral effects. |
| packages/melonjs/src/video/effects/hologram.js | Converts HologramEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/glsl_realization.js | Extracts GLSL source assembly into a dedicated realization module. |
| packages/melonjs/src/video/effects/glow.js | Converts GlowEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/flash.js | Converts FlashEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/dropShadow.js | Converts DropShadowEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/dissolve.js | Converts DissolveEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/desaturate.js | Adds DesaturateEffect (ColorMatrix-based) under backend-neutral effects. |
| packages/melonjs/src/video/effects/colorMatrix.js | Converts ColorMatrixEffect to dual GLSL/WGSL body and updates imports. |
| packages/melonjs/src/video/effects/chromaticAberration.js | Converts ChromaticAberrationEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/video/effects/blur.js | Converts BlurEffect to dual GLSL/WGSL body. |
| packages/melonjs/src/loader/parsers/shader.js | Adds support for {glsl, wgsl} shader assets (inline + URL) with inert fallback. |
| packages/melonjs/src/index.ts | Re-exports effects from backend-neutral video/effects/ tree. |
| packages/melonjs/src/camera/camera2d.ts | Updates ColorMatrixEffect import to backend-neutral effects location. |
| packages/melonjs/CHANGELOG.md | Documents dual-language ShaderEffect + WebGPU post effects in 20.0.0 notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+107
| signatureParts.push(`st${bindings.screenTexture}`); | ||
| if (builtins.screenSamplerClamp) { | ||
| entries.push({ | ||
| binding: bindings.screenSamplerClamp, | ||
| visibility: GPUShaderStage.FRAGMENT, | ||
| sampler: {}, | ||
| }); | ||
| } | ||
| if (builtins.screenSamplerRepeat) { | ||
| entries.push({ | ||
| binding: bindings.screenSamplerRepeat, | ||
| visibility: GPUShaderStage.FRAGMENT, | ||
| sampler: {}, | ||
| }); | ||
| signatureParts.push("sr"); | ||
| } |
Comment on lines
+188
to
+200
| const builtins = { | ||
| screenTexture, | ||
| screenSamplerClamp: /\bscreen_sampler\b/.test(source), | ||
| screenSamplerRepeat: /\bscreen_sampler_repeat\b/.test(source), | ||
| screenUV: | ||
| (screenTexture || /\bscreen_uv\b/.test(source)) && | ||
| !hasOwnDeclaration(source, "screen_uv"), | ||
| noiseUV: | ||
| /\bnoise_uv\b/.test(source) && !hasOwnDeclaration(source, "noise_uv"), | ||
| // the interpolated tint (the GLSL `vColor` varying) — bodies that | ||
| // re-sample the source texture reference it to re-apply the tint | ||
| vColor: /\bvColor\b/.test(source) && !hasOwnDeclaration(source, "vColor"), | ||
| }; |
Comment on lines
+648
to
+650
| capture?.destroy(); | ||
| capture = new WebGPUFrameTexture(this, width, height); | ||
| this.captureTexture = capture; |
…y, not backend type The main-viewport VignetteEffect was guarded by `renderer instanceof WebGLRenderer` — the backend-type check the 20.0.0 capability flags replace — so under the WebGPU renderer the full-screen vignette was silently never attached (the visible WebGL/WebGPU difference: dark corners on one, none on the other). Gate on `renderer.shaderLanguage !== null` instead: Canvas still skips, both GPU backends attach, and the platformer renders identically under WebGL and WebGPU (minimap close-ups verified pixel-equivalent). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 54 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/melonjs/src/video/effects/wgsl/parse.js:191
- WGSL builtin detection should honor the “owns-declaration guard” for
screen_samplerandscreen_sampler_repeat. As written, any user-defined local/global identifier namedscreen_sampler(orscreen_sampler_repeat) will be treated as a builtin reference, which can cause unintended scaffold declarations / binding expectations. This is inconsistent with the existing guards onscreen_texture,screen_uv,noise_uv, andvColor.
screenSamplerClamp: /\bscreen_sampler\b/.test(source),
screenSamplerRepeat: /\bscreen_sampler_repeat\b/.test(source),
packages/melonjs/src/video/webgpu/effect_binding.js:96
- The effect-layout cache signature does not encode whether
screen_sampler(clamp) is present. If a body references bothscreen_samplerandscreen_sampler_repeat, its layout entries differ from a body referencing onlyscreen_sampler_repeat, but both currently produce the same signature (...|st<binding>|sr). That can causegetEffectLayout()to return a layout missing the clamp sampler binding, leading to bind-group creation/validation failures for the “both samplers” case.
CodeQL flagged the lazy [\s\S]*? block-comment pattern as polynomially backtracking on pathological inputs (js/polynomial-redos). Replaced with the classic linear-time form; identical matching behavior, comment-handling specs unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 54 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/melonjs/src/video/webgpu/effect_binding.js:96
buildEffectGPUcaches effect bind-group layouts bysignatureParts.join("|"), but the signature currently does not encode whetherscreen_sampler(clamp) is present. If an effect references bothscreen_samplerandscreen_sampler_repeat, its signature includes only"sr", which collides with an effect that references ONLYscreen_sampler_repeat. Those two shapes produce differententriesarrays, so the cached layout can be wrong and lead to validation errors or incorrect bindings.
Include clamp presence (and ideally the binding indices) in the signature so each distinct group-3 layout gets its own cache entry.
packages/melonjs/src/video/webgpu/webgpu_renderer.js:2150
- In
restoreDevice(), device-scoped post-effect textures are nulled (captureTexture,stubTexture,depthTexture) without being destroyed/retired first. Even though the device is being replaced, explicitly destroying these GPU resources matches thedestroy()path and avoids leaking/retaining old GPUTexture objects across device-loss recovery.
CodeQL's polynomial-redos check also flags the classic linear-time block-comment regex form. Comment stripping is now a plain single-pass character scanner — provably linear, identical output, parser specs unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
…k at all Follow-up to the capability-flag guard: no guard is needed in the first place. ShaderEffect self-disables on a renderer without a programmable pipeline (warn once, enabled = false, scene renders without it), so the example simply attaches the effect unconditionally — the code a user should write. Verified headed on WebGL, WebGPU (vignetted) and Canvas (clean, un-vignetted). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 54 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/melonjs/src/video/webgpu/webgpu_renderer.js:651
- When the shared frame capture is reallocated (size change),
capture.generationis not bumped. Effect bind groups key oncapture.generation, so they may incorrectly reuse a bind group that still points at the old (retired/destroyed) capture view, leading to invalid sampling or validation errors after resize.
packages/melonjs/src/video/webgpu/effect_binding.js:96 - The effect-layout cache signature is not unique for different
screen_texturesampler shapes: repeat-only and clamp+repeat both push just"sr", so they can collide and reuse an incompatible group-3 bind-group layout. Include the sampler binding(s) (and clamp presence) insignaturePartsto guarantee uniqueness per layout entry set.
packages/melonjs/src/video/effects/wgsl/parse.js:220 screen_sampler/screen_sampler_repeatbuiltin detection doesn’t apply the owns-declaration guard. If a body declares a local/let/const with the same name, the parser can incorrectly activate the builtin and the scaffold will inject duplicate identifiers, causing WGSL compilation to fail. ApplyhasOwnDeclarationto these sampler builtins just likescreen_uv,noise_uv, andscreen_texture.
const builtins = {
screenTexture,
screenSamplerClamp: /\bscreen_sampler\b/.test(source),
screenSamplerRepeat: /\bscreen_sampler_repeat\b/.test(source),
screenUV:
packages/melonjs/src/video/webgpu/webgpu_renderer.js:537
setRenderTargetsupportsoptions.clearValueandoptions.clearStencil, but the JSDoc only documentsoptions.clear. Please document the additional options so callers know how to request a specific clear color and stencil clear.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 54 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/melonjs/src/video/effects/wgsl/parse.js:220
- The builtin sampler flags (
screen_sampler/screen_sampler_repeat) don’t honor the “owns-declaration guard”. If a body declares its ownscreen_sampleridentifier, the parser still treats it as a builtin and the scaffold/bind-group builder may inject bindings the user didn’t intend, potentially colliding with user-managed resources.
const builtins = {
screenTexture,
screenSamplerClamp: /\bscreen_sampler\b/.test(source),
screenSamplerRepeat: /\bscreen_sampler_repeat\b/.test(source),
screenUV:
Comment on lines
+17
to
+35
| export class WebGPUFrameTexture extends Texture2d { | ||
| /** | ||
| * @param {import("../webgpu_renderer.js").default} renderer - the owning renderer | ||
| * @param {number} width - capture width in pixels | ||
| * @param {number} height - capture height in pixels | ||
| */ | ||
| constructor(renderer, width, height) { | ||
| super(); | ||
| this.renderer = renderer; | ||
| /** @type {number} */ | ||
| this.width = width; | ||
| /** @type {number} */ | ||
| this.height = height; | ||
| /** | ||
| * bumped on every (re)allocation — bind groups referencing `view` | ||
| * cache against this | ||
| * @type {number} | ||
| */ | ||
| this.generation = 0; |
Second review pass over the dual-language effect arc — eight verified findings fixed, each with a regression test: - CRITICAL: WebGPUFrameTexture.generation was never advanced, so a canvas resize left every screen_texture bind group pointing at the destroyed capture — permanent whole-frame validation failure. Each capture instance now draws from a monotonic counter (and the test mock no longer models behavior the real class lacked) - pass restarts re-apply the stencil reference: content masked ACROSS a post-effect retarget or capture tested against level 0 (i.e. drew only OUTSIDE its mask) — beginPass now re-records maskVisibleRef - the clamp screen-sampler contributed no effect-layout signature token, so clamp-only and clamp+repeat shapes shared one cached layout (bind groups mismatched whichever registered second) - WGSL compilation failures now disable the effect asynchronously via getCompilationInfo (warn + enabled=false) instead of invalidating every subsequent submit — a parse-clean body with a bad expression black-screened forever - struct members split on top-level commas only: array<vec4f, N> — advertised and layout-supported — could never parse - mat3x3f values place per vec4-strided column (9-float column-major input); previously columns 2-3 read scrambled, diverging from GLSL - WGSL block comments nest — the scanner now tracks depth - effect destroy retires its resident setTexture textures; booleans normalize to 0/1; readPixels clamps its window and frees the staging buffer on rejection; clear() unwinds effectPassDepth (exception hygiene); capture generation only keys screen_texture consumers New webgpu_post_effect_flow.spec.js pins the pooled control flow's ordering laws through the REAL begin/endPostEffect bodies over recorded primitives: camera capture-before-retarget vs sprite capture-after, ping-pong clears, viewport clip bracket, keepBlend semantics, nested projection-slot restore, fast-path/filtering short-circuits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 70 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
packages/melonjs/src/video/webgpu/renderers/tmxlayer/orthogonal.js:150
queue.writeTexturerequiresbytesPerRowalignment (256-byte multiple) whenrowsPerImage/height > 1. Usinglayer.cols * 4will fail WebGPU validation for most layer widths on real devices (e.g. 8 cols → 32 bytes/row). Pad each row to an alignedbytesPerRowbefore callingwriteTexture(and use the correct Uint8Array view overlayer.layerData).
packages/melonjs/src/video/webgpu/effect_binding.js:307bindKeyincludesgpu.residentTextures.size, but that size can change during the same stale rebuild (whenresidentTexture()lazily creates the first resident texture). This causes an unnecessary extra bind-group rebuild on the nextprepareEffectBinding()(and after device-loss rebuilds). SinceShaderEffect.setTexture()already callswgslRealization.gpu?.invalidateBindGroup?.()for static textures, and live textures are re-keyed by generation below, thisresidentTextures.sizecomponent can be dropped.
Comment on lines
+212
to
+220
| if (dirty) { | ||
| device.queue.writeTexture( | ||
| { texture: entry.texture }, | ||
| data, | ||
| { bytesPerRow: entry.tileCount * 4, rowsPerImage: 1 }, | ||
| [entry.tileCount, 1], | ||
| ); | ||
| entry.uploaded = true; | ||
| } |
The pond effect was GLSL-only, so the WebGPU renderer disabled it and
drew flat water. The dual {glsl, wgsl} body brings the full
screen_texture / screen_uv / noise_uv refraction to WebGPU, matching
the WebGL rendering.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 71 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/melonjs/src/video/webgpu/renderers/tmxlayer/orthogonal.js:151
queue.writeTexturerequiresbytesPerRowto be 256-byte aligned when writing more than one row.layer.cols * 4will fail validation for most map widths, so the TMX index upload can break on real WebGPU devices. Pad each row into an aligned staging buffer (or otherwise ensure alignment) before callingwriteTexture.
packages/melonjs/src/video/webgpu/effect_binding.js:39- The JSDoc for
buildEffectGPUis missing theeffectparameter (the function signature is(renderer, effect, realization)). This makes the docs/types misleading for readers and tooling.
Port the GL #gradientMask machinery as two new pipeline stencil variants: "tag" stamps the shape's pixels with the dynamic stencil reference on a cleared stencil (always/replace — overdraw-immune, color writes off), and "mark" writes the reference only where the stencil's low 7 bits already match, so the high-bit marker can tag and untag visible pixels inside an active mask without disturbing mask levels. fillArc/fillEllipse/fillPolygon/fillRoundRect now clip the Canvas-baked gradient rect to the shape instead of falling back to a solid fill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 72 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/melonjs/src/video/effects/wgsl/parse.js:255
screen_sampler/screen_sampler_repeatare treated as builtins even whenscreen_textureisn’t used, and they don’t honor the “own declaration” guard. As a result, a body that referencesscreen_sampler*withoutscreen_texturewill parse as ok but the scaffold won’t declare/bind those samplers, leading to a guaranteed WGSL compilation failure; likewise, user-declaredscreen_sampler*names can incorrectly activate the builtin path and create duplicate declarations.
packages/melonjs/src/video/webgpu/renderers/tmxlayer/orthogonal.js:13- The TMXUniforms size breakdown in this comment is inaccurate (the WGSL struct has 9×vec2f, not 10×vec2f). The constant value (112) matches the actual uniform-layout rules, but the incorrect breakdown can mislead future changes.
Generalize the frame-capture machinery into the public toFrameTexture
contract: shared renderer-owned slot by default, `target: null` for a
caller-owned capture, a prior capture as target to refresh it in place
(WebGPUFrameTexture gains an identity-preserving realloc that retires
the old texture and advances the bind-group generation), and region
capture with the same clamp rules as the GL backend — the public
bottom-left origin converted to the copy's top-left one. captureFrame
now delegates to it. Two documented divergences: alpha is preserved,
and row 0 of the capture is the top of the frame — GLSL bodies flip
with `1.0 - uv.y`, WGSL twins must not.
The aquarium and heat-haze examples gain WGSL twins for their capture
shaders through the dual {glsl, wgsl} asset shape, closing their
black-screen gap under WebGPU.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Port the complete Light2d pipeline. The pure-CPU lighting math (packLights + the published std140 block) is shared with the GL backend; everything GPU-side is new: - WebGPULitQuadBatcher: normal-mapped sprites shaded by the std140 Light2dBlock at the reserved group 2. Single-texture-per-segment model — group 1 is a combined color+normal material, so the GL sampler ladder and per-vertex normal-texture id are gone and the frozen 28-byte quad layout is unchanged. Each setLightUniforms call snapshots the packed block into the effect uniform arena with a dynamic offset (queue writes execute before every recorded draw, so a shared region per camera would be retroactively clobbered). Normal maps are resident textures keyed by source, re-uploaded on the duck-typed version stamp, never premultiplied. - quad-lit.wgsl: the multitexture-lit port — same quadratic attenuation, Y-flipped normal decode and ambient floor. - drawLight rides the single-effect fast path with a now dual-language RadialGradientEffect (WGSL twin added), color+intensity packed into the per-vertex tint; the ambient-overlay cutouts already worked via the stencil mask machinery. - drawImage gates onto the lit batcher exactly like the GL backend (lit scene + normal map present), so unlit sprites pay nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 78 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/melonjs/src/video/effects/wgsl/parse.js:255
- Builtin sampler detection doesn’t honor the owns-declaration guard. If a body declares its own
screen_sampler/screen_sampler_repeatwhile also referencingscreen_texture, the parser will still mark the builtin sampler as used and the scaffold will emit a duplicate declaration, causing WGSL compilation to fail.
packages/melonjs/src/video/effects/wgsl/parse.js:115 hasOwnDeclaration()claims to treat struct members as user declarations, but the implementation only detectsvar/let/constdeclarations. This can cause false-positive builtin detection when a user has a struct member or function parameter named like a builtin (e.g.screen_texture), potentially disabling an otherwise valid effect body.
packages/melonjs/src/video/effects/wgsl/parse.js:31RESOURCE_DECLonly matches the exact spellingtexture_2d<f32>with no whitespace inside the angle brackets. Valid WGSL liketexture_2d< f32 >(or line breaks) will be missed and then rejected later as an “unsupported declaration”.
The renderer now carries its owning application (renderer.parentApplication, stamped by Application.init) so engine code holding a renderer reference never needs the global instance. TextureAtlas registration goes through the VIDEO_INIT-captured renderer's own cache — the same pattern the loader parsers already use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
The loader's dds/ktx/ktx2/pvr/pkm parsers are backend-neutral and emit WebGL format constants; the WebGPU side now consumes them: the renderer requests the texture-compression-bc/etc2/astc device features the adapter offers and reports format families in the same shape as the GL backend (so the shared capability gate and the loader pre-filter work unchanged, ETC1 synthesized from ETC2, PVRTC honestly null — it has no WebGPU equivalent), and the texture store uploads compressed sources through a dedicated createTexture + block-aligned per-mip writeTexture path (compressed formats cannot be render attachments). sRGB variants map for KTX1 files, which carry raw GL internal formats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
The HUD relied on depth = Infinity set in its constructor, which world.addChild overwrites with an auto-assigned z (23) below the grass strips (up to 30) — add it with an explicit z above them. Note: the score text is still not visible pending an engine-side bug with BitmapText nested in floating Containers (draws are recorded with correct coordinates but never reach the screen, on both backends). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
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
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.
What this does
Makes shader effects first-class on both GPU backends.
ShaderEffectand the built-in effects move out of the WebGL tree to a backend-neutral home (src/video/effects/), carry one body per shading language ({ glsl, wgsl }— a plain string keeps meaning GLSL everywhere), and each renderer realizes the description in its own language. When no matching body exists, the effect warns once and stays disabled while the scene keeps rendering — the Canvas contract, generalized.The WebGPU backend gains the full post-effect execution path: pooled offscreen render targets (shared depth-stencil), the camera and multi-effect ping-pong chains,
screen_texture/screen_uv/noise_uvbuiltins backed by encoder-ordered frame captures, and the single-effectcustomShaderfast path. All 18 dual-language built-in effects render matching between WebGL and WebGPU (verified side-by-side on the shader-effects showcase); the platformer minimap's vignette — the known phase-1 degradation — now renders under WebGPU.Backward compatibility (the design constraint)
{vertex, fragment}shader assets and manifests behave identically — a generated-GLSL golden spec pins the assembled sources byte-identical across the refactor.{glsl, wgsl}asset shape is additive; a wrong-language asset preloads as an inert stub instead of failing the load.The WGSL contract (documented on the ShaderEffect class)
WebGPU has no uniform reflection and preloaded assets are pure text, so everything derives from the body: a declaration-only parser reads the one uniform struct at
@group(3) @binding(0)(member names ARE thesetUniformnames — one call serves both backends), texture/sampler pairs, and builtin references; a WGSL-rules layout calculator places every member; malformed bodies warn and disable, never guess offsets. Uniform values are snapshot-per-bind into a dynamic-offset arena — the uniform twin of the vertex arena's queue-write-before-draws rationale — so shared effects bound twice per frame with different values stay correct.Test coverage
~120 new tests: golden GLSL, WGSL layout offsets (hand-computed, vec3-align-16 included), every parser refusal, scaffold snapshots + frozen-convention assertions, the dispatch matrix, mirror byte placement, clone/destroy, effect-binding snapshots/epoch/bind-group keying, render-target lifecycle + pool composition, blit recording, fast-path adoption/capture ordering, loader dual shapes — plus a device-gated spec compiling every built-in twin via
getCompilationInfo(skips visibly without WebGPU).Full suite: 5357 passed / 4 skipped; adapters, debug-plugin and spine build/tests green; headed verification on a real adapter across platformer (both backends), shader-effects (both), and spine.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi