mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.1k stars 35.34k forks source link

Better 3D object manipulation and even better rollercoaster sample #25333

Closed philipswan closed 8 months ago

philipswan commented 1 year ago

Description

The camera and roller coaster in the current rollercoaster sample does not bank in curves, and thus the sample does not fully demonstrate best practices for orienting an object to follow a curve.

Solution

I suggest: a) Implement GetTangent[At], GetNormal[At], and GetBinormal[At] in a way that (when possible) obtains the values from the curve analytically for maximum accuracy (as opposed to using points a short distance further along the curve). b) Implement a GetFrenetFrame[At] if this will compute and return all three vectors more efficiently. c) Implement the rollercoaster sample so that the coaster banks as it travels around the curves. Ideally this should do the math to figure out the direction of the local acceleration (that is, gravity plus centripetal), and orient the camera and coaster so that left is aligned with (local acceleration \cross forward vector). d) If it doesn't already exist, then adding an intuitive function (like "lookAt") that takes in two vectors such as "forwardDirection" and "upDirection" would be helpful. An object that uses the method should end up being fully oriented in the desired direction. For example, setOrientation(tangetToCurve, -normalToCurve) would be perhaps be appropriate for orienting the coaster in a valley.

Alternatives

I've looked at extrude geometry and observed its use of Frenet frames. But, this code does too much brute math on the various vectors. A more intuitive-to-use method that will automatically orienting things the way you want will be very helpful. It's true that if you are a wizard with Quaternions, you can usually figure out how to get the job done. But 3D manipulation by using quaternions is not intuitive enough, and thus this approach increases development and debug time.

Additional context

Excellent background information... https://www.youtube.com/watch?v=jvPPXbo87ds&ab_channel=FreyaHolm%C3%A9r No response

makc commented 1 year ago

does not bank in curves

that's because they use lookAt and the camera has up vector set to 0,1,0 at all times. you could easily change that by tracking 3D acceleration and adding a portion of its xz to that 0,1,0

edit: actually nvm, that would result in the angle different from the track tilt... so yea,

the sample does not fully demonstrate best practices for orienting an object to follow a curve

mrdoob commented 1 year ago

@philipswan Do you mind doing a PR with your suggestion?

philipswan commented 1 year ago

Do you mean implement a solution and submit a PR?

I'm open to the idea. I would need to be able to work with someone who would mentor me through the process and reassure me along the way that I am coding up something that will successfully make it through code review and be merged.

Phil

mrdoob commented 1 year ago

Do you mean implement a solution and submit a PR?

Yes

philipswan commented 1 year ago

I would, but I'm worried about the risk of having my time wasted in the end, since I don't really know what it would take to satisfy the reviewers and have the PR approved. Also, I'm not actually confident that I do know the best way to do this - I would need to be able to consult with an expert occasionally along the way.

Would that be acceptable?

Phil

mrdoob commented 1 year ago

First thing would be to stop responding by email.

Look what mess that feature creates: https://github.com/mrdoob/three.js/issues/25333

philipswan commented 1 year ago

Yikes! Ok, my first thoughts on this are that we need to define a new set of curves where the normals, binormals, and tangents can be computed quickly and 100% accurately by using straight math, not interpolation techniques. Second thought is that there's a need for a function such as... const orientation = curve.getQuaternionAt(modelForward, modelUpward, curvePosition) which can then be applied to the model with... model.setRotationFromQuaternion(this.orientation) This will point the model in the same direction as the curve at curvePosition and twist it right amount as well. Then we'd have to make some sample code to show examples of how one could twist the model more or less, based on the effect of gravity or something.

mrdoob commented 1 year ago

Go for it 👍

philipswan commented 1 year ago

Are there any guideline documents or videos that lay out the PR process that three.js uses? When I did PR's at my last company (SpaceX) it was a horribly complicated process with many pitfalls.

makc commented 1 year ago

@philipswan apart from this I have not seen any guidelines. I had like 20 PRs here I think, half of them made it, the other half did not. I'd say, you just submit and they either accept or tell you what's wrong with it, then you can either fix and re-submit or drop it.

LeviPesin commented 1 year ago

There also is https://github.com/mrdoob/three.js/blob/dev/.github/CONTRIBUTING.md.

philipswan commented 1 year ago

Quick update - I have been making progress...

image
makc commented 1 year ago

@philipswan that looks familiar

vis-prime commented 1 year ago

Does this example's camera motion do what you mean ?

tick the 'animationVIew' boolean

https://threejs.org/examples/#webgl_geometry_extrude_splines

philipswan commented 1 year ago

This sample does bank in curves, so nice find. It does not use "best practices" in my opinion. The banking relies on getting values indirectly from the tube geometry in a rather hacky way, as opposed to getting the values directly from the curve in an accurate way.

However, the lines ...

splineCamera.matrix.lookAt( splineCamera.position, lookAt, normal );
splineCamera.quaternion.setFromRotationMatrix( splineCamera.matrix );

May be a good implementation. I'll take a look to see if these operations are better than what I came up with.

Mugen87 commented 8 months ago

@philipswan If you are still interested this topic, just file a PR with your suggestions. A separate issue is not required.

philipswan commented 8 months ago

I'm still interested, and I have coded something up and working, but there's still some more work to do to get it to the quality level needed for a PR.