Skip to content

Conversation

@Korijn
Copy link
Contributor

@Korijn Korijn commented Feb 17, 2023

@ivoflipse ivoflipse changed the title Update conventions & reset Abject API Update conventions & reset Object API Feb 17, 2023
Copy link
Contributor

@FirefoxMetzger FirefoxMetzger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions on how we could improve these conventions

@Korijn
Copy link
Contributor Author

Korijn commented Feb 20, 2023

So, here's where I think my personal confusion stems from. By default, these are the directions in three.js and also in pygfx:

image

  • +X is right
  • +Y is up
  • -Z is forward

However, when you have a random object, and you use it's lookAt function to make it face a target, it's rotation will be adjusted such that it's local positive Z axis will point to target. See the screenshot below. I've translated the cylinder a little, and then made the cylinder "look at" the origin using lookAt(0,0,0).

three-lookAt

@FirefoxMetzger
Copy link
Contributor

@Korijn Could this simply be a bug/inconsistency in ThreeJS?

@almarklein
Copy link
Member

almarklein commented Feb 20, 2023

I played with the editor too. Indeed, calling lookAt at an object turns it's local +Z axis toward the target. Doing the same with a camera object turns it's -Z axis towards the target.

--> Looks like the implementation of lookAt is different for camera's, which is an inconsistency. I wonder whether this has a historical thing that we can avoid by doing it right from the start, or whether there is another reason ...

@Korijn
Copy link
Contributor Author

Korijn commented Feb 20, 2023

@Korijn Could this simply be a bug/inconsistency in ThreeJS?

No, I think it's intentional! There are similar differences when I try the Unity editor locally.

@Korijn
Copy link
Contributor Author

Korijn commented Feb 20, 2023

--> Looks like the implementation of lookAt is different for camera's, which is an inconsistency. I wonder whether this has a historical thing that we can avoid by doing it right from the start, or whether there is another reason ...

Same for lights: https://github.com/mrdoob/three.js/blob/acb58df60448e75561c363f142f0df878705443d/src/core/Object3D.js#L284

@Korijn
Copy link
Contributor Author

Korijn commented Feb 21, 2023

I've updated the conventions document with your suggestions and included the Z-axis distinction for cameras and lights and other objects.

@Korijn
Copy link
Contributor Author

Korijn commented Feb 21, 2023

I think I've found the reasoning for three.js exception for cameras and lights, it is based on the glTF conventions.

In the above links you can find the glTF conventions, including the exceptions to the Z-axis for cameras and lights.

So I think perhaps one of three.js goals is to be compatible with glTF assets out of the box. I wonder if that means their positive X axis points left instead of right in accordance with the glTF convention... looks like it does:

image

So in conclusion, three.js is by default completely aligned with the glTF spec. Some relevant quotes from the spec:

  • glTF uses a right-handed coordinate system. glTF defines +Y as up, +Z as forward, and -X as right; the front of a glTF asset faces +Z.
  • The camera is defined such that the local +X axis is to the right, the “lens” looks towards the local -Z axis, and the top of the camera is aligned with the local +Y axis.
    The view matrix is derived from the global transform of the node containing the camera with the scaling ignored. If the node’s global transform is identity, the location of the camera is at the origin.
  • For light types that have a direction (directional and spot lights), the light's direction is defined as the 3-vector (0.0, 0.0, -1.0) and the rotation of the node orients the light accordingly. That is, an untransformed light points down the -Z axis. The light's transform is affected by the node's world scale, but all properties of the light (such as range and intensity) are unaffected.

It might be very convenient for us to adopt the glTF specification as well. We'll be compatible by default and it is something we can refer to easily for the details.

Co-authored-by: Almar Klein <[email protected]>
@almarklein
Copy link
Member

I'm still don't fully understand why the view direction of cameras and lights is the backward direction for normal objects. But I think we can assume that the ppl from glTF have thought about this well. So let's adopt this and also mention glTF instead of pygfx (and link to it too).

glTF uses a right-handed coordinate system. glTF defines +Y as up, +Z as forward, and -X as right; the front of a glTF asset faces +Z.

This confused me, until I realized that if you rotate around these axiii, so that you view down the -Z axis, then +X is to the right again.

@Korijn
Copy link
Contributor Author

Korijn commented Feb 21, 2023

It also explains why the lookAt call in threejs can just flip the eye and target arguments for cameras and lights.

@Korijn
Copy link
Contributor Author

Korijn commented Feb 21, 2023

OK, I'm going to adjust the text to align with glTF spec (and call it out).

@Korijn
Copy link
Contributor Author

Korijn commented Feb 23, 2023

@FirefoxMetzger would you mind taking over this PR? I think you are better at documenting the coordinate frames and conventions than I am, and also I fractured a rib yesterday so I won't be using my laptop a whole lot anytime soon (typing this on my phone).

@FirefoxMetzger
Copy link
Contributor

@Korijn Sure. I'll do a review and finish it up during the next sprint 👍

I fractured a rib yesterday

Ouch. Hope that things will heal back together smoothly.

@Korijn Korijn changed the title Update conventions & reset Object API Update conventions Mar 1, 2023
@Korijn
Copy link
Contributor Author

Korijn commented Mar 1, 2023

I've split up this PR so we can merge #47 more quickly, and so that @FirefoxMetzger can take over this PR which is now isolated to just the changes to CONVENTIONS.md.

@FirefoxMetzger
Copy link
Contributor

I've updated the conventions and added a section about the various coordinate systems that we use in pylinalg and how we define them.

We have to decide on the convention to use for spherical coordinates. I left that definition a bit vague on purpose until we decide on its details. Otherwise, this should be ready to review (@almarklein ).

almarklein
almarklein previously approved these changes Mar 6, 2023
Copy link
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good from my end! One minor comment.

@FirefoxMetzger
Copy link
Contributor

If you both agree with the updates to the definition of spherical coordinates then I think we can merge. Looks good from my side otherwise :)

@Korijn
Copy link
Contributor Author

Korijn commented Mar 8, 2023

I'm good! Since I opened the PR, it requires you or Almar's approval.

@Korijn Korijn merged commit 99a60ba into main Mar 8, 2023
@Korijn Korijn deleted the reboot branch March 8, 2023 14:19
@Korijn
Copy link
Contributor Author

Korijn commented Mar 8, 2023

Nice work! This exercise will prove very much worthwhile.

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.

Document coordinate frame axes conventions Update conventions

4 participants