fix: Polygon winding correct after negative transforms (#3063)

Fixes an issue brought in the discord where negative transforms would cause polygon winding to change causing collision logic to no longer function properly


https://github.com/excaliburjs/Excalibur/assets/612071/6f16b2a4-ac67-409a-9857-b2c51d70acbd

CHANGELOG.md

@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed issue where `ex.SpriteFont` did not respect scale when measuring text
+- Fixed issue where negative transforms would cause collision issues because polygon winding would change.
 
 ### Updates
 

src/engine/Collision/Colliders/PolygonCollider.ts

@@ -58,6 +58,7 @@ export class PolygonCollider extends Collider {
    */
   public set points(points: Vector[]) {
     this._points = points;
+    this._checkAndUpdateWinding(this._points);
     this.flagDirty();
   }
 
@@ -80,10 +81,6 @@ export class PolygonCollider extends Collider {
     this.offset = options.offset ?? Vector.Zero;
     this._globalMatrix.translate(this.offset.x, this.offset.y);
     this.points = options.points ?? [];
-    const counterClockwise = this._isCounterClockwiseWinding(this.points);
-    if (!counterClockwise) {
-      this.points.reverse();
-    }
 
     if (!this.isConvex()) {
       if (!options.suppressConvexWarning) {
@@ -98,6 +95,13 @@ export class PolygonCollider extends Collider {
     this._calculateTransformation();
   }
 
+  private _checkAndUpdateWinding(points: Vector[]) {
+    const counterClockwise = this._isCounterClockwiseWinding(points);
+    if (!counterClockwise) {
+      points.reverse();
+    }
+  }
+
   private _isCounterClockwiseWinding(points: Vector[]): boolean {
     // https://stackoverflow.com/a/1165943
     let sum = 0;
@@ -368,6 +372,9 @@ export class PolygonCollider extends Collider {
     for (let i = 0; i < len; i++) {
       this._transformedPoints[i] = this._globalMatrix.multiply(points[i].clone());
     }
+    // TODO possible optimization here only check if the transform has a potentially problematic value?
+    // it is possible for the transform to change the winding, scale (-1, 1) for example
+    this._checkAndUpdateWinding(this._transformedPoints);
   }
 
   /**

src/spec/CollisionShapeSpec.ts

@@ -573,6 +573,40 @@ describe('Collision Shape', () => {
       expect(contact.normal.y).toBeCloseTo(0, 0.01);
     });
 
+    it('can collide when the transform changes the winding', () => {
+      const polyA = new ex.PolygonCollider({
+        offset: ex.Vector.Zero.clone(),
+        // specified relative to the position
+        points: [new ex.Vector(-10, -10), new ex.Vector(10, -10), new ex.Vector(10, 10), new ex.Vector(-10, 10)]
+      });
+
+      const polyB = new ex.PolygonCollider({
+        offset: new ex.Vector(10, 0),
+        points: [new ex.Vector(-10, -10), new ex.Vector(10, -10), new ex.Vector(10, 10), new ex.Vector(-10, 10)]
+      });
+
+      const transform = new ex.Transform();
+      transform.scale = ex.vec(-1, 1);
+
+      polyA.update(transform);
+
+      const directionOfBodyB = polyB.center.sub(polyA.center);
+
+      const contact = polyA.collide(polyB)[0];
+
+      // there should be a collision
+      expect(contact).not.toBe(null);
+
+      // normal and mtv should point away from bodyA
+      expect(directionOfBodyB.dot(contact.mtv)).toBeGreaterThan(0);
+      expect(directionOfBodyB.dot(contact.normal)).toBeGreaterThan(0);
+
+      expect(contact.mtv.x).toBeCloseTo(10, 0.01);
+      expect(contact.normal.x).toBeCloseTo(1, 0.01);
+      expect(contact.mtv.y).toBeCloseTo(0, 0.01);
+      expect(contact.normal.y).toBeCloseTo(0, 0.01);
+    });
+
     it('can collide with the middle of an edge', () => {
       const actor = new ex.Actor({ x: 5, y: -6, width: 20, height: 20 });
       actor.rotation = Math.PI / 4;