Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update EditorControls #687

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vincentfretin
Copy link
Contributor

I'm opening this PR in draft while I'm looking into it.

The EditorControls.js file in this repo was added the day after this commit in three.js repo
mrdoob/three.js@12b8981 (that was included in three r100)
Initial commit was aae61a3
comparing those two versions gave those differences:

--- old_EditorControls_from_threerepo.js	2023-04-17 17:16:52.851987686 +0200
+++ old_EditorControls.js	2023-04-17 17:14:54.659227028 +0200
@@ -1,3 +1,70 @@
+import debounce from 'lodash.debounce';
+
+THREE.Box3.prototype.expandByObject = (function () {
+  // Computes the world-axis-aligned bounding box of an object (including its children),
+  // accounting for both the object's, and children's, world transforms
+
+  var scope, i, l;
+
+  var v1 = new THREE.Vector3();
+
+  function traverse( node ) {
+
+    var geometry = node.geometry;
+
+    if ( geometry !== undefined ) {
+
+      if ( geometry.isGeometry ) {
+
+        var vertices = geometry.vertices;
+
+        for ( i = 0, l = vertices.length; i < l; i ++ ) {
+
+          v1.copy( vertices[ i ] );
+          v1.applyMatrix4( node.matrixWorld );
+
+          if (isNaN(v1.x) || isNaN(v1.y) || isNaN(v1.z)) { continue; }
+          scope.expandByPoint( v1 );
+
+        }
+
+      } else if ( geometry.isBufferGeometry ) {
+
+        var attribute = geometry.attributes.position;
+
+        if ( attribute !== undefined ) {
+
+          for ( i = 0, l = attribute.count; i < l; i ++ ) {
+
+            v1.fromBufferAttribute( attribute, i ).applyMatrix4( node.matrixWorld );
+
+            if (isNaN(v1.x) || isNaN(v1.y) || isNaN(v1.z)) { continue; }
+            scope.expandByPoint( v1 );
+
+          }
+
+        }
+
+      }
+
+    }
+
+  }
+
+  return function expandByObject( object ) {
+
+    scope = this;
+
+    object.updateMatrixWorld( true );
+
+    object.traverse( traverse );
+
+    return this;
+
+  };
+
+})();
+
 /**
  * @author qiao / https://github.com/qiao
  * @author mrdoob / http://mrdoob.com
@@ -38,13 +105,17 @@
 
 	var changeEvent = { type: 'change' };
 
+  this.dispatchChange = debounce(() => {
+    scope.dispatchEvent(changeEvent);
+  }, 100);
+
 	this.focus = function ( target ) {
 
 		var distance;
 
 		box.setFromObject( target );
 
-		if ( box.isEmpty() === false ) {
+		if ( box.isEmpty() === false && !isNaN(box.min.x) ) {
 
 			box.getCenter( center );
 			distance = box.getBoundingSphere( sphere ).radius;
@@ -60,7 +131,7 @@
 
 		delta.set( 0, 0, 1 );
 		delta.applyQuaternion( object.quaternion );
-		delta.multiplyScalar( distance * 4 );
+		delta.multiplyScalar( distance * 2 );
 
 		object.position.copy( center ).add( delta );
 
@@ -78,7 +149,7 @@
 		object.position.add( delta );
 		center.add( delta );
 
-		scope.dispatchEvent( changeEvent );
+		scope.dispatchChange();
 
 	};
 
@@ -94,7 +165,7 @@
 
 		object.position.add( delta );
 
-		scope.dispatchEvent( changeEvent );
+		scope.dispatchChange();
 
 	};
 
@@ -115,7 +186,7 @@
 
 		object.lookAt( center );
 
-		scope.dispatchEvent( changeEvent );
+		scope.dispatchChange();
 
 	};

further changes to it was around focus and ortho camera:
f658d15
26eb77f#diff-d05c541b1811ac8b854c10217ee45ec94afc1f78c715842c7463c2fb9b05efa7
ee70fa1
b371202

Latest version in the three.js repo:
is at https://github.com/mrdoob/three.js/commits/dev/editor/js/EditorControls.js

Some interesting updates around better touch support may be interesting to backport here:
mrdoob/three.js@1eccf74
mrdoob/three.js@e55b718

@kfarr
Copy link
Contributor

kfarr commented May 23, 2023

Thanks for digging through this @vincentfretin. I might be wrong but the only "new" feature I see in green above might be a bounding box around an entity and its children? I think currently selecting a parent entity with children in inspector will not result in a bounding box displayed unless it has a mesh attached to that exact node.

The touch updates appear to be using pointer instead of mouse events to support a wider range of devices but TBH I've tested this on ipad, touch screen displays, etc. and never had a problem before. There seems to be placeholder for custom touch behavior such as "// TODO touch" but nothing substantive.

@vincentfretin
Copy link
Contributor Author

vincentfretin commented May 23, 2023

The part in green with expandByObject patch seems to be an old version of what is currently in threejs Box3 plus with some isNaN checks. I'm no sure in which case x y z on a vertex could be NaN here. See #686 for this specific part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants