fix: Path2D arc continuity, Rect geometry, loader asset hygiene (audit mediums)

packages/melonjs/CHANGELOG.md

@@ -25,6 +25,11 @@
 - **`moveTowards()` never reached its target — and returned the target instead of `this`** (all four vector classes) — the arrival test compared the remaining distance against `step * step` (wrong units: snapped way too early for steps above 1, dithered around the target forever for fractional steps), and on "arrival" it returned the **caller's target vector** without moving `this` at all — so the vector never landed, and chaining onto the result silently mutated the target. It now lands exactly on the target and always returns `this`, per its own documentation; negative-step flee semantics are unchanged.
 - **Every preloaded video was fetched with forced anonymous CORS** — the video parser unconditionally stamped the `crossorigin` attribute with the loader's `crossOrigin` setting, which defaults to `undefined` — and per the HTML spec an *invalid* value (the string `"undefined"`) maps to `anonymous`, unlike a *missing* attribute (no CORS). Cross-origin videos served without CORS headers failed to load outright when they would have played fine untainted. The attribute is now only set when `crossOrigin` is actually configured (the empty string remains a valid, honored value).
 - **Video preloading hung forever on autoplay-restricted browsers (e.g. iOS Safari) unless `autoplay: false` was spelled out** — the parser waited for `canplay`, which those browsers never fire for a video that isn't allowed to play, and only an *explicit* `autoplay: false` opted into the reliable `loadedmetadata` path — so the common manifest shape (no `autoplay` key) stalled the whole preloader and the game never started. Preloading now completes on `loadedmetadata` unless `autoplay: true` explicitly asks to wait for `canplay`.
+- **`Path2D.arc()`/`arcTo()`/`ellipse()` appended to an open path started a new sub-path instead of connecting** — the native Path2D spec joins an arc to the current point with a straight line (`arcTo`'s own documentation even promises it), but all three called `moveTo()` unconditionally. Under this release's new multi-sub-path fill semantics that stray sub-path is treated as a **hole**, so an arc appended to a filled outline punched a hole in the shape. They now connect per the spec; the canonical circle-hole idiom (explicit `moveTo` before `arc()`) still registers a hole, and a virgin-path `arc()` still starts cleanly.
+- **`Rect.right`/`Rect.bottom` returned the width/height when the edge sat exactly at coordinate 0** — the getters computed `this.left + w || w`, so an edge landing precisely on 0 (falsy) fell back to the size. Bounds and collision math around the origin came out wrong for such rectangles.
+- **`Rect.toPolygon()` handed out the rectangle's live vertices** — `Polygon.setVertices` stores a `Vector2d[]` by reference, so the "new" polygon shared the rect's actual points array: transforming or mutating the polygon silently corrupted the rectangle. The vertices are now cloned.
+- **`loader.load()` mutated the caller's asset descriptor, so retries double-prepended `baseURL`** — the resolved URL (fontface `url()` unwrap + `baseURL` prefix) was written back into `asset.src`; `loader.reload()` (which re-loads the same stored object) — or simply `load()`ing the same manifest entry twice — then fetched `base + base + src`. The URL is now resolved into a copy handed to the parser; the caller's object is never touched.
+- **`loader.unload()` of a never-loaded fontface threw a TypeError** — the fontface case was the only one without a membership guard, and `FontFaceSet.delete(undefined)` is a WebIDL type error rather than a `false` return. It now returns `false` like every other asset type.
 
 ## [19.8.0] (melonJS 2) - _2026-06-26_
 

packages/melonjs/src/geometries/path2d.ts

@@ -59,6 +59,16 @@ class Path2D {
 	 */
 	subPaths: number[] = [];
 
+	/**
+	 * whether the pen has been explicitly placed (via {@link moveTo}) since
+	 * the last {@link beginPath} — distinguishes "virgin path" from "pen
+	 * parked at (0, 0)" so {@link arc}/{@link ellipse}/{@link arcTo} know
+	 * whether to start fresh or connect from the current point (the native
+	 * Path2D behavior).
+	 * @ignore
+	 */
+	private penMoved = false;
+
 	/**
 	 * space between interpolated points for quadratic and bezier curve approx. in pixels.
 	 * @default 2
@@ -158,8 +168,9 @@ class Path2D {
 							ry,
 							xAxisRotation,
 						);
-						// generate arc points inline using lineTo (not ellipse/arc
-						// which call moveTo and break path continuity)
+						// generate arc points inline using lineTo — the SVG
+						// endpoint parameterization (large-arc/sweep flags)
+						// needs custom math that ellipse() doesn't expose
 						{
 							let { startAngle, endAngle } = p;
 							const {
@@ -285,6 +296,7 @@ class Path2D {
 		this.points.length = 0;
 		this.subPaths.length = 0;
 		this.startPoint.set(0, 0);
+		this.penMoved = false;
 	}
 
 	/**
@@ -383,9 +395,27 @@ class Path2D {
 	 */
 	moveTo(x: number, y: number) {
 		this.startPoint.set(x, y);
+		this.penMoved = true;
 		this.isDirty = true;
 	}
 
+	/**
+	 * Start the outline of an arc/ellipse at (x, y): per the native Path2D
+	 * spec, when the path already has content the arc is CONNECTED to the
+	 * current point with a straight line — only a virgin path (no points, no
+	 * explicit moveTo) starts fresh. An unconditional moveTo here would
+	 * silently start a new sub-path, which fill() treats as a HOLE cut out
+	 * of the shape.
+	 * @ignore
+	 */
+	private connectTo(x: number, y: number) {
+		if (this.points.length === 0 && !this.penMoved) {
+			this.moveTo(x, y);
+		} else {
+			this.lineTo(x, y);
+		}
+	}
+
 	/**
 	 * connects the last point in the current sub-path to the (x, y) coordinates with a straight line.
 	 * @param x - the x-axis coordinate of the line's end point.
@@ -490,7 +520,7 @@ class Path2D {
 		const dangle = diff / nr_of_interpolation_points;
 		const angleStep = dangle * direction;
 
-		this.moveTo(
+		this.connectTo(
 			x + radius * Math.cos(startAngle),
 			y + radius * Math.sin(startAngle),
 		);
@@ -556,7 +586,7 @@ class Path2D {
 		const tangent2_pointx = x1 + b0 * adj_l;
 		const tangent2_pointy = y1 + b1 * adj_l;
 
-		this.moveTo(tangent1_pointx, tangent1_pointy);
+		this.connectTo(tangent1_pointx, tangent1_pointy);
 
 		let bisec0 = (a0 + b0) / 2.0;
 		let bisec1 = (a1 + b1) / 2.0;
@@ -652,7 +682,7 @@ class Path2D {
 
 		const sx = radiusX * Math.cos(startAngle);
 		const sy = radiusY * Math.sin(startAngle);
-		this.moveTo(
+		this.connectTo(
 			x + sx * cos_rotation - sy * sin_rotation,
 			y + sx * sin_rotation + sy * cos_rotation,
 		);

packages/melonjs/src/geometries/rectangle.ts

@@ -54,8 +54,7 @@ export class Rect extends Polygon {
 	 * right coordinate of the Rectangle
 	 */
 	get right() {
-		const w = this.width;
-		return this.left + w || w;
+		return this.left + this.width;
 	}
 
 	/**
@@ -69,8 +68,7 @@ export class Rect extends Polygon {
 	 * bottom coordinate of the Rectangle
 	 */
 	get bottom() {
-		const h = this.height;
-		return this.top + h || h;
+		return this.top + this.height;
 	}
 
 	/**
@@ -290,7 +288,16 @@ export class Rect extends Polygon {
 	 * @returns a new Polygon that represents this rectangle.
 	 */
 	toPolygon() {
-		return polygonPool.get(this.left, this.top, this.points as PolygonVertices);
+		// clone the vertices: Polygon.setVertices stores a Vector2d[] by
+		// reference, and handing out this rect's live points would let any
+		// transform on the "new" polygon silently corrupt the rectangle
+		return polygonPool.get(
+			this.left,
+			this.top,
+			this.points.map((p) => {
+				return p.clone();
+			}) as PolygonVertices,
+		);
 	}
 }
 

packages/melonjs/src/loader/loader.js

@@ -545,24 +545,36 @@ export function load(asset, onload, onerror) {
 		initParsers();
 	}
 
+	// Resolve the effective src WITHOUT mutating the caller's asset
+	// descriptor: load() used to write the transformed url back into
+	// asset.src, so retrying the same object — loader.reload() after a
+	// failure, or simply load()ing the same manifest entry twice —
+	// prepended baseURL onto an already-transformed src and fetched a
+	// mangled URL. Caches, events and failure tracking all key on the
+	// caller's original src; only the parser sees the resolved one.
+	let src = asset.src;
+
 	// strip url() wrapper for fontface assets so baseURL can be prepended to the raw path
-	if (asset.type === "fontface" && typeof asset.src === "string") {
-		const urlMatch = asset.src.match(/^url\(\s*['"]?(.*?)['"]?\s*\)$/);
+	if (asset.type === "fontface" && typeof src === "string") {
+		const urlMatch = src.match(/^url\(\s*['"]?(.*?)['"]?\s*\)$/);
 		if (urlMatch) {
-			asset.src = urlMatch[1];
+			src = urlMatch[1];
 		}
 	}
 
 	// transform the url if necessary (skip for local() font sources and data URIs)
 	if (
 		typeof baseURL[asset.type] !== "undefined" &&
-		typeof asset.src === "string" &&
-		!asset.src.startsWith("local(") &&
-		!asset.src.startsWith("data:")
+		typeof src === "string" &&
+		!src.startsWith("local(") &&
+		!src.startsWith("data:")
 	) {
-		asset.src = baseURL[asset.type] + asset.src;
+		src = baseURL[asset.type] + src;
 	}
 
+	const resource =
+		src === asset.src ? asset : Object.assign({}, asset, { src });
+
 	const parser = parsers.get(asset.type);
 
 	if (typeof parser === "undefined") {
@@ -584,7 +596,7 @@ export function load(asset, onload, onerror) {
 			// if it splits into several); 0 means already cached → resolve now.
 			const count = parser.call(
 				this,
-				asset,
+				resource,
 				() => {
 					resolve();
 				},
@@ -598,7 +610,7 @@ export function load(asset, onload, onerror) {
 	}
 
 	// parser returns the amount of asset to be loaded (usually 1 unless an asset is splitted into several ones)
-	return parser.call(this, asset, onload, onerror, settings);
+	return parser.call(this, resource, onload, onerror, settings);
 }
 
 /**
@@ -639,6 +651,11 @@ export function unload(asset) {
 			return true;
 
 		case "fontface":
+			// membership guard like every other type — FontFaceSet.delete is
+			// WebIDL-typed and THROWS on undefined instead of returning false
+			if (!(asset.name in fontList)) {
+				return false;
+			}
 			if (
 				typeof globalThis.document !== "undefined" &&
 				typeof globalThis.document.fonts !== "undefined"

packages/melonjs/tests/loader-asset-src.spec.js

@@ -0,0 +1,36 @@
+import { afterAll, describe, expect, it } from "vitest";
+import { loader } from "../src/index.js";
+
+describe("loader asset descriptor hygiene (audit fixes)", () => {
+	afterAll(() => {
+		loader.setBaseURL("json", "");
+	});
+
+	it("load() leaves asset.src untouched, so retrying the same object never double-prepends baseURL", async () => {
+		loader.setBaseURL("json", "missing-fixture-dir/");
+		const asset = {
+			name: "src-mutation-probe",
+			type: "json",
+			src: "probe.json",
+		};
+
+		// pre-fix: load() wrote the baseURL-prepended url back into asset.src,
+		// so loader.reload() (which re-loads the SAME stored object) — or any
+		// caller retrying its own manifest entry — fetched base+base+src
+		await expect(loader.load(asset)).rejects.toThrow();
+		expect(asset.src).toBe("probe.json");
+
+		await expect(loader.load(asset)).rejects.toThrow();
+		expect(asset.src).toBe("probe.json");
+	});
+
+	it("unload() of a never-loaded fontface returns false instead of throwing", () => {
+		// pre-fix: document.fonts.delete(fontList[name]) with undefined threw a
+		// WebIDL TypeError — every other asset type guards membership first
+		let result;
+		expect(() => {
+			result = loader.unload({ name: "never-loaded-ff", type: "fontface" });
+		}).not.toThrow();
+		expect(result).toBe(false);
+	});
+});

packages/melonjs/tests/path2d-arc-connect.spec.ts

@@ -0,0 +1,91 @@
+import { describe, expect, it } from "vitest";
+import Path2D from "../src/geometries/path2d.ts";
+
+/**
+ * Per the native Path2D spec, arc()/ellipse() appended to a non-empty path
+ * are CONNECTED to the current point with a straight line, and arcTo() is
+ * "connected to the previous point by a straight line" by definition (it
+ * says so in its own JSDoc). Our implementations instead called moveTo()
+ * unconditionally, silently starting a new sub-path.
+ *
+ * Written failing-first: with the 19.9 multi-sub-path fill semantics (every
+ * sub-path after the first is a HOLE), an appended arc didn't just miss its
+ * connecting line — it punched a hole in the filled shape. The SVG `A`
+ * parser even worked around this internally rather than calling arc().
+ */
+describe("Path2D arc/arcTo/ellipse continuity (native Path2D semantics)", () => {
+	it("arc() appended to an open path connects with a line instead of starting a hole sub-path", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.moveTo(0, 0);
+		p.lineTo(100, 0);
+		// arc starting at (100, 40): the spec adds the segment (100,0) → (100,40)
+		p.arc(100, 50, 10, -Math.PI / 2, Math.PI / 2);
+		expect(p.subPaths.length).toBe(0);
+		// the connecting segment is part of the outline
+		expect(p.points[2].x).toBeCloseTo(100, 6);
+		expect(p.points[2].y).toBeCloseTo(0, 6);
+		expect(p.points[3].x).toBeCloseTo(100, 6);
+		expect(p.points[3].y).toBeCloseTo(40, 6);
+	});
+
+	it("ellipse() appended to an open path connects instead of starting a hole sub-path", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.moveTo(0, 0);
+		p.lineTo(50, 0);
+		// ellipse outline starts at (50, 20)
+		p.ellipse(50, 30, 20, 10, 0, -Math.PI / 2, Math.PI / 2);
+		expect(p.subPaths.length).toBe(0);
+	});
+
+	it("arcTo() appended off the tangent line connects instead of starting a hole sub-path", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.moveTo(0, 0);
+		p.lineTo(0, 20);
+		// pen (0,20) is NOT on the incoming tangent — the spec still connects
+		p.arcTo(50, 0, 50, 50, 20);
+		expect(p.subPaths.length).toBe(0);
+	});
+
+	it("arcTo() after a bare moveTo keeps the connecting line from the pen", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.moveTo(0, 0);
+		p.arcTo(50, 0, 50, 50, 20);
+		// native canvas draws (0,0) → tangent (30,0); the old moveTo dropped it
+		expect(p.points[0].x).toBeCloseTo(0, 6);
+		expect(p.points[0].y).toBeCloseTo(0, 6);
+	});
+
+	it("arc() on a virgin path still starts cleanly at the arc start (no spurious origin line)", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.arc(50, 50, 40, 0, Math.PI * 2);
+		expect(p.subPaths.length).toBe(0);
+		expect(p.points[0].x).toBeCloseTo(90, 6);
+		expect(p.points[0].y).toBeCloseTo(50, 6);
+	});
+
+	it("explicit moveTo sub-paths (holes) still register", () => {
+		const p = new Path2D();
+		p.beginPath();
+		// outer triangle
+		p.moveTo(0, 0);
+		p.lineTo(100, 0);
+		p.lineTo(50, 80);
+		// inner hole — an explicit moveTo must STILL start a sub-path
+		p.moveTo(40, 20);
+		p.lineTo(60, 20);
+		p.lineTo(50, 40);
+		expect(p.subPaths.length).toBe(1);
+	});
+
+	it("roundRect() stays a single sub-path", () => {
+		const p = new Path2D();
+		p.beginPath();
+		p.roundRect(10, 10, 80, 50, 12);
+		expect(p.subPaths.length).toBe(0);
+	});
+});

packages/melonjs/tests/path2d.spec.js

@@ -724,10 +724,26 @@ describe("Path2D", () => {
 			expect(path.points[path.subPaths[0]].y).toBe(200);
 		});
 
-		it("a far disjoint arc() mid-path records a pen-up boundary", () => {
+		it("a far arc() mid-path CONNECTS from the current point (native Path2D semantics)", () => {
+			// unlike rect() above, native arc() never starts a sub-path on its
+			// own — it always joins from the current point with a straight
+			// line. (This originally asserted the pre-fix auto-detach, which
+			// was the accident of an unconditional moveTo inside arc().)
 			path.moveTo(0, 0);
 			path.lineTo(50, 0);
 			path.arc(300, 300, 20, 0, Math.PI);
+			expect(path.subPaths.length).toBe(0);
+		});
+
+		it("the canonical circle-hole idiom — explicit moveTo before arc() — records the boundary", () => {
+			// outer square
+			path.moveTo(0, 0);
+			path.lineTo(100, 0);
+			path.lineTo(100, 100);
+			path.lineTo(0, 100);
+			// pen-up to the arc start, then the hole circle
+			path.moveTo(70, 50);
+			path.arc(50, 50, 20, 0, Math.PI * 2);
 			expect(path.subPaths.length).toBe(1);
 		});
 

packages/melonjs/tests/rect-geometry.spec.ts

@@ -0,0 +1,24 @@
+import { describe, expect, it } from "vitest";
+import { Rect } from "../src/index.js";
+
+describe("Rect geometry (audit fixes)", () => {
+	it("right/bottom are exact when the edge sits at coordinate 0", () => {
+		// pre-fix: `this.left + w || w` — a right/bottom edge landing exactly
+		// on 0 is falsy, so the getter returned the width/height instead
+		const r = new Rect(-100, -50, 100, 50);
+		expect(r.right).toBe(0);
+		expect(r.bottom).toBe(0);
+	});
+
+	it("toPolygon() returns an independent polygon (no shared live vertices)", () => {
+		const r = new Rect(10, 20, 30, 40);
+		const p = r.toPolygon();
+		// pre-fix: setVertices stored the rect's own Vector2d array by
+		// reference — mutating the polygon corrupted the rectangle
+		p.points[0].set(999, 999);
+		expect(r.points[0].x).toBe(0);
+		expect(r.points[0].y).toBe(0);
+		expect(r.width).toBe(30);
+		expect(r.height).toBe(40);
+	});
+});