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

Fix: fix lookAt, rightEye did have wrong orientation #1096

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Nov 4, 2022

Description

rightEye did have wrong orientation, fixed this.

It turns out that it's an easy mistake, See the diff of VRMLookAtBoneApplier.

Also added a test addressing this case.

turns out that it's an easy mistake, See the diff of VRMLookAtBoneApplier

Also added a test addressing this case
@0b5vr 0b5vr added bug Something isn't working easy labels Nov 4, 2022
@0b5vr 0b5vr added this to the next milestone Nov 4, 2022
@0b5vr 0b5vr requested a review from ke456-png November 4, 2022 02:52
@0b5vr 0b5vr self-assigned this Nov 4, 2022
const rightEye = this.humanoid.getRawBoneNode('leftEye');
const rightEye = this.humanoid.getRawBoneNode('rightEye');
Copy link
Contributor

Choose a reason for hiding this comment

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

[MEMO]

右目と左目でレストの姿勢が違う場合に問題が発生していた。

ありそうなケースで言うと"より目"にしている場合などが考えられそうです。

Comment on lines +147 to +176

describe('when leftEye and rightEye have different rest orientations', () => {
beforeEach(() => {
scene = new THREE.Scene();

humanoid = createHumanoid();
scene.add(humanoid.getRawBoneNode('hips')!);
humanoid.getRawBoneNode('rightEye')!.quaternion.copy(QUAT_Y_CCW90);

applier = createBoneApplier(humanoid);
});

it('makes each eye look toward the specified angle', () => {
applier.applyYawPitch(15.0, 15.0);

const rawLeftEye = humanoid.getRawBoneNode('leftEye')!;
const rawRightEye = humanoid.getRawBoneNode('rightEye')!;

// saturate( angle / rangeMap.inputMaxValue ) * rangeMap.outputScale = saturate( 15 / 30 ) * 5 = 2.5
const expectedLeft = new THREE.Quaternion()
.setFromEuler(new THREE.Euler(DEG2RAD * 2.5, DEG2RAD * 2.5, 0.0, 'YXZ'))
.multiply(QUAT_Y_CW90);
const expectedRight = new THREE.Quaternion()
.setFromEuler(new THREE.Euler(DEG2RAD * 2.5, DEG2RAD * 2.5, 0.0, 'YXZ'))
.multiply(QUAT_Y_CCW90);

expect(rawLeftEye.quaternion).toBeCloseToQuaternion(expectedLeft);
expect(rawRightEye.quaternion).toBeCloseToQuaternion(expectedRight);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

NICE! 👍

@0b5vr
Copy link
Contributor Author

0b5vr commented Nov 4, 2022

Thank you for the review. Will merge and publish next monday.

@0b5vr 0b5vr merged commit 1d39ca0 into dev Nov 7, 2022
@0b5vr 0b5vr deleted the fix-lookat branch November 7, 2022 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working easy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants