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

[1.0] BREAKING Update LookAt #964

Merged
merged 16 commits into from
May 11, 2022
Merged

[1.0] BREAKING Update LookAt #964

merged 16 commits into from
May 11, 2022

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Apr 20, 2022

Description

There are many changes in this PR.
I have added descriptions for each commit; you might want to see each in order.

The primary purpose of this PR is to make VRMLookAt able to use manually set yaw and pitch.

  • 💡 LookAt APIs now mainly use yaw-pitch angles instead of Euler.
  • ✨ Add an ability to set yaw and pitch manually.
    vrm.lookAt.yaw = 30.0 * Math.sin(time)
    vrm.lookAt.pitch = 30.0 * Math.cos(time)
  • 🐛 VRM0.0 and VRM1.0 now look in the same direction when we manually set yaw and pitch (formerly euler).
  • 🐛 VRMLookAtBoneApplier is now compatible with eye bones witch have non-uniform rest rotations.
  • 🐛 VRMLookAtHelper now works properly on VRM0.0.
  • 💡 VRMLookAtHelper now hides target gizmo when VRMLookAt.autoUpdate is false.

Breaking Changes

  • 🚨 VRMLookAt.euler is deprecated. Use yaw and pitch, or getEuler() instead.
  • 🚨 Protected method VRMLookAt._calcEuler() is removed. Use VRMLookAt.lookAt() instead.
  • 🚨 The set value to VRMLookAt is now not applied to its applier immediately.
    • VRMLookAt has its internal flag _needsUpdate, and it's applied when we call its update()
  • 🚨 VRMLookAtApplier.lookAt() is deprecated. Use applyYawPitch() instead.
  • 🚨 VRMLookAtBoneApplier now requires faceFront in order to use with VRM0.0 models.
    • It should be handled automatically if you use VRMLookAtLoaderPlugin

Points need review

  • Does the logic look good?
  • Any breaking changes I forgot to mention?
  • Does the breaking changes worth enough?

0b5vr added 10 commits April 20, 2022 10:47
Rewrite angle math using RAD2DEG and DEG2RAD
… euler

- Add new accessors `yaw` and `pitch` to `VRMLookAt`
- Arguments of `VRMLookAtApplier.lookAt` has been changed
- `VRMLookAt.euler`
- `VRMLookAt._calcEuler` has been renamed to `VRMLookAt._updateYawPitchByPosition` considering its role
- `VRMLookAt` now uses its protected member `_needsUpdate` to check whether it should apply its yaw and pitch to its applier or not
    - Be aware that yaw and pitch won't be applied until you call `update` !
- `apply` receives `yaw` and `pitch`
- `lookAt` is preserved and receives `euler` but it's deprecated
… `VRMLookAt`

- `_updateYawPitchByPosition` is removed and integrated to `lookAt`
Follow latest lookAt API
- yaw, pitch was not compatible between VRM0.0 and VRM1.0, I have to add `faceFront` to `VRMLookAtBoneApplier`
- VRMLookAtHelper also needs `faceFront`, I added a function `getFaceFrontQuaternion` to `VRMLookAt`
- `VRMLookAtBoneApplier` was not compatible with eye bones which have non-uniform rest rotation, I fixed this by considering the rest rotation of eye bones
- I wanted to test new utility functions, setup jest for `three-vrm-core`
- `VRMLookAt.getEuler()` should be used instead
@0b5vr 0b5vr added bug Something isn't working Breaking Change Choose wisely VRM 1.0 labels Apr 20, 2022
@0b5vr 0b5vr added this to the VRM 1.0 milestone Apr 20, 2022
@0b5vr 0b5vr self-assigned this Apr 20, 2022
@0b5vr 0b5vr requested a review from nyamadan April 21, 2022 03:35
* @param vector The vector
* @returns A tuple contains two angles, `[ azimuth, altitude ]`
*/
export function calcAzimuthAltitude(vector: THREE.Vector3): [ azimuth: number, altitude: number ] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 133 to 137
leftEye.quaternion
.copy(_quatB)
.premultiply(_quatA)
.premultiply(_quatB.invert())
.multiply(this._restQuatLeftEye);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

quatB^-1 * quatA * quatB * restQuatLeftEye

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なんでpremultiplyしたかっていうと、 _quatB.invert() を2回叩きたくなかったから……

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
leftEye.quaternion
.copy(_quatB)
.premultiply(_quatA)
.premultiply(_quatB.invert())
.multiply(this._restQuatLeftEye);
// quatB^-1 * quatA * quatB * restQuatLeftEye
leftEye.quaternion
.copy(_quatB)
.premultiply(_quatA)
.premultiply(_quatB.invert())
.multiply(this._restQuatLeftEye);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

c94396b で対応しました

@0b5vr
Copy link
Contributor Author

0b5vr commented Apr 25, 2022

99d96fc : deprecatedにした .euler を一箇所使っていたので、これを getEuler() に変えました。

@0b5vr 0b5vr merged commit b5173f1 into 1.0 May 11, 2022
@0b5vr 0b5vr deleted the update-lookat branch May 11, 2022 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Choose wisely bug Something isn't working VRM 1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant