fix(physics, tilemap): ellipse SAT sign + stale bounds on centered level

packages/melonjs/CHANGELOG.md

@@ -11,6 +11,8 @@
 
 ### Fixed
 - WebGL: `MaterialBatcher.uploadTexture` was using its `w` and `h` parameters (the destination quad size, not the texture's) for the `isPOT` check, which drives both the wrap-mode fallback and the `generateMipmap` gate. Visible as a `GL_INVALID_OPERATION` from `gl.generateMipmap` on WebGL 1; silent wasted work (unnecessary mipmaps, wrong `isPOT`-derived state) on WebGL 2. Texture dimensions are now derived from the source itself.
+- SAT: ellipse collisions silently failed whenever the body's ancestor container had a non-zero absolute position (the typical case: `level.load` auto-centers the level container when the viewport is larger than the map, setting `container.pos` to a non-zero offset). `testEllipseEllipse` and `testPolygonEllipse` built the relative-position vector by *adding* `a.ancestor.getAbsolutePosition()` where they should have subtracted it, shifting the circle by `2 * ancestor.absPos`. The polygon/polygon path is unaffected — it builds two absolute positions and lets `isSeparatingAxis` do the subtraction. Latent because every existing SAT unit test wired the mock ancestor to `(0, 0)`, where the sign error is arithmetically invisible.
+- TMX: static children of an auto-centered level container kept stale absolute bounds. `TMXTileMap.addTo` sets `container.pos` *after* adding children, so each child's cached absolute bounds (computed at `addChild` time) didn't include the centering offset. Children that moved on their own refreshed via the `pos` observer, but TMX layers, Tiled collision shapes, triggers, and decorative sprites stayed stuck at their pre-centering bounds — visible as debug overlay shapes drawn at the wrong screen position, and as broken viewport culling for anything outside the pre-centering box. `_setBounds` now walks the container subtree and refreshes absolute bounds after the position actually moves (both initial load and viewport resize).
 
 ### Changed
 - WebGL 1: removed the unconditional `[Texture] ... is not a POT texture` warning. The engine handles NPOT correctly (clamp wrap, non-mipmapped filters). A targeted warning now fires only when `repeat: "repeat*"` is requested on an NPOT texture under WebGL 1, the one case where the user's intent is silently downgraded.

packages/melonjs/src/level/tiled/TMXTileMap.js

@@ -115,6 +115,23 @@ function readObjectGroup(map, data, z) {
 	return new TMXGroup(map, data, z);
 }
 
+/**
+ * Recursively recompute absolute bounds for a container and all its
+ * descendants. Used when the container's pos changes (centering on a wider
+ * viewport): children cache their absolute bounds at addChild time, and
+ * those caches are stale until each child happens to call updateBounds on
+ * its own (which only happens organically when its own pos changes).
+ * @ignore
+ */
+function refreshAbsoluteBounds(container) {
+	container.forEach((child) => {
+		child.updateBounds(true);
+		if (child instanceof Container) {
+			refreshAbsoluteBounds(child);
+		}
+	});
+}
+
 /**
  * a TMX Tile Map Object
  * Tiled QT +0.7.x format
@@ -425,12 +442,20 @@ export default class TMXTileMap {
 					Math.max(levelBounds.height, height),
 				);
 				// center the map if smaller than the current viewport
-				container.pos.set(
-					Math.max(0, ~~((width - levelBounds.width) / 2)),
-					Math.max(0, ~~((height - levelBounds.height) / 2)),
-					// don't change the container z position if defined
-					container.pos.z,
-				);
+				const newX = Math.max(0, ~~((width - levelBounds.width) / 2));
+				const newY = Math.max(0, ~~((height - levelBounds.height) / 2));
+				if (container.pos.x !== newX || container.pos.y !== newY) {
+					container.pos.set(newX, newY, container.pos.z);
+					// A child caches its absolute bounds (centered around
+					// ancestor.absPos at the time of the last `updateBounds`).
+					// When the parent's pos changes, descendants that don't
+					// move on their own (TMX layers, static collision
+					// objects, triggers) keep stale absolute bounds, so
+					// anything that reads them (debug overlay, viewport
+					// culling) sees the pre-centering position. Walk the
+					// tree once after a real move and refresh.
+					refreshAbsoluteBounds(container);
+				}
 			}
 
 			off(VIEWPORT_ONRESIZE, _setBounds);

packages/melonjs/src/physics/sat.js

@@ -300,13 +300,19 @@ export function testEllipseEllipse(a, ellipseA, b, ellipseB, response) {
 
 	// Fast path: both are circles
 	// Check if the distance between the centers of the two
-	// circles is greater than their combined radius.
+	// circles is greater than their combined radius. We need the absolute
+	// position of B relative to A — both sides resolve their world position
+	// via ancestor.getAbsolutePosition(), so A's offset must be subtracted
+	// too, not added. (The bug was invisible when ancestor.absPos == 0, e.g.
+	// the level was the same size as the viewport; it manifests as missed
+	// circle collisions whenever the container is shifted, such as TMX
+	// auto-centering on a wider viewport.)
 	const differenceV = T_VECTORS[--T_VECTORS_IDX]
 		.copy(b.pos)
 		.add(b.ancestor.getAbsolutePosition())
 		.add(ellipseB.pos)
 		.sub(a.pos)
-		.add(a.ancestor.getAbsolutePosition())
+		.sub(a.ancestor.getAbsolutePosition())
 		.sub(ellipseA.pos);
 	const radiusA = ellipseA.radius;
 	const radiusB = ellipseB.radius;
@@ -350,13 +356,19 @@ export function testPolygonEllipse(a, polyA, b, ellipseB, response) {
 	}
 
 	// Fast path: ellipse is a circle
-	// Get the position of the circle relative to the polygon.
+	// Get the position of the circle relative to the polygon. Both A and B
+	// resolve their world position via ancestor.getAbsolutePosition(), so
+	// A's ancestor offset must be subtracted (matches the polygon/polygon
+	// path which builds absolute positions). The previous `+a.ancestor.abs`
+	// was masked whenever the level container was at the origin, but missed
+	// every circle-vs-polygon collision once the container was shifted (TMX
+	// auto-centering on a wider viewport, manually translated containers).
 	const circlePos = T_VECTORS[--T_VECTORS_IDX]
 		.copy(b.pos)
 		.add(b.ancestor.getAbsolutePosition())
 		.add(ellipseB.pos)
 		.sub(a.pos)
-		.add(a.ancestor.getAbsolutePosition())
+		.sub(a.ancestor.getAbsolutePosition())
 		.sub(polyA.pos);
 	const radius = ellipseB.radius;
 	const radius2 = radius * radius;

packages/melonjs/tests/sat.spec.js

@@ -12,14 +12,16 @@ import {
  * Helper to create a mock renderable with position and ancestor
  * (SAT functions expect objects with .pos and .ancestor.getAbsolutePosition())
  */
-function createMockRenderable(x, y) {
+function createMockRenderable(x, y, ancestorOffset) {
 	const renderable = new Renderable(x, y, 0, 0);
 	renderable.anchorPoint.set(0, 0);
-	// ensure ancestor chain returns zero offset
+	const offset =
+		ancestorOffset instanceof Vector2d ? ancestorOffset : new Vector2d(0, 0);
+	// ensure ancestor chain returns the requested offset
 	if (typeof renderable.ancestor === "undefined") {
 		renderable.ancestor = {
 			getAbsolutePosition() {
-				return new Vector2d(0, 0);
+				return offset;
 			},
 		};
 	}
@@ -711,4 +713,189 @@ describe("Physics : SAT (Separating Axis Theorem)", () => {
 			}
 		});
 	});
+
+	// Regression: SAT must respect a non-zero ancestor.getAbsolutePosition()
+	// the same way for both colliders. Both operands resolve their world
+	// position via ancestor.getAbsolutePosition(), so the relative-position
+	// vector must subtract A's ancestor offset (the polygon/polygon path
+	// builds two absolute positions and subtracts inside isSeparatingAxis,
+	// which is correct; the ellipse paths build the difference directly
+	// and used to ADD A's ancestor offset by mistake). This was invisible
+	// whenever ancestor.abs was (0, 0), so a TMX level the same size as
+	// the viewport hid the bug; it broke every circle collision the moment
+	// the level container was shifted (`level.load` auto-centering on a
+	// wider viewport, or any manually translated container).
+	describe("non-zero ancestor offset", () => {
+		let response;
+		beforeEach(() => {
+			response = new ResponseObject();
+		});
+
+		// Each entry exercises one shape-type combination and asserts that
+		// (a) overlap detection still fires when both bodies share a
+		// non-zero ancestor offset, (b) the response overlap matches the
+		// zero-offset baseline (shift-invariance), and (c) the concrete
+		// near-miss geometry the platformer hits is detected.
+		const offset = new Vector2d(255, 0);
+		const makeCases = () => {
+			return [
+				{
+					name: "Polygon vs Polygon (Rect/Rect via testPolygonPolygon)",
+					fn: testPolygonPolygon,
+					shapeA: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					shapeB: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					// Polygon/polygon was already correct (isSeparatingAxis
+					// subtracts internally), so this entry guards against a
+					// future regression rather than reproducing today's bug.
+					preFixWouldMiss: false,
+				},
+				{
+					name: "Polygon vs Polygon (custom Polygon/Rect via testPolygonPolygon)",
+					fn: testPolygonPolygon,
+					shapeA: () => {
+						return new Polygon(0, 0, [
+							new Vector2d(0, 0),
+							new Vector2d(30, 0),
+							new Vector2d(15, 40),
+						]);
+					},
+					shapeB: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					preFixWouldMiss: false,
+				},
+				{
+					name: "Polygon vs Circle (Rect/Circle via testPolygonEllipse)",
+					fn: testPolygonEllipse,
+					shapeA: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					shapeB: () => {
+						return new Ellipse(0, 0, 40, 40);
+					},
+					preFixWouldMiss: true,
+				},
+				{
+					name: "Polygon vs true Ellipse (Rect/Ellipse via testPolygonEllipse)",
+					fn: testPolygonEllipse,
+					shapeA: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					// non-square radii → falls through to testPolygonPolygon
+					// internally, so this exercises the conversion path too.
+					shapeB: () => {
+						return new Ellipse(0, 0, 40, 24);
+					},
+					preFixWouldMiss: false,
+				},
+				{
+					name: "Circle vs Polygon (Circle/Rect via testEllipsePolygon)",
+					fn: testEllipsePolygon,
+					shapeA: () => {
+						return new Ellipse(0, 0, 40, 40);
+					},
+					shapeB: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					preFixWouldMiss: true,
+				},
+				{
+					name: "true Ellipse vs Polygon (Ellipse/Rect via testEllipsePolygon)",
+					fn: testEllipsePolygon,
+					shapeA: () => {
+						return new Ellipse(0, 0, 40, 24);
+					},
+					shapeB: () => {
+						return new Rect(0, 0, 40, 40).toPolygon();
+					},
+					preFixWouldMiss: false,
+				},
+				{
+					name: "Circle vs Circle (testEllipseEllipse fast path)",
+					fn: testEllipseEllipse,
+					shapeA: () => {
+						return new Ellipse(0, 0, 40, 40);
+					}, // radius 20
+					shapeB: () => {
+						return new Ellipse(0, 0, 40, 40);
+					},
+					preFixWouldMiss: true,
+				},
+				{
+					name: "Circle vs true Ellipse (testEllipseEllipse mixed)",
+					fn: testEllipseEllipse,
+					shapeA: () => {
+						return new Ellipse(0, 0, 40, 40);
+					},
+					shapeB: () => {
+						return new Ellipse(0, 0, 40, 24);
+					},
+					preFixWouldMiss: true,
+				},
+				{
+					name: "true Ellipse vs true Ellipse (testEllipseEllipse polygonized)",
+					fn: testEllipseEllipse,
+					shapeA: () => {
+						return new Ellipse(0, 0, 40, 24);
+					},
+					shapeB: () => {
+						return new Ellipse(0, 0, 40, 24);
+					},
+					preFixWouldMiss: false,
+				},
+			];
+		};
+
+		for (const c of makeCases()) {
+			it(`${c.name}: shift-invariant under a common ancestor offset`, () => {
+				const aRef = createMockRenderable(0, 0);
+				const bRef = createMockRenderable(15, 0);
+				const refResp = new ResponseObject();
+				const refHit = c.fn(aRef, c.shapeA(), bRef, c.shapeB(), refResp);
+				expect(refHit).toBe(true);
+
+				const aShifted = createMockRenderable(0, 0, offset);
+				const bShifted = createMockRenderable(15, 0, offset);
+				const shiftedHit = c.fn(
+					aShifted,
+					c.shapeA(),
+					bShifted,
+					c.shapeB(),
+					response,
+				);
+				expect(shiftedHit).toBe(true);
+				expect(response.overlap).toBeCloseTo(refResp.overlap);
+			});
+
+			if (c.preFixWouldMiss) {
+				it(`${c.name}: pre-fix sign error mis-placed the circle by 2 * ancestor.abs`, () => {
+					// Geometry chosen so that the bodies overlap normally,
+					// but the buggy +offset would push them ~510 px apart
+					// (far beyond any reasonable shape extent).
+					const a = createMockRenderable(0, 0, offset);
+					const b = createMockRenderable(10, 0, offset);
+					expect(c.fn(a, c.shapeA(), b, c.shapeB(), response)).toBe(true);
+				});
+			}
+		}
+
+		// Concrete repro of the platformer's player-polygon vs coin-circle
+		// collision at world.pos (255, 0). Without the SAT fix the circle's
+		// computed position is +510 px from where it should be, missing
+		// the polygon entirely.
+		it("platformer regression: player Rect vs coin Circle inside a shifted level container", () => {
+			const offset = new Vector2d(255, 0);
+			const player = createMockRenderable(785, 790, offset);
+			const coin = createMockRenderable(840, 805, offset);
+			const playerPoly = new Rect(20, 8, 35, 88).toPolygon();
+			const coinCircle = new Ellipse(17.5, 17.5, 35, 35); // radius 17.5
+			expect(
+				testPolygonEllipse(player, playerPoly, coin, coinCircle, response),
+			).toBe(true);
+		});
+	});
 });