From fecf11f7502c6e316a33f1af87f9831353860742 Mon Sep 17 00:00:00 2001 From: Olivier Biot Date: Sat, 3 Oct 2026 15:42:27 +0800 Subject: [PATCH] Effects: a single post effect reaches a renderable that draws with primitives One effect is applied by drawing the renderable with the effect's own program instead of capturing it offscreen, which costs no render target. That is equivalent only when everything the renderable draws is a textured quad: fillRect and the shape dispatch go to a batcher that never reads customShader, so ONE effect silently did nothing while two worked, because a chain always captures. Measured with a DesaturateEffect over pure red, one effect read back [255, 0, 0] untouched and two read [76, 76, 76]. The predicate deciding that path was written out inline at both ends of both GPU backends, and the two ends have to agree: a begin that opens a render target an end then declines to resolve draws the renderable into a buffer nobody reads. It is now one method on the base Renderer, asked from all five sites. Renderable#postEffectNeedsCapture opts in, defaults to false so nothing existing moves, and is set on ProgressBar and on Trail, which was affected and is fixed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGvtaUNATVCVxD2qcbiY4t --- packages/melonjs/CHANGELOG.md | 1 + .../melonjs-effects-and-shaders/SKILL.md | 30 ++- packages/melonjs/src/renderable/renderable.js | 31 +++ packages/melonjs/src/renderable/trail.js | 6 + .../melonjs/src/renderable/ui/progressbar.ts | 6 + packages/melonjs/src/video/renderer.js | 32 ++- .../melonjs/src/video/webgl/webgl_renderer.js | 4 +- .../src/video/webgpu/webgpu_renderer.js | 4 +- .../melonjs/tests/posteffect-fastpath.spec.js | 224 ++++++++++++++++++ .../tests/webgpu_post_effect_flow.spec.js | 52 ++++ 10 files changed, 384 insertions(+), 6 deletions(-) create mode 100644 packages/melonjs/tests/posteffect-fastpath.spec.js diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index 6df6990c4..8a29655ba 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -19,6 +19,7 @@ - `BloomEffect`: the bright parts of a frame bleed light into the pixels around them, which is what makes emitters, neon and specular highlights read as light rather than as bright paint. `threshold`, `intensity` and `radius` are settable live, and it sizes itself from the renderer. One gather pass with a soft knee, so a light fading through the threshold ramps in rather than popping. `GlowEffect` is an outline drawn outside a sprite's silhouette and returns early on an opaque fragment, so it never was the screen bloom people reached for it as ### Fixed +- A single post effect on a renderable that draws with primitives applies, instead of silently doing nothing. One effect is normally applied by drawing the renderable with the effect's own program rather than capturing it offscreen, which costs no render target but only works when everything the renderable draws is a textured quad: `fillRect` and the shape dispatch go to a batcher that never reads that shader. Measured with a `DesaturateEffect` over pure red, one effect read back `[255, 0, 0]` untouched while two read `[76, 76, 76]`, so only the single-effect case was ever wrong. `Trail` was affected and is fixed; a renderable of your own that draws with primitives sets `postEffectNeedsCapture` to opt in, and everything else keeps the cheap path exactly as before. Both GPU backends - Input: a region covered by something that consumed the pointer is told it lost it, instead of being left in its hover state. A widget only ever got its leave by the pointer going outside its own bounds, so a button half covered by a panel stayed lit when the pointer slid off its exposed part and onto the panel, which never takes it out of the button's bounds. A consumed move now carries on down the candidate list, not to offer the event to anything underneath but to take it away from whatever still holds it. A consumed press, release or wheel does not, since none of those says where the pointer is - Input: the pointer hit test asks the renderable that is drawn on top first. `pos.z` is container-local, because `autoDepth` numbers each container's own children from 1, and `Container#draw` never compares across containers: it recurses, so a child's z is only ever weighed against its siblings. The hit test sorted one flat list of broadphase candidates on raw z instead, so a button at local z 8 inside a low panel outranked an entire panel stacked on top of it and **a covered widget answered clicks and lit up on hover right through whatever was drawn over it**. Each pair is now resolved where `draw` resolves it, between the two siblings whose order decides which subtree paints last, with a child ahead of the container holding it and equal sibling z falling back to child order. Ordering between siblings of one container is unchanged, which is every case a game with a single container has - The loading screen's progress bar is the public `ProgressBar`, and the loader subscription moved out of it. It used to call `on(LOADER_PROGRESS, ...)` from its own constructor, which is what kept it private: a renderable that subscribes to the loader can only ever show loading. It also stored its fill as a pixel count rather than a ratio, so a viewport resize part way through a load left the fill at the old scale until the next asset happened to land. Output is unchanged but for the fill's leading edge, which no longer truncates to a whole pixel diff --git a/packages/melonjs/skills/melonjs-effects-and-shaders/SKILL.md b/packages/melonjs/skills/melonjs-effects-and-shaders/SKILL.md index c3f5bd0b3..d6410414d 100644 --- a/packages/melonjs/skills/melonjs-effects-and-shaders/SKILL.md +++ b/packages/melonjs/skills/melonjs-effects-and-shaders/SKILL.md @@ -1,6 +1,6 @@ --- name: melonjs-effects-and-shaders -description: "Use this skill for post-processing effects, custom shaders, blend modes and colour grading in melonJS. Covers the built-in ShaderEffect presets, addPostEffect on renderables and cameras, writing a custom dual-language GLSL/WGSL effect, the screen_texture builtins, and why effects silently do nothing on the Canvas fallback. Triggers on: ShaderEffect, addPostEffect, removePostEffect, getPostEffect, VignetteEffect, GlowEffect, BlurEffect, PixelateEffect, ScanlineEffect, shader, GLSL, WGSL, uniform, setUniform, setTexture, setTime, blendMode, colorMatrix, screen_texture, toFrameTexture, post effect, filter." +description: "Use this skill for post-processing effects, custom shaders, blend modes and colour grading in melonJS. Covers the built-in ShaderEffect presets, addPostEffect on renderables and cameras, writing a custom dual-language GLSL/WGSL effect, the screen_texture builtins, and why effects silently do nothing on the Canvas fallback. Triggers on: ShaderEffect, addPostEffect, removePostEffect, getPostEffect, postEffectNeedsCapture, VignetteEffect, GlowEffect, BlurEffect, PixelateEffect, ScanlineEffect, shader, GLSL, WGSL, uniform, setUniform, setTexture, setTime, blendMode, colorMatrix, screen_texture, toFrameTexture, post effect, filter." license: MIT --- @@ -64,6 +64,33 @@ wants a small depth, not the huge z that would put it on top in 2D). Either way, verify it rather than reasoning about it: return a flat colour from the body for one frame and see what it tints. +## A renderable that draws with PRIMITIVES has to ask to be captured + +One effect is applied the cheap way: the renderable is drawn with the effect's +own program instead of being captured offscreen and post-processed, which costs +no render target. That is equivalent only when everything the renderable draws +is a textured quad. `fillRect`, `strokeRect` and `renderer.fill(shape)` go +through a batcher that never reads that shader, so **one** effect on such a +renderable would silently do nothing, while **two** would work, because a chain +always captures. + +So a renderable of your own that draws with primitives says so: + +```js +class Bar extends Renderable { + constructor(x, y, w, h) { + super(x, y, w, h); + this.postEffectNeedsCapture = true; // ← or one effect is a no-op + } + draw(renderer) { + renderer.fillRect(this.pos.x, this.pos.y, this.width, this.height); + } +} +``` + +`ProgressBar` and `Trail` set it for you, and a sprite needs nothing: the flag +defaults to `false` and only ever changes the single-effect case. + ## Toggle with `enabled`, do not remove **`removePostEffect()` destroys the effect** — it calls `effect.destroy()` and @@ -576,6 +603,7 @@ same question after construction. | effect does nothing, warning in console | Canvas fallback — no programmable pipeline | | effect does nothing on some machines only | GLSL-only shader, `video.AUTO` chose WebGPU | | a white or solid box where the effect should be | carrier renderable drawn while the effect is disabled | +| ONE effect does nothing but two of them work | the renderable draws with primitives — set `postEffectNeedsCapture` | | effect cannot be re-enabled | `removePostEffect()` destroyed it — use `enabled` | | ported shader renders upside down | a hand-bound `toFrameTexture()` capture — GL is bottom-up, WebGPU top-down | | shadow/smear offset flips on some draws | vertical UV offset not multiplied by `uUVYDir` | diff --git a/packages/melonjs/src/renderable/renderable.js b/packages/melonjs/src/renderable/renderable.js index 4cc8e23d8..b877e9c04 100644 --- a/packages/melonjs/src/renderable/renderable.js +++ b/packages/melonjs/src/renderable/renderable.js @@ -428,6 +428,37 @@ export default class Renderable extends Rect { */ this.isKinematic = true; + /** + * whether a single post effect on this renderable needs an offscreen + * capture instead of the renderer's shader-swap fast path. + * + * One effect is normally applied by drawing the renderable with the + * effect's own program rather than capturing and post-processing it, + * which costs no render target. That is only equivalent when everything + * the renderable draws is a textured quad. `fillRect` and the shape + * dispatch go through a batcher that never looks at that shader, so a + * single effect on a renderable that draws with primitives silently + * does nothing. Set this and the capture is used instead, at the cost + * of a render target while the effect is active. + * + * Two or more effects always capture, so this only ever changes the + * single-effect case. + * @type {boolean} + * @default false + * @example + * class Bar extends Renderable { + * constructor(x, y, w, h) { + * super(x, y, w, h); + * // this draws itself with primitives, not as a sprite + * this.postEffectNeedsCapture = true; + * } + * draw(renderer) { + * renderer.fillRect(this.pos.x, this.pos.y, this.width, this.height); + * } + * } + */ + this.postEffectNeedsCapture = false; + /** * when true the renderable will be redrawn during the next update cycle * @type {boolean} diff --git a/packages/melonjs/src/renderable/trail.js b/packages/melonjs/src/renderable/trail.js index d7802f9f5..a2f2ebc4a 100644 --- a/packages/melonjs/src/renderable/trail.js +++ b/packages/melonjs/src/renderable/trail.js @@ -80,6 +80,12 @@ export default class Trail extends Renderable { * @ignore * @internal */ + // Every segment is a path filled through `renderer.fill()`, which goes + // to the primitive batcher. That batcher never reads `customShader`, + // so the renderer's single-effect shader-swap would apply to nothing + // at all: a trail with one post effect has to be captured instead. + this.postEffectNeedsCapture = true; + this._gradient = this._buildGradient(options); /** * @ignore diff --git a/packages/melonjs/src/renderable/ui/progressbar.ts b/packages/melonjs/src/renderable/ui/progressbar.ts index f14397d75..2168b5f00 100644 --- a/packages/melonjs/src/renderable/ui/progressbar.ts +++ b/packages/melonjs/src/renderable/ui/progressbar.ts @@ -201,6 +201,12 @@ export default class ProgressBar extends Renderable { // everything by half the bar this.anchorPoint.set(0, 0); + // Drawn with primitives rather than as a textured quad, so a single + // post effect has to capture rather than take the shader-swap path: + // the primitive batcher never reads that shader and the effect would + // silently do nothing. + this.postEffectNeedsCapture = true; + // Bound for exactly as long as this bar exists. Tying the two together // is what stops the listener outliving the thing it writes into: a // subscription held somewhere else goes on firing after the bar is diff --git a/packages/melonjs/src/video/renderer.js b/packages/melonjs/src/video/renderer.js index 3b00db53e..cbb6c07c7 100644 --- a/packages/melonjs/src/video/renderer.js +++ b/packages/melonjs/src/video/renderer.js @@ -1103,6 +1103,36 @@ export default class Renderer { return false; } + /** + * Whether ONE post effect on this renderable can be applied by swapping the + * shader the draw uses, rather than capturing the renderable offscreen and + * post-processing what it drew. + * + * The swap is the cheap path and costs no render target, but it is only + * equivalent when everything the renderable draws is a textured quad: the + * effect's own program stands in for the quad shader and samples the same + * texture. A renderable that draws with primitives goes through a batcher + * that never reads `customShader`, so its effect would silently do nothing. + * Such a renderable sets {@link Renderable#postEffectNeedsCapture}. + * + * Asked at BOTH ends from this one place on purpose. `beginPostEffect` and + * `endPostEffect` have to reach the same answer, and a `begin` that opens a + * render target which `end` then declines to resolve leaves the renderable + * drawn into a buffer nobody reads: it simply disappears. + * @param {Renderable} renderable - the renderable being drawn + * @param {object[]} effects - its enabled effect chain + * @returns {boolean} true to take the shader-swap path + * @ignore + * @internal + */ + _usesPostEffectFastPath(renderable, effects) { + return ( + effects.length === 1 && + !renderable._postEffectManaged && + renderable.postEffectNeedsCapture !== true + ); + } + /** * Begin capturing rendering to an offscreen buffer for post-effect processing. * Call endPostEffect() after rendering to blit the result to the screen. @@ -1117,7 +1147,7 @@ export default class Renderer { const effects = renderable.postEffects.filter((fx) => { return fx.enabled !== false; }); - if (effects.length === 1) { + if (this._usesPostEffectFastPath(renderable, effects)) { this.customShader = effects[0]; } else { this.customShader = undefined; diff --git a/packages/melonjs/src/video/webgl/webgl_renderer.js b/packages/melonjs/src/video/webgl/webgl_renderer.js index 85263adc2..73311aa25 100644 --- a/packages/melonjs/src/video/webgl/webgl_renderer.js +++ b/packages/melonjs/src/video/webgl/webgl_renderer.js @@ -1728,7 +1728,7 @@ export default class WebGLRenderer extends Renderer { return false; } // single effect on non-managed renderable: fast path via customShader (no FBO) - if (effects.length === 1 && !renderable._postEffectManaged) { + if (this._usesPostEffectFastPath(renderable, effects)) { this.customShader = effects[0]; return false; } @@ -1790,7 +1790,7 @@ export default class WebGLRenderer extends Renderer { return; } // single effect on non-managed renderable used customShader — no FBO to unbind - if (effects.length === 1 && !renderable._postEffectManaged) { + if (this._usesPostEffectFastPath(renderable, effects)) { return; } diff --git a/packages/melonjs/src/video/webgpu/webgpu_renderer.js b/packages/melonjs/src/video/webgpu/webgpu_renderer.js index 20a7ba39b..2585eb064 100644 --- a/packages/melonjs/src/video/webgpu/webgpu_renderer.js +++ b/packages/melonjs/src/video/webgpu/webgpu_renderer.js @@ -1274,7 +1274,7 @@ export default class WebGPURenderer extends Renderer { return false; } // single effect on a non-managed renderable: fast path (no target) - if (effects.length === 1 && !renderable._postEffectManaged) { + if (this._usesPostEffectFastPath(renderable, effects)) { this.customShader = effects[0]; return false; } @@ -1362,7 +1362,7 @@ export default class WebGPURenderer extends Renderer { return; } // the fast path set customShader — nothing offscreen to composite - if (effects.length === 1 && !renderable._postEffectManaged) { + if (this._usesPostEffectFastPath(renderable, effects)) { return; } diff --git a/packages/melonjs/tests/posteffect-fastpath.spec.js b/packages/melonjs/tests/posteffect-fastpath.spec.js new file mode 100644 index 000000000..509e11d7a --- /dev/null +++ b/packages/melonjs/tests/posteffect-fastpath.spec.js @@ -0,0 +1,224 @@ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + boot, + DesaturateEffect, + ProgressBar, + Renderable, + Trail, +} from "../src/index.js"; +import Renderer from "../src/video/renderer.js"; +import WebGLRenderer from "../src/video/webgl/webgl_renderer.js"; +import { + getWebGLRenderer, + releaseWebGLRenderer, +} from "./helpers/webgl-context.js"; + +/** + * ONE post effect is normally applied by drawing the renderable with the + * effect's own program instead of capturing it offscreen, which costs no + * render target. That is equivalent only when everything the renderable draws + * is a textured quad: `fillRect` and the shape dispatch go to the primitive + * batcher, and NO batcher reads `customShader`, so the effect silently did + * nothing on a renderable that draws with primitives. + * + * Measured before this existed, pure red through a `DesaturateEffect`: + * one effect read back `[255, 0, 0]` (untouched) while two read `[76, 76, 76]`. + * Two already captured, so the bug was the single-effect case alone. + */ +describe("post-effect fast path", () => { + describe("the predicate", () => { + // a bare renderer: the predicate is pure, and reaching it through a + // real GPU context would say nothing extra about it + const renderer = Object.create(Renderer.prototype); + const fx = () => { + return { enabled: true }; + }; + + it("takes the fast path for a single effect on a plain renderable", () => { + const r = new Renderable(0, 0, 10, 10); + expect(renderer._usesPostEffectFastPath(r, [fx()])).toBe(true); + }); + + it("does not, once the renderable asks to be captured", () => { + const r = new Renderable(0, 0, 10, 10); + r.postEffectNeedsCapture = true; + expect(renderer._usesPostEffectFastPath(r, [fx()])).toBe(false); + }); + + it("does not for a camera, which manages its own target", () => { + const r = new Renderable(0, 0, 10, 10); + r._postEffectManaged = true; + expect(renderer._usesPostEffectFastPath(r, [fx()])).toBe(false); + }); + + it("does not for a chain, which always captures", () => { + const r = new Renderable(0, 0, 10, 10); + expect(renderer._usesPostEffectFastPath(r, [fx(), fx()])).toBe(false); + }); + + it("does not for an empty chain", () => { + const r = new Renderable(0, 0, 10, 10); + expect(renderer._usesPostEffectFastPath(r, [])).toBe(false); + }); + }); + + describe("who asks to be captured", () => { + it("a plain renderable does not", () => { + expect(new Renderable(0, 0, 10, 10).postEffectNeedsCapture).toBe(false); + }); + + it("ProgressBar does, since it draws with primitives", () => { + const bar = new ProgressBar(0, 0, { width: 10, height: 10 }); + expect(bar.postEffectNeedsCapture).toBe(true); + bar.destroy(); + }); + + it("Trail does, for the same reason", () => { + const trail = new Trail(); + expect(trail.postEffectNeedsCapture).toBe(true); + trail.destroy(); + }); + }); + + describe("on WebGL, measured in pixels", () => { + const SIZE = 128; + let renderer; + let gl; + let isWebGL; + + beforeAll(async () => { + boot(); + renderer = await getWebGLRenderer(SIZE, SIZE); + isWebGL = renderer instanceof WebGLRenderer; + if (isWebGL) { + gl = renderer.gl; + } + }); + + afterAll(() => { + releaseWebGLRenderer(); + }); + + const skipIfNoWebGL = (ctx) => { + if (!isWebGL) { + ctx.skip("WebGL renderer not available in this environment"); + return true; + } + return false; + }; + + /** one pixel, converted from the GL bottom-left origin */ + const readPixel = (x, y) => { + const px = new Uint8Array(4); + gl.finish(); + gl.readPixels(x, SIZE - 1 - y, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE, px); + return [px[0], px[1], px[2]]; + }; + + const paint = (renderable) => { + renderer.backgroundColor.setColor(0, 0, 0, 255); + renderer.clear(); + renderer.save(); + renderer.resetTransform(); + renderable.preDraw(renderer); + renderable.draw(renderer); + renderable.postDraw(renderer); + renderer.restore(); + renderer.flush(); + }; + + /** a bar filling the frame with pure red, nothing but the fill */ + const makeBar = () => { + return new ProgressBar(0, 0, { + width: SIZE, + height: 40, + value: 1, + trackColor: null, + borderColor: null, + fillColor: "#ff0000", + }); + }; + + it("draws its fill untouched with no effect", (ctx) => { + if (skipIfNoWebGL(ctx)) { + return; + } + const bar = makeBar(); + try { + paint(bar); + expect(readPixel(SIZE / 2, 20)).toEqual([255, 0, 0]); + } finally { + bar.destroy(); + } + }); + + it("a SINGLE effect reaches the primitives it draws", (ctx) => { + if (skipIfNoWebGL(ctx)) { + return; + } + // this is the case that silently did nothing + const bar = makeBar(); + try { + bar.addPostEffect(new DesaturateEffect(renderer)); + paint(bar); + const [r, g, b] = readPixel(SIZE / 2, 20); + expect(r).toBe(g); + expect(g).toBe(b); + expect(r).toBeGreaterThan(0); + expect(r).toBeLessThan(255); + } finally { + bar.destroy(); + } + }); + + it("a chain still works, as it already did", (ctx) => { + if (skipIfNoWebGL(ctx)) { + return; + } + const bar = makeBar(); + try { + bar.addPostEffect(new DesaturateEffect(renderer)); + bar.addPostEffect(new DesaturateEffect(renderer)); + paint(bar); + const [r, g, b] = readPixel(SIZE / 2, 20); + expect(r).toBe(g); + expect(g).toBe(b); + } finally { + bar.destroy(); + } + }); + + it("a disabled effect leaves it alone", (ctx) => { + if (skipIfNoWebGL(ctx)) { + return; + } + const bar = makeBar(); + try { + const effect = new DesaturateEffect(renderer); + effect.enabled = false; + bar.addPostEffect(effect); + paint(bar); + expect(readPixel(SIZE / 2, 20)).toEqual([255, 0, 0]); + } finally { + bar.destroy(); + } + }); + + it("a renderable that has NOT opted in keeps the fast path", (ctx) => { + if (skipIfNoWebGL(ctx)) { + return; + } + // the guard on the change: a sprite must be unaffected, so a bar + // with the flag cleared has to behave exactly as it did before + const bar = makeBar(); + try { + bar.postEffectNeedsCapture = false; + bar.addPostEffect(new DesaturateEffect(renderer)); + paint(bar); + expect(readPixel(SIZE / 2, 20)).toEqual([255, 0, 0]); + } finally { + bar.destroy(); + } + }); + }); +}); diff --git a/packages/melonjs/tests/webgpu_post_effect_flow.spec.js b/packages/melonjs/tests/webgpu_post_effect_flow.spec.js index ca16378e5..d2be2ebfa 100644 --- a/packages/melonjs/tests/webgpu_post_effect_flow.spec.js +++ b/packages/melonjs/tests/webgpu_post_effect_flow.spec.js @@ -205,6 +205,58 @@ describe("WebGPU post-effect control flow (recorded primitives)", () => { expect(log).toEqual([]); }); + // A renderable that draws with primitives cannot use that fast path: the + // primitive batcher never reads `customShader`, so its one effect would + // apply to nothing. `postEffectNeedsCapture` sends it through the pool + // instead, and BOTH ends have to agree, since a `begin` that opens a target + // an `end` declines to resolve draws the renderable into a buffer nobody + // reads and it disappears. + it("postEffectNeedsCapture sends a single effect through the pool instead", () => { + const fx = makeEffect({ name: "solo" }); + const bar = makeRenderable([fx]); + bar.postEffectNeedsCapture = true; + + expect(renderer.beginPostEffect(bar)).toBe(true); + expect(renderer.customShader).toBeUndefined(); + log.push("---content---"); + renderer.endPostEffect(bar); + + expect(log).toEqual([ + "save", + // a sprite-style pass opens on a transparent clear, unscissored + "target:rt0:clear", + "descissor", + "---content---", + // back to the parent, then the one effect blits straight across + // with no ping-pong, keeping the blend for non-camera content + "target:canvas", + "blit:rt0→solo:keep=true", + "restore", + "pushGlobals", + ]); + }); + + it("both ends agree, so the pass depth comes back to where it started", () => { + const bar = makeRenderable([makeEffect({ name: "solo" })]); + bar.postEffectNeedsCapture = true; + expect(renderer.effectPassDepth).toBe(0); + renderer.beginPostEffect(bar); + expect(renderer.effectPassDepth).toBe(1); + renderer.endPostEffect(bar); + expect(renderer.effectPassDepth).toBe(0); + }); + + it("leaves a renderable that has not opted in on the fast path", () => { + const fx = makeEffect({ name: "solo" }); + const sprite = makeRenderable([fx]); + sprite.postEffectNeedsCapture = false; + expect(renderer.beginPostEffect(sprite)).toBe(false); + expect(renderer.customShader).toBe(fx); + expect(log).toEqual([]); + renderer.endPostEffect(sprite); + expect(log).toEqual([]); + }); + it("disabled effects are filtered before any pooling decision", () => { const sprite = makeRenderable([ makeEffect({ name: "off", enabled: false }),