Repository: playcanvas/engine
Pull request #5196 (2023-03-27 15:45:03)

Updates to Plane’s class constructor, to take a distance instead of a point
based on a feedback.


Patch:
diff --git a/scripts/utils/planar-renderer.js b/scripts/utils/planar-renderer.js
index 0a782c810..02864f0a8 100644
--- a/scripts/utils/planar-renderer.js
+++ b/scripts/utils/planar-renderer.js
@@ -128,8 +128,8 @@ PlanarRenderer.prototype.frameUpdate = function () {
     if (planarCamera.enabled) {
 
         // update reflection camera orientation by mirroring the scene camera by the plane
-        this.plane.set(this.planeNormal, this.planePoint);
-        this.reflectionMatrix.setReflection(this.plane);
+        this.plane.setFromPointNormal(this.planePoint, this.planeNormal);
+        this.reflectionMatrix.setReflection(this.plane.normal, this.plane.distance);
 
         var pos = this.sceneCameraEntity.getPosition();
         var reflectedPos = this.reflectionMatrix.transformPoint(pos);
diff --git a/src/core/math/mat4.js b/src/core/math/mat4.js
index 80c27ba5a..7ecb0d049 100644
--- a/src/core/math/mat4.js
+++ b/src/core/math/mat4.js
@@ -782,16 +782,15 @@ class Mat4 {
      * Sets the matrix to a reflection matrix, which can be used as a mirror transformation by the
      * plane.
      *
-     * @param {import('./plane.js').Plane} plane - The plane to reflect by.
+     * @param {Vec3} normal - The normal of the plane to reflect by.
+     * @param {number} distance - The distance of plane to reflect by.
      * @returns {Mat4} Self for chaining.
      */
-    setReflection(plane) {
+    setReflection(normal, distance) {
 
-        const normal = plane.normal;
         const a = normal.x;
         const b = normal.y;
         const c = normal.z;
-        const d = plane.distance;
         const data = this.data;
 
         data[0] = 1.0 - 2 * a * a;
@@ -806,9 +805,9 @@ class Mat4 {
         data[9] = -2 * b * c;
         data[10] = 1.0 - 2 * c * c;
         data[11] = 0;
-        data[12] = -2 * a * d;
-        data[13] = -2 * b * d;
-        data[14] = -2 * c * d;
+        data[12] = -2 * a * distance;
+        data[13] = -2 * b * distance;
+        data[14] = -2 * c * distance;
         data[15] = 1;
 
         return this;
diff --git a/src/core/shape/plane.js b/src/core/shape/plane.js
index 48d61e348..c9b6aa1e3 100644
--- a/src/core/shape/plane.js
+++ b/src/core/shape/plane.js
@@ -14,7 +14,7 @@ class Plane {
     normal = new Vec3();
 
     /**
-     * The distance from the plan to the origin, along its normal.
+     * The distance from the plane to the origin, along its normal.
      *
      * @readonly
      * @type {number}
@@ -25,22 +25,23 @@ class Plane {
      * Create a new Plane instance.
      *
      * @param {Vec3} [normal] - Normal of the plane. The constructor copies this parameter. Defaults
-     * to {@link pc.Vec3#UP}.
-     * @param {Vec3} [point] - Point position on the plane. The constructor copies this parameter.
-     * Defaults to {@link pc.Vec3#ZERO}.
+     * to {@link Vec3#UP}.
+     * @param {Vec3} [distance] - The distance from the plane to the origin, along its normal.
+     * Defaults to 0.
      */
-    constructor(normal = Vec3.UP, point = Vec3.ZERO) {
-        this.set(normal, point);
+    constructor(normal = Vec3.UP, distance = 0) {
+        this.normal.copy(normal);
+        this.distance = distance;
     }
 
     /**
-     * Sets the plane to use a specified normal and pass through a specified point.
+     * Sets the plane based on a specified normal and a point on the plane.
      *
+     * @param {Vec3} point - The point on the plane.
      * @param {Vec3} normal - The normal of the plane.
-     * @param {Vec3} point - The point on a plane.
      * @returns {Plane} Self for chaining.
      */
-    set(normal, point) {
+    setFromPointNormal(point, normal) {
         this.normal.copy(normal);
         this.distance = -this.normal.dot(point);
         return this;
diff --git a/src/framework/components/element/element-drag-helper.js b/src/framework/components/element/element-drag-helper.js
index 301ab67aa..c15be7a74 100644
--- a/src/framework/components/element/element-drag-helper.js
+++ b/src/framework/components/element/element-drag-helper.js
@@ -161,7 +161,7 @@ class ElementDragHelper extends EventHandler {
         }
 
         _normal.copy(this._element.entity.forward).mulScalar(-1);
-        _plane.set(_normal, this._element.entity.getPosition());
+        _plane.setFromPointNormal(this._element.entity.getPosition(), _normal);
 
         if (_plane.intersectsRay(_ray, _point)) {
             _entityRotation.copy(this._element.entity.getRotation()).invert().transformVector(_point, _point);


Test patch:
diff --git a/test/core/shape/plane.test.mjs b/test/core/shape/plane.test.mjs
index b9a63e0b1..54318012d 100644
--- a/test/core/shape/plane.test.mjs
+++ b/test/core/shape/plane.test.mjs
@@ -16,18 +16,30 @@ describe('Plane', function () {
         });
 
         it('supports arguments', function () {
-            const p = new Plane(Vec3.UP, new Vec3(1, 2, 3));
+            const p = new Plane(Vec3.UP, 5);
             expect(p.normal.x).to.equal(0);
             expect(p.normal.y).to.equal(1);
             expect(p.normal.z).to.equal(0);
-            expect(p.distance).to.equal(-2);
+            expect(p.distance).to.equal(5);
         });
 
     });
 
+    describe('#setFromPointNormal', function () {
+
+        const p = new Plane();
+        p.setFromPointNormal(new Vec3(1, 2, 3), Vec3.UP);
+        expect(p.normal.x).to.equal(0);
+        expect(p.normal.y).to.equal(1);
+        expect(p.normal.z).to.equal(0);
+        expect(p.distance).to.equal(-2);
+
+    });
+
     describe('#intersectsLine', function () {
 
-        const p = new Plane(Vec3.UP, new Vec3(0, 5, 0));
+        const p = new Plane();
+        p.setFromPointNormal(new Vec3(0, 5, 0), Vec3.UP);
         const intersection = new Vec3();
         const intersects = p.intersectsLine(new Vec3(1, 0, 3), new Vec3(1, 6, 3), intersection);
         expect(intersects).to.equal(true);