Skip to content

Commit c630485

Browse files
obiotclaude
andcommitted
Address Copilot review: push deopt, cache encapsulation, scissor tracking
- push(): use vertexSize > 5 instead of arguments.length (avoids V8 deopt), always write default textureId 0 when vertexSize is 6 - TextureCache.resetUnitAssignments(): encapsulate unit reset logic - clipRect: use _scissorActive instead of gl.isEnabled() query - Update vertex buffer tests for new push behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 36e87d3 commit c630485

7 files changed

Lines changed: 61 additions & 12 deletions

File tree

packages/melonjs/src/application/settings.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,42 @@ export type ApplicationSettings = {
6767
* @default true
6868
*/
6969
consoleHeader: boolean;
70+
71+
/**
72+
* the default blend mode to use ("normal", "multiply", "lighter", "additive", "screen")
73+
* @default "normal"
74+
*/
7075
blendMode: BlendMode;
7176

7277
/**
7378
* the physic system to use (default: "builtin", or "none" to disable builtin physic)
7479
* @default "builtin"
7580
*/
7681
physic: PhysicsType;
82+
/**
83+
* if true, the renderer will fail if the browser reports a major performance caveat
84+
* (e.g. software WebGL). Set to false to allow WebGL on machines with
85+
* blocklisted GPU drivers or software renderers.
86+
* @default false
87+
*/
7788
failIfMajorPerformanceCaveat: boolean;
89+
90+
/**
91+
* whether to enable sub-pixel rendering (avoid sprite flickering when using transforms)
92+
* @default false
93+
*/
7894
subPixel: boolean;
95+
96+
/**
97+
* whether to enable verbose mode (additional console output for debugging)
98+
* @default false
99+
*/
79100
verbose: boolean;
101+
102+
/**
103+
* whether to enable legacy mode (compatibility with older melonJS APIs)
104+
* @default false
105+
*/
80106
legacy: boolean;
81107

82108
/**
@@ -99,13 +125,18 @@ export type ApplicationSettings = {
99125
batcher?: (new (renderer: any) => Batcher) | undefined;
100126
} & (
101127
| {
102-
// the DOM parent element (or its string ID) to hold the canvas in the HTML file
128+
/**
129+
* the DOM parent element (or its string ID) to hold the canvas in the HTML file
130+
*/
103131
parent: string | HTMLElement;
104132
canvas?: never;
105133
}
106134
| {
107135
parent?: never;
108-
// an existing canvas element to use as the renderer target (by default melonJS will create its own canvas based on given parameters)
136+
/**
137+
* an existing canvas element to use as the renderer target
138+
* (by default melonJS will create its own canvas based on given parameters)
139+
*/
109140
canvas: HTMLCanvasElement;
110141
}
111142
);

packages/melonjs/src/video/rendertarget/canvasrendertarget.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,8 @@ class CanvasRenderTarget {
275275
*/
276276
invalidate(renderer) {
277277
if (typeof renderer.gl !== "undefined") {
278-
// flush pending draws that reference the current texture data
279-
// before invalidating (required for multi-texture batching)
278+
// flush pending draws referencing the current texture data
280279
renderer.flush();
281-
// make sure the right batcher is active
282280
renderer.setBatcher("quad");
283281
// invalidate the previous corresponding texture so that it can reuploaded once changed
284282
this.glTextureUnit = renderer.cache.getUnit(

packages/melonjs/src/video/texture/cache.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,16 @@ class TextureCache {
6161
return 0;
6262
}
6363

64+
/**
65+
* Reset all texture unit assignments without clearing the texture cache.
66+
* Used by multi-texture batching when the shader's sampler range is exceeded.
67+
* @ignore
68+
*/
69+
resetUnitAssignments() {
70+
this.units.clear();
71+
this.usedUnits.clear();
72+
}
73+
6474
/**
6575
* @ignore
6676
*/

packages/melonjs/src/video/webgl/batchers/quad_batcher.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,7 @@ export default class QuadBatcher extends MaterialBatcher {
200200
// reset if the cache assigned a unit beyond the shader's range
201201
if (unit >= this.maxBatchTextures) {
202202
this.flush();
203-
this.renderer.cache.units.clear();
204-
this.renderer.cache.usedUnits.clear();
203+
this.renderer.cache.resetUnitAssignments();
205204
unit = this.uploadTexture(texture, w, h, reupload, false);
206205
}
207206
} else {

packages/melonjs/src/video/webgl/buffer/vertex.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ export default class VertexArrayBuffer {
5656
this.bufferF32[offset + 2] = u;
5757
this.bufferF32[offset + 3] = v;
5858
this.bufferU32[offset + 4] = tint;
59-
if (arguments.length > 5) {
60-
this.bufferF32[offset + 5] = textureId;
59+
if (this.vertexSize > 5) {
60+
this.bufferF32[offset + 5] = textureId || 0;
6161
}
6262

6363
this.vertexCount++;

packages/melonjs/src/video/webgl/webgl_renderer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1701,7 +1701,7 @@ export default class WebGLRenderer extends Renderer {
17011701
height !== canvas.height
17021702
) {
17031703
const currentScissor = this.currentScissor;
1704-
if (gl.isEnabled(gl.SCISSOR_TEST)) {
1704+
if (this._scissorActive) {
17051705
// if same as the current scissor box do nothing
17061706
if (
17071707
currentScissor[0] === x &&

packages/melonjs/tests/vertexBuffer.spec.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,24 @@ describe("VertexArrayBuffer", () => {
3333
expect(buf.bufferF32[5]).toBe(3); // textureId
3434
});
3535

36-
it("should not write textureId when not provided", () => {
36+
it("should write default textureId 0 when not provided (vertexSize 6)", () => {
3737
const buf = new VertexArrayBuffer(6, 4);
3838

3939
buf.push(10, 20, 0.0, 1.0, 0xffffffff);
4040

4141
expect(buf.vertexCount).toBe(1);
42-
expect(buf.bufferF32[5]).toBe(0); // untouched (default zero)
42+
expect(buf.bufferF32[5]).toBe(0); // default 0
43+
});
44+
45+
it("should not write textureId when vertexSize is 5", () => {
46+
const buf = new VertexArrayBuffer(5, 4);
47+
48+
// write a sentinel at offset 5
49+
buf.bufferF32[5] = 99;
50+
buf.push(10, 20, 0.0, 1.0, 0xffffffff);
51+
52+
expect(buf.vertexCount).toBe(1);
53+
expect(buf.bufferF32[5]).toBe(99); // untouched
4354
});
4455

4556
it("should write multiple vertices sequentially", () => {

0 commit comments

Comments
 (0)