Extract renderer-agnostic buffer classes from WebGL layer#1414
Merged
Conversation
…et architecture (#1396) Replace the single `shader` property on Renderable with a `postEffects` array supporting multiple shader effects applied in sequence via FBO ping-pong. Single-effect renderables use a zero-overhead customShader fast path (no FBO). Introduce an abstract `RenderTarget` base class with `WebGLRenderTarget` and `CanvasRenderTarget` implementations, and a renderer-agnostic `RenderTargetPool` using a factory pattern — ready for future WebGPU support. Extract direct GL state calls from the post-effect pipeline into renderer methods (setViewport, clearRenderTarget, enableScissor, disableScissor, setBlendEnabled), all no-ops on the base Renderer, implemented on WebGLRenderer. Fix WebGL texture unit corruption during FBO resize by explicitly using TEXTURE0. Fix projection matrix not being saved/restored across post-effect passes. Fix Canvas renderer not syncing customShader in save/restore. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Clear customShader when no enabled effects or entering FBO path (prevents shader leak to children) - Reset _activeBase/_previousBase in RenderTargetPool.destroy() - Guard against nested sprite post-effect passes in RenderTargetPool.begin() - Handle null blob in toBlob() callbacks (reject instead of resolving null) - Restore toImageBitmap() fast path on CanvasRenderTarget using createImageBitmap(canvas) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a public `colorMatrix` property to Camera2d — a built-in ColorMatrix that is always applied as the final post-processing pass, after any effects added via addPostEffect(). Zero overhead when identity (default). The effect is transient: pushed to postEffects before beginPostEffect and removed after endPostEffect each frame. Camera2d uses only the public ColorMatrixEffect API (reset + multiply), no internal shader knowledge. Also fix FileReader error handling in RenderTarget.toDataURL(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Save/restore active texture unit in WebGLRenderTarget constructor and resize() to keep the batcher's currentTextureUnit cache in sync (TEXTURE0 fix preserved) - Handle null 2d context in RenderTarget.toBlob() fallback paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve conflicts: keep colorMatrix property, Copilot review fixes (active texture save/restore, null context handling). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
onloadend fires on success, error, and abort — could resolve with null. onload only fires on success. Also handle onabort explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move VertexArrayBuffer from video/webgl/buffer/ to video/buffer/ (zero GL dependency — pure typed array management) - Split IndexBuffer into renderer-agnostic IndexBuffer base class (data accumulation, quad patterns) and WebGLIndexBuffer subclass (GL buffer creation, bind, upload) - Update all imports in batchers and tests Part of #1410 (TextureCache + Batcher abstraction). The TextureCache unit management refactoring is deferred pending investigation of a rendering regression with per-sprite shader effects. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances the renderer-agnostic rendering architecture by extracting typed-array buffer management out of the WebGL layer, while also adding a built-in camera color-grading matrix and tightening some render target / image-export edge cases.
Changes:
- Moved
VertexArrayBufferout ofvideo/webgl/into a renderer-agnosticvideo/buffer/location and updated call sites/tests. - Split index buffering into a renderer-agnostic
IndexBuffer(data accumulation/patterns) and aWebGLIndexBuffer(GL bind/upload), and updated the WebGL batcher to use it. - Added
Camera2d.colorMatrix(implemented via a transientColorMatrixEffectpost-effect) and improved WebGL render target texture-unit state preservation andRenderTargetblob/dataURL error handling.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/melonjs/tests/vertexBuffer.spec.js | Updates VertexArrayBuffer import path to new renderer-agnostic location. |
| packages/melonjs/tests/drawVertices.spec.js | Updates VertexArrayBuffer import path to new renderer-agnostic location. |
| packages/melonjs/tests/camera.spec.js | Adds tests around Camera2d.colorMatrix lifecycle and interaction with user postEffects. |
| packages/melonjs/src/video/webgl/buffer/index.js | Converts WebGL index buffer into WebGLIndexBuffer subclass of renderer-agnostic IndexBuffer. |
| packages/melonjs/src/video/webgl/batchers/batcher.js | Updates imports and instantiation to use new buffer locations/classes. |
| packages/melonjs/src/video/rendertarget/webglrendertarget.js | Preserves/restores active texture unit during texture creation/resizing to reduce GL state desync. |
| packages/melonjs/src/video/rendertarget/rendertarget.ts | Improves null-context handling and FileReader error/abort handling for toBlob/toDataURL. |
| packages/melonjs/src/video/buffer/vertex.js | Updates docstring for renderer-agnostic vertex array buffer. |
| packages/melonjs/src/video/buffer/index.js | Introduces renderer-agnostic IndexBuffer for index data accumulation/pattern generation. |
| packages/melonjs/src/camera/camera2d.ts | Adds colorMatrix and transient internal ColorMatrixEffect integration in draw() + cleanup in destroy(). |
| packages/melonjs/CHANGELOG.md | Documents new Camera.colorMatrix and the buffer refactor. |
| packages/examples/src/examples/platformer/play.ts | Updates platformer example to use viewport.colorMatrix instead of adding a ColorMatrixEffect. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The dynamic flag was stored but never used — upload() always used STREAM_DRAW regardless. Removed dead code per Copilot review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
video/webgl/buffer/tovideo/buffer/— zero GL dependency, pure typed array management. Usable by a future WebGPU batcher.IndexBufferbase class (data accumulation, quad index patterns) andWebGLIndexBuffersubclass (GL buffer creation, bind, upload).Part of #1410. The TextureCache unit management refactoring is deferred pending investigation of a rendering regression with per-sprite shader effects.
Test plan
🤖 Generated with Claude Code