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: mixamoのモーション仕様変更への対応 #1032

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

ke456-png
Copy link
Contributor

mixamoのモーションをNormalizedBone用に変換し、アニメーショントラックに格納する処理を追加

ローカル軸が正規化されていないrigにバインドされたアニメーションでもvrmに適用できるようになったため、
新旧どちらのFBXファイルにおいても同様にアニメーションの再生ができる。

#889 (comment)

mixamoのモーションをNormalizedBone用に変換し、アニメーショントラックに格納する処理を追加

ローカル軸が正規化されていないrigにバインドされたアニメーションでもvrmに適用できるようになったため、
新旧どちらのFBXファイルにおいても同様にアニメーションの再生ができる。

#889 (comment)
@ke456-png ke456-png requested a review from 0b5vr September 1, 2022 05:29
@ke456-png ke456-png self-assigned this Sep 1, 2022
@ke456-png ke456-png marked this pull request as ready for review September 1, 2022 06:06
Comment on lines 48 to 55
targetRotation
.fromArray( flatQuaternion )
.premultiply( parentRestWorldRotation );

retargetRotation
.copy( targetRotation )
.multiply( restRotationInverse )
.toArray( flatQuaternion );
Copy link
Contributor

Choose a reason for hiding this comment

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

結局やりたいのは、

親のレスト時ワールド回転 * トラックの回転 * レスト時ワールド回転の逆

ですかね?

Copy link
Contributor

Choose a reason for hiding this comment

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

一時変数 _quatA がお嫌いでなければ、僕だったらこう書きます:

_quatA
    .fromArray( flatQuaternion )
    .premultiply( parentRestWorldRotation )
    .multiply( restRotationInverse );

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と .multiplyを連結すると混乱するので避けていました。
冗長なコピーで変数名を変えているのもその流れでした。

たしかに、コメントで式を書いてしまって処理部分をコンパクトにしてしまうのも良いですね
// 親のレスト時ワールド回転 * トラックの回転 * レスト時ワールド回転の逆

Copy link
Contributor

Choose a reason for hiding this comment

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

premultiplyとmultiply連結するとだいぶキモいのはそれはそうですね。
ただ、自分は変数が多いほうが混乱しました。
が、これはあくまでもThree.jsのコーディングに慣れたエンジニアの意見なので、ただの1サンプルです。

Comment on lines 49 to 54
.fromArray( flatQuaternion )
.premultiply( parentRestWorldRotation );
_quatA.fromArray( flatQuaternion );

retargetRotation
.copy( targetRotation )
.multiply( restRotationInverse )
.toArray( flatQuaternion );
// 親のレスト時ワールド回転 * トラックの回転 * レスト時ワールド回転の逆
_quatA
.premultiply( parentRestWorldRotation )
.multiply( restRotationInverse );

flatQuaternion.map( ( v, index ) => track.values[ index + i ] = v );
_quatA.toArray( flatQuaternion );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

回転部分の処理を連結しました

Array変換の部分は分けました

Comment on lines +56 to +60
flatQuaternion.forEach( ( v, index ) => {

track.values[ index + i ] = v;

} );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

MapからforEachに変更しました

Copy link
Contributor

@0b5vr 0b5vr left a comment

Choose a reason for hiding this comment

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

good

@ke456-png ke456-png merged commit 9bcb37d into 1.0 Sep 2, 2022
@ke456-png ke456-png deleted the fix/example-mixamo-animation branch September 2, 2022 02:47
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.

2 participants