Lighting: warn on Canvas only when a normal-mapped sprite goes unlit

packages/melonjs/CHANGELOG.md

@@ -48,6 +48,7 @@
 - Camera: `offset` can shift the view for good, such as to look above the hero, and not only for a shake: a shake adds its jitter to the offset and gives it back when it ends, where it used to reset it to zero. Culling, the pointer (`localToWorld`, `worldToLocal`), tile layers and parallax `ImageLayer`s all count it, so a parallax background now shakes with the world, by its ratio
 - Lighting: on the Canvas renderer, a light's hole in the ambient darkness is cut where the light is, under a scrolled or zoomed camera and one drawn at an offset such as a minimap. Under a scrolled camera the mask kept the darkness inside the light and cleared it around it, and the holes of a zoomed or offset camera were cut as if it were neither
 - Lighting: the ambient darkness covers the whole view of a camera. Zoomed out, it covered only as much of the world as the camera is wide, and at a fractional position, such as a camera following its target with damping, it left the first column and row lit unless `subPixel` was set
+- Lighting: on the Canvas renderer, a lit game whose sprites have no normal map no longer warns that normal-map lighting needs a GPU backend. It warned as soon as a `Light2d` existed, though a sprite without a normal map is drawn unlit on every renderer; it now warns once, when it draws a sprite with a normal map under a light
 - Audio: a sprite region declared looping (`[start, duration, true]`) loops when played with `audio.play(name, { sprite })`. It played only once unless `loop: true` was passed as well
 - Text: a label whose text is empty or only spaces no longer blanks the whole frame on WebGPU. Its canvas was sized 0 pixels wide, which WebGPU refuses to upload
 - Typings: `TextureAtlas#getAnimationSettings()` is typed as the settings it returns, so they pass to a `Sprite` constructor from TypeScript without a cast. It returned `object`, and `Sprite3d` typed the named animations an aseprite atlas carries as an array

packages/melonjs/src/video/canvas/canvas_renderer.js

@@ -435,6 +435,10 @@ export default class CanvasRenderer extends Renderer {
 			// Fast path: don't draw fully transparent
 			return;
 		}
+		// a sprite with a normal map, under lights it cannot be lit by here
+		if (this.currentNormalMap !== null) {
+			this._warnUnlitNormalMap();
+		}
 		const context = this.getContext();
 
 		if (typeof sw === "undefined") {

packages/melonjs/src/video/renderer.js

@@ -335,11 +335,11 @@ export default class Renderer {
 		this.currentShininess = 0;
 
 		/**
-		 * Number of active `Light2d` instances uploaded to the lit batcher
-		 * for the current frame. Set by `setLightUniforms`. The WebGL
-		 * renderer's `drawImage` reads this to decide between the unlit
-		 * fast-path batcher (default) and the lit batcher (only when
-		 * lights AND a normal map are both in play).
+		 * Number of active `Light2d` instances for the camera drawing, set by
+		 * `setLightUniforms`. The GPU renderers' `drawImage` reads it to
+		 * choose between the unlit batcher (default) and the lit one (only
+		 * when lights AND a normal map are both in play); the Canvas renderer,
+		 * to say when a normal-mapped sprite is drawn unlit.
 		 * @type {number}
 		 */
 		this.activeLightCount = 0;
@@ -1264,26 +1264,6 @@ export default class Renderer {
 		this.currentBlendMode = mode;
 	}
 
-	/**
-	 * Upload the active scene lights to the lit sprite pipeline.
-	 *
-	 * Called once per camera per frame by `Camera2d.draw()` (after the
-	 * FBO is bound, before the world tree walk fires `Sprite.draw` for
-	 * any normal-mapped sprite). GPU renderers (WebGL today, WebGPU once
-	 * it lands) override this to pack the lights into the lit shader's
-	 * uniform buffers; the Canvas renderer cannot do per-pixel normal-map
-	 * lighting and silently ignores the call. The first time a non-empty
-	 * light list is passed in Canvas mode, a one-shot console warning is
-	 * emitted.
-	 *
-	 * Stage stays renderer-agnostic by passing the raw scene data —
-	 * lights iterable and ambient color — and letting the renderer
-	 * decide how to encode them.
-	 * @param {Iterable<object>} [lights] - active `Light2d` instances; falsy/empty no-ops
-	 * @param {object} [ambient] - ambient lighting color (0..255 RGB)
-	 * @param {number} [translateX=0] - world-to-screen X translate (matches `Camera2d.draw()`)
-	 * @param {number} [translateY=0] - world-to-screen Y translate
-	 */
 	/**
 	 * Install the distance fog for the camera about to draw, or clear it with
 	 * `null`.
@@ -1304,29 +1284,64 @@ export default class Renderer {
 		this._fog3d = fog ?? null;
 	}
 
+	/**
+	 * Upload the active scene lights to the lit sprite pipeline.
+	 *
+	 * Called once per camera per frame by `Camera2d.draw()` (after the
+	 * FBO is bound, before the world tree walk fires `Sprite.draw` for
+	 * any normal-mapped sprite). The WebGL and WebGPU renderers override
+	 * this to pack the lights into the lit shader's uniform buffers. A light
+	 * only reaches a sprite with a normal map, on those renderers alone: the
+	 * Canvas renderer cannot light per pixel, so it only counts the lights,
+	 * and says once when it draws a sprite with a normal map under them.
+	 *
+	 * Stage stays renderer-agnostic by passing the raw scene data —
+	 * lights iterable and ambient color — and letting the renderer
+	 * decide how to encode them.
+	 * @param {Iterable<object>} [lights] - active `Light2d` instances; falsy/empty no-ops
+	 * @param {object} [ambient] - ambient lighting color (0..255 RGB)
+	 * @param {number} [translateX=0] - world-to-screen X translate (matches `Camera2d.draw()`)
+	 * @param {number} [translateY=0] - world-to-screen Y translate
+	 */
 	// eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
 	setLightUniforms(lights, ambient, translateX, translateY) {
-		if (this._litPipelineWarned || !lights) {
-			return;
+		// count them: an array's length, or by iterating
+		let count = 0;
+		if (lights) {
+			if (typeof lights.length === "number") {
+				count = lights.length;
+			} else if (typeof lights[Symbol.iterator] === "function") {
+				const it = lights[Symbol.iterator]();
+				while (!it.next().done) {
+					count++;
+				}
+			}
 		}
-		// detect "non-empty" via `Set.size`, array length, or by iterating
-		const hasAny =
-			(typeof lights.size === "number" && lights.size > 0) ||
-			(typeof lights.length === "number" && lights.length > 0) ||
-			(typeof lights[Symbol.iterator] === "function" &&
-				!lights[Symbol.iterator]().next().done);
-		if (hasAny) {
-			/**
-			 * @ignore
-			 * @internal
-			 */
-			this._litPipelineWarned = true;
-			console.warn(
-				"melonJS: Light2d normal-map lighting requires a GPU backend " +
-					"(WebGL 2 or WebGPU); the Canvas fallback renders sprites without " +
-					"per-pixel lighting. Light2d glow and stage lighting still draw.",
-			);
+		this.activeLightCount = count;
+	}
+
+	/**
+	 * On a renderer that cannot light per pixel (Canvas): say once that a
+	 * sprite with a normal map, drawn while lights shine, is drawn without
+	 * that lighting. A sprite without one is drawn unlit by every renderer,
+	 * so it has nothing to say about it.
+	 * @ignore
+	 * @internal
+	 */
+	_warnUnlitNormalMap() {
+		if (this._litPipelineWarned === true || this.activeLightCount === 0) {
+			return;
 		}
+		/**
+		 * @ignore
+		 * @internal
+		 */
+		this._litPipelineWarned = true;
+		console.warn(
+			"melonJS: Light2d normal-map lighting requires a GPU backend " +
+				"(WebGL 2 or WebGPU); the Canvas fallback renders sprites without " +
+				"per-pixel lighting. Light2d glow and stage lighting still draw.",
+		);
 	}
 
 	/**

packages/melonjs/tests/lights.spec.js

@@ -1582,41 +1582,70 @@ describe("Light2d + Stage lighting", () => {
 		});
 	});
 
-	describe("Renderer.setLightUniforms (Canvas fallback warning)", () => {
-		it("emits a one-shot console warning when called with count > 0 in Canvas mode", () => {
+	// A light only reaches a sprite with a normal map, and only on a GPU
+	// renderer: a sprite without one is drawn unlit everywhere. So the Canvas
+	// renderer has something to say only when it draws a normal-mapped sprite
+	// under lights. It used to warn as soon as any light existed, which every
+	// lit game without normal maps heard for nothing.
+	describe("the Canvas renderer's warning about normal-mapped sprites", () => {
+		const fakeLight = {
+			getBounds: () => {
+				return { centerX: 0, centerY: 0, width: 30, height: 30 };
+			},
+			intensity: 1,
+			color: { r: 255, g: 255, b: 255 },
+			lightHeight: 1,
+		};
+
+		// what console.warn was called with while `run` ran, from a renderer
+		// that has not warned yet
+		const warningsDuring = (run) => {
 			const renderer = app.renderer;
-			// Reset the once-flag on the live renderer so we can re-trigger.
 			renderer._litPipelineWarned = false;
-
 			const orig = console.warn;
 			const warnings = [];
 			console.warn = (...args) => {
 				return warnings.push(args.join(" "));
 			};
-
-			const fakeLight = {
-				getBounds: () => {
-					return { centerX: 0, centerY: 0, width: 30, height: 30 };
-				},
-				intensity: 1,
-				color: { r: 255, g: 255, b: 255 },
-				lightHeight: 1,
-			};
 			try {
-				renderer.setLightUniforms([], null, 0, 0); // empty — no warn
-				renderer.setLightUniforms([fakeLight], null, 0, 0); // first warn
-				renderer.setLightUniforms(
-					[fakeLight, fakeLight, fakeLight, fakeLight, fakeLight],
-					null,
-					0,
-					0,
-				);
-				renderer.setLightUniforms([fakeLight], null, 0, 0);
+				run(renderer);
 			} finally {
 				console.warn = orig;
+				renderer.setLightUniforms([], null, 0, 0);
+				renderer.currentNormalMap = null;
 			}
+			return warnings;
+		};
+
+		// a sprite drawn as the world draws it, so it hands the renderer its
+		// normal map as it does in a game
+		const drawSprite = (renderer, settings = {}) => {
+			const sprite = new Sprite(0, 0, {
+				image: Renderer.createCanvas(16, 16),
+				...settings,
+			});
+			sprite.preDraw(renderer);
+			sprite.draw(renderer);
+			sprite.postDraw(renderer);
+		};
+
+		it("says nothing about lights alone: a sprite without a normal map is unlit everywhere", () => {
+			const warnings = warningsDuring((renderer) => {
+				for (let i = 0; i < 10; i++) {
+					renderer.setLightUniforms([fakeLight, fakeLight], null, 0, 0);
+					drawSprite(renderer);
+				}
+			});
+			expect(warnings).toEqual([]);
+		});
 
-			// only the first non-empty call warns
+		it("warns once when it draws a sprite with a normal map under lights", () => {
+			const warnings = warningsDuring((renderer) => {
+				renderer.setLightUniforms([fakeLight], null, 0, 0);
+				for (let i = 0; i < 5; i++) {
+					drawSprite(renderer, { normalMap: Renderer.createCanvas(16, 16) });
+				}
+			});
 			expect(warnings.length).toBe(1);
 			// the message names a GPU *backend*, not WebGL specifically —
 			// WebGPU implements the lit pipeline too, so naming one renderer
@@ -1626,25 +1655,79 @@ describe("Light2d + Stage lighting", () => {
 			);
 		});
 
-		it("does NOT warn when count is 0 (every-frame default with no active lights)", () => {
-			const renderer = app.renderer;
-			renderer._litPipelineWarned = false;
-
-			const orig = console.warn;
-			const warnings = [];
-			console.warn = (...args) => {
-				return warnings.push(args.join(" "));
-			};
+		it("says nothing about a sprite with a normal map when no light shines", () => {
+			const warnings = warningsDuring((renderer) => {
+				renderer.setLightUniforms([], null, 0, 0);
+				drawSprite(renderer, { normalMap: Renderer.createCanvas(16, 16) });
+				renderer.setLightUniforms(null, null, 0, 0);
+				drawSprite(renderer, { normalMap: Renderer.createCanvas(16, 16) });
+			});
+			expect(warnings).toEqual([]);
+		});
 
+		it("counts the lights it is given, whatever holds them", () => {
+			const renderer = app.renderer;
 			try {
-				for (let i = 0; i < 10; i++) {
-					renderer.setLightUniforms([], null, 0, 0);
-				}
+				renderer.setLightUniforms([fakeLight, fakeLight], null, 0, 0);
+				expect(renderer.activeLightCount).toBe(2);
+				renderer.setLightUniforms(new Set([fakeLight]), null, 0, 0);
+				expect(renderer.activeLightCount).toBe(1);
+				renderer.setLightUniforms(
+					new Map([[1, fakeLight]]).values(),
+					null,
+					0,
+					0,
+				);
+				expect(renderer.activeLightCount).toBe(1);
+				renderer.setLightUniforms(undefined, null, 0, 0);
+				expect(renderer.activeLightCount).toBe(0);
 			} finally {
-				console.warn = orig;
+				renderer.setLightUniforms([], null, 0, 0);
 			}
+		});
+
+		// a whole frame with a light and the sprite in the world: the warnings,
+		// and whether the sprite was drawn at all (a culled one would pass the
+		// quiet case for nothing)
+		const frame = (settings) => {
+			const image = Renderer.createCanvas(16, 16);
+			const light = new Light2d(100, 100, 50);
+			const sprite = new Sprite(100, 100, { image, ...settings });
+			let drawn = false;
+			const warnings = warningsDuring((renderer) => {
+				const drawImage = renderer.drawImage;
+				renderer.drawImage = function (...args) {
+					drawn ||= args[0] === image;
+					return drawImage.apply(this, args);
+				};
+				try {
+					app.world.addChild(light);
+					app.world.addChild(sprite);
+					// the update finds the sprite in view, the draw draws it
+					app.world.update(16);
+					app.repaint();
+					app.draw();
+				} finally {
+					renderer.drawImage = drawImage;
+					app.world.removeChildNow(sprite);
+					app.world.removeChildNow(light);
+				}
+			});
+			return { drawn, warnings };
+		};
 
-			expect(warnings.length).toBe(0);
+		it("in a whole frame: a lit world without normal maps draws without a word", () => {
+			const { drawn, warnings } = frame({});
+			expect(drawn).toBe(true);
+			expect(warnings).toEqual([]);
+		});
+
+		it("in a whole frame: a normal-mapped sprite under a light is warned about", () => {
+			const { drawn, warnings } = frame({
+				normalMap: Renderer.createCanvas(16, 16),
+			});
+			expect(drawn).toBe(true);
+			expect(warnings.length).toBe(1);
 		});
 	});