Object3D: Introduce .forward property #14703
Conversation
I've added the code so you can see the approach. The idea is to add Code is mainly based on: https://github.com/juj/MathGeoLib/blob/master/src/Math/float3x3.h#L147-L178 |
|
Interesting! This would also allow people to change the what's the forward direction of the camera 👌 What do you think @WestLangley? |
|
I also like the idea that |
|
|
||
| var localRight = new Vector3(); | ||
| var worldRight = new Vector3(); | ||
| var worldUp = new Vector3( 0, 1, 0 ); |
There was a problem hiding this comment.
This is fixing world up to be +y ?
What impact would this have for using +z as up?
There was a problem hiding this comment.
This is fixing world up to be +y ?
worldUp could be actually configurable. I've hard-wired this to always assumes Y-Up as global up direction of the scene in world space.
There was a problem hiding this comment.
some awkward people (ie me*) work with up = +z, the existing Matrix4.lookAt() has an
exception around for the z up case #11543 already.
- geographic models fit better.
There was a problem hiding this comment.
If we find a way to make it configurable, it should be no problem. Do you have any ideas?
There was a problem hiding this comment.
Damn. I realize right now that this PR breaks all user code where camera.up.set( 0, 0, 1 ) is already set. https://threejs.org/examples/webgl_loader_pcd.html is such a problematic case. You would now have to change worldUp instead. Even if we apply a configuration option, the approach is not a backward-compatible. Too bad, it seems we can't go this way 😞 .
There was a problem hiding this comment.
Do you mean that these use cases will break unless the user also does camera.forward.set( 0, -1, 0 )?
There was a problem hiding this comment.
I don't know why yet but this does not solve the issue. It works if you remove camera.up.set( 0, 0, 1 ) and set worldUp to (0, 0, 1) (e.g. something like THREE.WorldUp.set( 0, 0, 1)). Even then, for some reasons TransformControls seems to have problems with this approach (it internally uses Matrix4.lookAt()).
There was a problem hiding this comment.
After a closer look, the main problem is caused since the new Matrix3.lookAt() interprets Object3D.up differently than Matrix4.lookAt(). That's the reason why both methods do not work together like in the example. From my point of view, Matrix3.lookAt() is the better approach since it's a more clear implementation of what's actually happening in lookAt(): A given Object3D.up vector is mapped like all other local basis vectors (forward, right) to the counterpart of a world coordinate frame (defined by worldUp, targetDirection, and worldRight). This is the way .lookAt() should work and Matrix3.lookAt() is exactly implemented like that.
Even if we introduce THREE.WorldUp to allow a configuration (e.g. to enable Z-UP) and then use this value in Matrix3.lookAt(), we would have to migrate all code that uses Matrix4.lookAt() to Matrix3.lookAt(). Besides, setting camera.up.set( 0, 0, 1 ) is not necessary anymore.
So introducing this new approach is a breaking change. If there will ever be a three.js 2, this should be the way to go^^.
|
48c15a1 is necessary since |
|
Closing, see #14703 (comment) |
I'd like to suggest a new property
Object3D.forwardwhich defines the forward direction in the local space of the object. Using such a property makes it much nicer to implementObject3D.getWorldDirection()since it's not necessary anymore to override the method in classes likeCamera. It's sufficient to change theforwardproperty to e.g.( 0, 0, - 1 )in the ctor.This PR would undo #14677 but I personally would choose a more flexible and clean approach than the current version in
dev. The interesting thing is that the introduction of the localforwardvector could be the basis for an alternative implementation ofObject3D.lookAt()which does not need to know it's type of object anymore. So statements like this.isCamera would be not necessary.