From 4a8e0d48131a1f7c7e57f32b9b234a26f1041375 Mon Sep 17 00:00:00 2001 From: AryanKoundal Date: Fri, 10 Feb 2023 21:47:05 +0530 Subject: [PATCH 1/5] resolves issue#5958 3D orbitControl allowing +-90 degrees not enough --- src/webgl/p5.Camera.js | 19 +++++++------- test/unit/webgl/p5.Camera.js | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/webgl/p5.Camera.js b/src/webgl/p5.Camera.js index 66a55ffa24..ffbefb5b4d 100644 --- a/src/webgl/p5.Camera.js +++ b/src/webgl/p5.Camera.js @@ -1720,22 +1720,21 @@ p5.Camera.prototype._orbit = function(dTheta, dPhi, dRadius) { let camTheta = Math.atan2(diffX, diffZ); // equatorial angle let camPhi = Math.acos(Math.max(-1, Math.min(1, diffY / camRadius))); // polar angle - // add change - camTheta += dTheta; - camPhi += dPhi; - camRadius += dRadius; + // add change according to the direction of this.upY + let directionflag = this.upY > 0 ? 1 : -1; + camTheta += directionflag*dTheta; + camPhi += directionflag*dPhi; + if (camPhi <= 0 || camPhi>=Math.PI) { + directionflag *= -1; + } + camRadius += dRadius; // prevent zooming through the center: if (camRadius < 0) { camRadius = 0.1; } // prevent rotation over the zenith / under bottom - if (camPhi > Math.PI) { - camPhi = Math.PI; - } else if (camPhi <= 0) { - camPhi = 0.001; - } // from https://github.com/mrdoob/three.js/blob/dev/src/math/Vector3.js#L628-L632 const _x = Math.sin(camPhi) * camRadius * Math.sin(camTheta); @@ -1750,7 +1749,7 @@ p5.Camera.prototype._orbit = function(dTheta, dPhi, dRadius) { this.centerY, this.centerZ, 0, - 1, + directionflag, 0 ); }; diff --git a/test/unit/webgl/p5.Camera.js b/test/unit/webgl/p5.Camera.js index 9fff7e1fcc..7cb6f3379e 100644 --- a/test/unit/webgl/p5.Camera.js +++ b/test/unit/webgl/p5.Camera.js @@ -449,6 +449,53 @@ suite('p5.Camera', function() { assert.deepEqual(myCam.cameraMatrix.mat4, expectedMatrix); }); + + test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi <= 0', function() { + // the following should produce the upY with inverted direction(from 1 to -1) + // when camPhi changes from positive to negative or zero + myCam._orbit(0, -PI, 0); + // upY should switch from 1(dPhi=0) to -1 (dPhi=-PI) + // myCam.upY should be -1 + assert(myCam.upY === -1); + }); + + test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi <= 0', function() { + // the following should produce the upY with inverted direction(from -1 to 1) + // when camPhi changes from negative to positive or zero + myCam._orbit(0, -PI, 0); + myCam._orbit(0, PI, 0); + // upY should switch from -1(dPhi=-PI) to 1 (dPhi=PI) + // myCam.upY should be 1 + assert(myCam.upY === 1); + }); + + test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi >= PI', function() { + // the following should produce the upY with inverted direction(from 1 to -1) + // when camPhi reaches PI + myCam._orbit(0, PI, 0); + // upY should switch from 1(dPhi=0) to -1 (dPhi=PI) + // myCam.upY should be -1 + assert(myCam.upY === -1); + }); + + test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi >= PI', function() { + // the following should produce the upY with inverted direction(from -1 to 1) + // when camPhi reaches PI + myCam._orbit(0, PI, 0); + myCam._orbit(0, -PI, 0); + // upY should switch from -1(dPhi=PI) to 1 (dPhi=-PI) + // myCam.upY should be 1 + assert(myCam.upY === 1); + }); + + test('_orbit() ensures camera can do multiple continuous 360deg rotations', function() { + // the following should produce same values as myCam does a 360deg rotation twice + var myCamCopy = myCam.copy(); + myCam._orbit(0, 4*PI, 0); + // upY should switch from 1 to -1, then to 1 again + assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); + }); + /* test('_orbit() ensures altitude phi <= PI', function() { var myCamCopy = myCam.copy(); @@ -468,6 +515,7 @@ suite('p5.Camera', function() { assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); }); + */ test('_orbit() ensures radius > 0', function() { var myCamCopy = myCam.copy(); From 8c6d0ce148ea35158f27b955996419f28753183f Mon Sep 17 00:00:00 2001 From: AryanKoundal Date: Fri, 10 Feb 2023 22:00:10 +0530 Subject: [PATCH 2/5] resolves issue#5958 3D orbitControl allowing +-90 degrees not enough --- src/webgl/p5.Camera.js | 3 --- test/unit/webgl/p5.Camera.js | 26 -------------------------- 2 files changed, 29 deletions(-) diff --git a/src/webgl/p5.Camera.js b/src/webgl/p5.Camera.js index ffbefb5b4d..a0eb40f211 100644 --- a/src/webgl/p5.Camera.js +++ b/src/webgl/p5.Camera.js @@ -1733,9 +1733,6 @@ p5.Camera.prototype._orbit = function(dTheta, dPhi, dRadius) { if (camRadius < 0) { camRadius = 0.1; } - - // prevent rotation over the zenith / under bottom - // from https://github.com/mrdoob/three.js/blob/dev/src/math/Vector3.js#L628-L632 const _x = Math.sin(camPhi) * camRadius * Math.sin(camTheta); const _y = Math.cos(camPhi) * camRadius; diff --git a/test/unit/webgl/p5.Camera.js b/test/unit/webgl/p5.Camera.js index 7cb6f3379e..49cc9c1d0d 100644 --- a/test/unit/webgl/p5.Camera.js +++ b/test/unit/webgl/p5.Camera.js @@ -449,7 +449,6 @@ suite('p5.Camera', function() { assert.deepEqual(myCam.cameraMatrix.mat4, expectedMatrix); }); - test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi <= 0', function() { // the following should produce the upY with inverted direction(from 1 to -1) // when camPhi changes from positive to negative or zero @@ -458,7 +457,6 @@ suite('p5.Camera', function() { // myCam.upY should be -1 assert(myCam.upY === -1); }); - test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi <= 0', function() { // the following should produce the upY with inverted direction(from -1 to 1) // when camPhi changes from negative to positive or zero @@ -468,7 +466,6 @@ suite('p5.Camera', function() { // myCam.upY should be 1 assert(myCam.upY === 1); }); - test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi >= PI', function() { // the following should produce the upY with inverted direction(from 1 to -1) // when camPhi reaches PI @@ -477,7 +474,6 @@ suite('p5.Camera', function() { // myCam.upY should be -1 assert(myCam.upY === -1); }); - test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi >= PI', function() { // the following should produce the upY with inverted direction(from -1 to 1) // when camPhi reaches PI @@ -487,7 +483,6 @@ suite('p5.Camera', function() { // myCam.upY should be 1 assert(myCam.upY === 1); }); - test('_orbit() ensures camera can do multiple continuous 360deg rotations', function() { // the following should produce same values as myCam does a 360deg rotation twice var myCamCopy = myCam.copy(); @@ -495,27 +490,6 @@ suite('p5.Camera', function() { // upY should switch from 1 to -1, then to 1 again assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); }); - /* - test('_orbit() ensures altitude phi <= PI', function() { - var myCamCopy = myCam.copy(); - - // the following should produce the same values because phi is capped at PI - myCamCopy._orbit(0, 10, 0); - myCam._orbit(0, 20, 0); - - assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); - }); - test('_orbit() ensures altitude phi > 0', function() { - var myCamCopy = myCam.copy(); - - // the following should produce the same values because phi is restricted - // to > 0 - myCamCopy._orbit(0, -10, 0); - myCam._orbit(0, -20, 0); - - assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); - }); - */ test('_orbit() ensures radius > 0', function() { var myCamCopy = myCam.copy(); From 36edf87aeb5fc30504aec93e57e6978472087ddb Mon Sep 17 00:00:00 2001 From: AryanKoundal Date: Fri, 10 Feb 2023 22:23:44 +0530 Subject: [PATCH 3/5] resolves issue#5958 3D orbitControl allowing +-90 degrees not enough --- test/unit/webgl/p5.Camera.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/unit/webgl/p5.Camera.js b/test/unit/webgl/p5.Camera.js index 49cc9c1d0d..8135829b6c 100644 --- a/test/unit/webgl/p5.Camera.js +++ b/test/unit/webgl/p5.Camera.js @@ -452,7 +452,7 @@ suite('p5.Camera', function() { test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi <= 0', function() { // the following should produce the upY with inverted direction(from 1 to -1) // when camPhi changes from positive to negative or zero - myCam._orbit(0, -PI, 0); + myCam._orbit(0, -Math.PI, 0); // upY should switch from 1(dPhi=0) to -1 (dPhi=-PI) // myCam.upY should be -1 assert(myCam.upY === -1); @@ -460,8 +460,8 @@ suite('p5.Camera', function() { test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi <= 0', function() { // the following should produce the upY with inverted direction(from -1 to 1) // when camPhi changes from negative to positive or zero - myCam._orbit(0, -PI, 0); - myCam._orbit(0, PI, 0); + myCam._orbit(0, -Math.PI, 0); + myCam._orbit(0, Math.PI, 0); // upY should switch from -1(dPhi=-PI) to 1 (dPhi=PI) // myCam.upY should be 1 assert(myCam.upY === 1); @@ -469,7 +469,7 @@ suite('p5.Camera', function() { test('_orbit() ensures myCam.upY switches direction (from 1 to -1) at camPhi >= PI', function() { // the following should produce the upY with inverted direction(from 1 to -1) // when camPhi reaches PI - myCam._orbit(0, PI, 0); + myCam._orbit(0, Math.PI, 0); // upY should switch from 1(dPhi=0) to -1 (dPhi=PI) // myCam.upY should be -1 assert(myCam.upY === -1); @@ -477,8 +477,8 @@ suite('p5.Camera', function() { test('_orbit() ensures myCam.upY switches direction (from -1 to 1) at camPhi >= PI', function() { // the following should produce the upY with inverted direction(from -1 to 1) // when camPhi reaches PI - myCam._orbit(0, PI, 0); - myCam._orbit(0, -PI, 0); + myCam._orbit(0, Math.PI, 0); + myCam._orbit(0, -Math.PI, 0); // upY should switch from -1(dPhi=PI) to 1 (dPhi=-PI) // myCam.upY should be 1 assert(myCam.upY === 1); @@ -486,7 +486,7 @@ suite('p5.Camera', function() { test('_orbit() ensures camera can do multiple continuous 360deg rotations', function() { // the following should produce same values as myCam does a 360deg rotation twice var myCamCopy = myCam.copy(); - myCam._orbit(0, 4*PI, 0); + myCam._orbit(0, 4*Math.PI, 0); // upY should switch from 1 to -1, then to 1 again assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); }); From 7c2735d320b56d8025c58f7a56e1053b25487642 Mon Sep 17 00:00:00 2001 From: AryanKoundal Date: Sat, 11 Feb 2023 14:16:31 +0530 Subject: [PATCH 4/5] Modified failing tests --- test/unit/webgl/p5.Camera.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/unit/webgl/p5.Camera.js b/test/unit/webgl/p5.Camera.js index 8135829b6c..655a1bae08 100644 --- a/test/unit/webgl/p5.Camera.js +++ b/test/unit/webgl/p5.Camera.js @@ -484,21 +484,24 @@ suite('p5.Camera', function() { assert(myCam.upY === 1); }); test('_orbit() ensures camera can do multiple continuous 360deg rotations', function() { - // the following should produce same values as myCam does a 360deg rotation twice + // the following should produce two camera objects having same properties. + myCam._orbit(0, Math.PI, 0); var myCamCopy = myCam.copy(); - myCam._orbit(0, 4*Math.PI, 0); - // upY should switch from 1 to -1, then to 1 again - assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); + myCamCopy._orbit(0, Math.PI, 0); + myCamCopy._orbit(0, Math.PI, 0); + for (let i = 0; i < myCamCopy.cameraMatrix.mat4.length; i++) { + expect( + myCamCopy.cameraMatrix.mat4[i]).to.be.closeTo( + myCam.cameraMatrix.mat4[i], 0.001); + } }); test('_orbit() ensures radius > 0', function() { + // the following should produce two camera objects having same properties. + myCam._orbit(0, Math.PI, 0); var myCamCopy = myCam.copy(); - - // the following should produce the same values because radius is - // restricted to > 0 - myCamCopy._orbit(0, 0, -200); - myCam._orbit(0, 0, -300); - - assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4); + myCamCopy._orbit(0, 0, -100); + myCam._orbit(0, 0, -250); + assert.deepEqual(myCam.cameraMatrix.mat4, myCamCopy.cameraMatrix.mat4, 'deep equal is failing'); }); }); From 5d981ae532bf27d0e6bcec49a38087aaf3118133 Mon Sep 17 00:00:00 2001 From: AryanKoundal Date: Mon, 13 Feb 2023 10:51:24 +0530 Subject: [PATCH 5/5] Added inline documentation, modified variable naming and styling. --- src/webgl/p5.Camera.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/webgl/p5.Camera.js b/src/webgl/p5.Camera.js index a0eb40f211..5c07cb4daa 100644 --- a/src/webgl/p5.Camera.js +++ b/src/webgl/p5.Camera.js @@ -1720,12 +1720,14 @@ p5.Camera.prototype._orbit = function(dTheta, dPhi, dRadius) { let camTheta = Math.atan2(diffX, diffZ); // equatorial angle let camPhi = Math.acos(Math.max(-1, Math.min(1, diffY / camRadius))); // polar angle - // add change according to the direction of this.upY - let directionflag = this.upY > 0 ? 1 : -1; - camTheta += directionflag*dTheta; - camPhi += directionflag*dPhi; - if (camPhi <= 0 || camPhi>=Math.PI) { - directionflag *= -1; + let newUpY = this.upY > 0 ? 1 : -1; + // add change according to the direction of newupY + camTheta += newUpY * dTheta; + camPhi += newUpY * dPhi; + // if camPhi becomes >= PI or <= 0, + // upY of camera need to be flipped to the other side + if (camPhi <= 0 || camPhi >= Math.PI) { + newUpY *= -1; } camRadius += dRadius; @@ -1746,7 +1748,7 @@ p5.Camera.prototype._orbit = function(dTheta, dPhi, dRadius) { this.centerY, this.centerZ, 0, - directionflag, + newUpY, 0 ); };