Conversation
…Gradient) Closes #1352 - New Gradient class with addColorStop(), matching Canvas 2D API - createLinearGradient() and createRadialGradient() on base Renderer - setColor() accepts Gradient objects on both Canvas and WebGL renderers - Canvas: uses native CanvasGradient directly as fillStyle - WebGL: renders gradient to CanvasRenderTarget texture, draws via drawImage - WebGL non-rect shapes (ellipse, arc, polygon, roundRect) use stencil masking - Gradient state saved/restored with save()/restore() via RenderState - Canvas renderer restore() handles CanvasGradient fillStyle gracefully - New gradients example showcasing sky, health bars, shapes, rainbow star - 24 unit tests covering API, caching, save/restore, and color/gradient mixing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class gradient fills to the melonJS renderer abstraction, aligning the engine’s drawing API more closely with the Canvas 2D API while supporting both Canvas and WebGL backends.
Changes:
- Introduces a new
Gradientclass (addColorStop, canvas/texture generation + caching) and exposescreateLinearGradient/createRadialGradienton the baseRenderer. - Extends Canvas and WebGL renderers to accept gradients via
setColor()and render gradient fills across primitive fill methods. - Adds unit tests, a new gradients example, and updates docs/changelog to reflect the new API.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Links the rendering API bullet to the wiki page. |
| packages/melonjs/tests/gradient.spec.js | Adds unit tests for the new Gradient API and renderer integration. |
| packages/melonjs/src/video/webgl/webgl_renderer.js | Implements gradient support in WebGL, including stencil-based masking for non-rect shapes. |
| packages/melonjs/src/video/renderstate.js | Tracks gradient state in the save/restore render state stack. |
| packages/melonjs/src/video/renderer.js | Adds gradient factory methods to the base renderer API. |
| packages/melonjs/src/video/gradient.js | New Gradient implementation (color stops, canvas gradient, POT texture canvas). |
| packages/melonjs/src/video/canvas/canvas_renderer.js | Implements gradient support for Canvas via native CanvasGradient. |
| packages/melonjs/src/index.ts | Exports Gradient from the public API. |
| packages/melonjs/CHANGELOG.md | Documents the new gradient renderer API. |
| packages/examples/src/main.tsx | Registers the new Gradients example route. |
| packages/examples/src/examples/gradients/ExampleGradients.tsx | Adds a gradients showcase example scene. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ting, cache example - WebGL setColor: preserve existing alpha instead of forcing 1.0 - WebGL restore: sync _currentGradient from renderState - #gradientMask: nest within existing mask levels instead of clearing stencil - Example: cache gradients in onActivateEvent, reuse across frames Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * current gradient fill (null when using solid color) | ||
| * @type {Gradient|null} | ||
| */ | ||
| this.currentGradient = null; |
There was a problem hiding this comment.
RenderState uses a JSDoc type Gradient but the symbol isn’t in scope in this module. With checkJs/TS tooling this typically becomes “Cannot find name 'Gradient'”. Add a JSDoc @import for Gradient (or otherwise make the type available) to keep type-checking/linting consistent.
| toCanvasGradient(context) { | ||
| if (this._canvasGradient && !this._dirty) { | ||
| return this._canvasGradient; | ||
| } |
There was a problem hiding this comment.
Gradient.toCanvasGradient(context) caches a single CanvasGradient instance, but CanvasGradient objects are context-specific. If the same Gradient instance is used with a different canvas/context later, this will return a gradient created from the wrong context. Consider caching per-context (e.g., WeakMap keyed by context) or invalidating the cache when context changes.
| addColorStop(offset, color) { | ||
| this.colorStops.push({ | ||
| offset, | ||
| color: typeof color === "string" ? color : color.toRGBA(), | ||
| }); | ||
| this._dirty = true; | ||
| this._canvasGradient = undefined; | ||
| this._renderTarget = undefined; | ||
| return this; |
There was a problem hiding this comment.
addColorStop(offset, color) is documented as matching the Canvas 2D API, but it currently accepts any offset value and only fails later when the native canvas gradient is generated. The Canvas API throws immediately if offset is not finite or is outside [0, 1]. Validating and throwing on invalid offsets here would make behavior consistent and prevent hard-to-debug runtime errors during rendering.
| if (color instanceof Gradient) { | ||
| this.renderState.currentGradient = color; | ||
| this._currentGradient = color; | ||
| } else { | ||
| this.renderState.currentGradient = null; | ||
| this._currentGradient = null; | ||
| const alpha = this.currentColor.alpha; | ||
| this.currentColor.copy(color); | ||
| this.currentColor.alpha *= alpha; | ||
| } |
There was a problem hiding this comment.
After setColor(gradient), WebGL stroke APIs (e.g. strokeRect, strokePolygon, etc.) still render using currentColor (which is left unchanged), so stroke output becomes inconsistent (and differs from CanvasRenderer where strokeStyle is set to the gradient). Either implement gradient strokes for WebGL, or explicitly document/guard that gradients are supported for fills only and define what stroke should do when a gradient is active.
| * @returns {Array<{x: number, y: number}>} triangle vertices | ||
| * @ignore | ||
| */ | ||
| /** | ||
| * Draw a gradient-filled shape by masking with the shape and filling the bounding rect. |
There was a problem hiding this comment.
There are two consecutive JSDoc blocks here; the first one (for #generateTriangleFan) is now separated from the actual #generateTriangleFan method by the new #gradientMask definition, so the doc is effectively orphaned/misattached. Move the triangle-fan JSDoc immediately above #generateTriangleFan and keep #gradientMask’s JSDoc above #gradientMask only.
| gl.colorMask(true, true, true, true); | ||
| // restore parent mask stencil test | ||
| gl.stencilFunc(gl.NOTEQUAL, this.maskLevel + 1, 1); | ||
| gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP); |
There was a problem hiding this comment.
#gradientMask() restores the parent mask stencil test with gl.stencilFunc(gl.NOTEQUAL, ...), but setMask(mask, invert=true) sets gl.stencilFunc(gl.EQUAL, ...). If an inverted mask is active, calling any gradient-filled shape will leave the stencil state in the wrong mode for subsequent draws. Track the current mask invert mode (or snapshot/restore stencilFunc state) and restore the correct stencilFunc here.
| it("should accept Color objects in addColorStop", () => { | ||
| const gradient = app.renderer.createLinearGradient(0, 0, 100, 0); | ||
| gradient.addColorStop(0, "#FF0000"); | ||
| expect(gradient.colorStops[0].color).toEqual("#FF0000"); | ||
| }); |
There was a problem hiding this comment.
This test case is named as if it verifies passing Color objects to addColorStop, but it actually passes a string ("#FF0000"). Either update the test to pass a real Color instance or rename the test so it reflects what’s being asserted.
…s, test name - RenderState: add @import for Gradient type - Gradient.addColorStop: validate offset is 0.0-1.0 - WebGL renderer: fix orphaned #generateTriangleFan JSDoc block - Test: rename misleading "Color objects" test to "store color stop as string" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… cache invalidation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (this._currentGradient) { | ||
| const canvas = this._currentGradient.toCanvas(x, y, width, height); | ||
| this.drawImage(canvas, 0, 0, width, height, x, y, width, height); | ||
| return; |
There was a problem hiding this comment.
fillRect() draws gradients via a canvas returned from Gradient.toCanvas(), but drawImage() only forces texture reupload for videos. Since toCanvas() re-renders (and may resize) the same canvas element across calls, the cached WebGL texture/atlas can become stale (content and/or dimensions), producing incorrect gradients when drawing the same Gradient at different positions/sizes. Consider adding a way to force reupload / invalidate the texture cache for these dynamic gradient canvases whenever toCanvas() re-renders or resizes them.
| if (hasMask) { | ||
| // restore the parent mask level by decrementing stencil | ||
| this._currentGradient = null; | ||
| gl.colorMask(false, false, false, false); | ||
| gl.stencilFunc(gl.EQUAL, stencilRef, 0xff); | ||
| gl.stencilOp(gl.KEEP, gl.KEEP, gl.DECR); | ||
| drawShape(); | ||
| this.flush(); | ||
| gl.colorMask(true, true, true, true); | ||
| // restore parent mask stencil test | ||
| gl.stencilFunc(gl.NOTEQUAL, this.maskLevel + 1, 1); | ||
| gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP); | ||
| this._currentGradient = grad; | ||
| } else { | ||
| gl.disable(gl.STENCIL_TEST); | ||
| } |
There was a problem hiding this comment.
#gradientMask() assumes the parent stencil mask uses gl.NOTEQUAL and does not account for setMask(mask, invert=true). When an inverted mask is active, this function will clip against the wrong region and then resets the stencil func back to NOTEQUAL, which can corrupt subsequent rendering under the parent inverted mask. To avoid breaking masking, preserve/restore the existing stencil func/op state (or track the invert flag) and restore it exactly after the gradient pass.
| if (hasMask) { | ||
| // nest within existing mask level | ||
| gl.stencilFunc(gl.EQUAL, this.maskLevel, 0xff); | ||
| gl.stencilOp(gl.KEEP, gl.KEEP, gl.INCR); | ||
| } else { | ||
| gl.clear(gl.STENCIL_BUFFER_BIT); | ||
| gl.stencilFunc(gl.ALWAYS, 1, 0xff); | ||
| gl.stencilOp(gl.REPLACE, gl.REPLACE, gl.REPLACE); | ||
| } |
There was a problem hiding this comment.
Within #gradientMask() the nesting path uses gl.stencilFunc(gl.EQUAL, this.maskLevel, 0xff), while setMask() configures masking with a bitmask of 1. Using 0xff here makes nesting depend on exact stencil values and can break when maskLevel > 1 (nested masks), because the parent mask test elsewhere only considers the low bit. Align the stencil compare mask with the existing mask pipeline so nested masks behave consistently.
| }); | ||
| this._dirty = true; | ||
| this._canvasGradient = undefined; | ||
| this._renderTarget = undefined; |
There was a problem hiding this comment.
addColorStop() sets this._renderTarget = undefined. In WebGL usage, the old render target canvas may already be cached in TextureCache and have an associated GL texture; dropping the reference here prevents any cleanup and can leak GPU memory across repeated addColorStop() calls. Prefer keeping the same render target and just marking it dirty (and/or provide an explicit cleanup path) so the cached texture can be reuploaded instead of abandoned.
| this._renderTarget = undefined; |
| // reuse or create the render target | ||
| if (!this._renderTarget) { | ||
| this._renderTarget = new CanvasRenderTarget(tw, th); | ||
| } else if ( | ||
| this._renderTarget.width !== tw || | ||
| this._renderTarget.height !== th | ||
| ) { | ||
| this._renderTarget.canvas.width = tw; | ||
| this._renderTarget.canvas.height = th; |
There was a problem hiding this comment.
toCanvas() mutates an existing canvas element's width/height when the requested POT dimensions change. If that canvas has already been used with the WebGL renderer, the renderer's TextureCache will still hold a TextureAtlas sized for the old dimensions and (unless explicitly reuploaded) a GL texture allocated for the old size. This can lead to incorrect UVs or stale textures when the same Gradient is drawn at different sizes. Consider allocating a new canvas when dimensions change, or ensure the WebGL-side cache/texture is invalidated & recreated on resize.
| // reuse or create the render target | |
| if (!this._renderTarget) { | |
| this._renderTarget = new CanvasRenderTarget(tw, th); | |
| } else if ( | |
| this._renderTarget.width !== tw || | |
| this._renderTarget.height !== th | |
| ) { | |
| this._renderTarget.canvas.width = tw; | |
| this._renderTarget.canvas.height = th; | |
| // create the render target, or replace it entirely if the POT size changed. | |
| // Avoid resizing an existing canvas in place, as renderer-side texture caches | |
| // may still hold metadata or GL resources for the previous dimensions. | |
| if ( | |
| !this._renderTarget || | |
| this._renderTarget.width !== tw || | |
| this._renderTarget.height !== th | |
| ) { | |
| this._renderTarget = new CanvasRenderTarget(tw, th); |
| * Created via {@link CanvasRenderer#createLinearGradient}, {@link CanvasRenderer#createRadialGradient}, | ||
| * {@link WebGLRenderer#createLinearGradient}, or {@link WebGLRenderer#createRadialGradient}. |
There was a problem hiding this comment.
The class docstring says gradients are created via CanvasRenderer#createLinearGradient/WebGLRenderer#createLinearGradient, but these factory methods are implemented on the base Renderer class and inherited by both renderers. Updating the references to Renderer#createLinearGradient/Renderer#createRadialGradient will keep the documentation accurate and avoid implying renderer-specific APIs.
| * Created via {@link CanvasRenderer#createLinearGradient}, {@link CanvasRenderer#createRadialGradient}, | |
| * {@link WebGLRenderer#createLinearGradient}, or {@link WebGLRenderer#createRadialGradient}. | |
| * Created via {@link Renderer#createLinearGradient} or {@link Renderer#createRadialGradient}. |
Summary
Closes #1352
Gradientclass withaddColorStop(), matching the Canvas 2D APIcreateLinearGradient(x0, y0, x1, y1)andcreateRadialGradient(x0, y0, r0, x1, y1, r1)on both rendererssetColor()acceptsGradientobjects — works with all fill methods (fillRect,fillEllipse,fillArc,fillPolygon,fillRoundRect)CanvasGradientdirectly asfillStyleCanvasRenderTargettexture viadrawImage; non-rect shapes use stencil maskingRenderStateand saved/restored withsave()/restore()Test plan
🤖 Generated with Claude Code