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.

Sign up for GitHub

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

Jump to bottom

VRM1 は、XYZ_RightUpForward_RH にする #727

Merged
merged 19 commits into from
Feb 19, 2021
Merged

VRM1 は、XYZ_RightUpForward_RH にする #727

merged 19 commits into from
Feb 19, 2021

Conversation

ousttrue
Copy link
Contributor

@ousttrue ousttrue commented Feb 5, 2021

vrm-c/vrm-specification#205

vrm0 は 座標変換で x, y, -z してたが、
vrm1 は 座標変換で -x, y, z する。

@ousttrue ousttrue added the 1.0 label Feb 5, 2021
@ousttrue ousttrue added this to the v0.67 milestone Feb 9, 2021
@ousttrue ousttrue requested a review from hiroj February 9, 2021 05:45
@ousttrue ousttrue modified the milestones: v0.67, v0.68 Feb 10, 2021
@ousttrue
Copy link
Contributor Author

#728 なおした。

byteOffset = -1

@ousttrue ousttrue requested a review from Santarh February 16, 2021 08:14
Santarh
Santarh requested changes Feb 16, 2021
View reviewed changes
Copy link
Contributor

@Santarh Santarh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

migration に bindpose の変換がありません。
0.x と 1.x で bindpose の値が一緒になってしまっているのは、どこかがバグっていると思われます。
正規化された VRM でも bindpose は移動行列相当になっているので、モデルが回転している以上は変わるはずです。

@ousttrue ousttrue force-pushed the feature10/z_plus_forward branch from c31f73a to 9086021 Compare February 16, 2021 09:22
@ousttrue
Copy link
Contributor Author

  • vrm-1.0 importer/exporter の座標変換バグ(Matrix が Z Reverse)
  • Migration 周りの Matrix4x4 のバイトオーダーバグ(m01 が 16byte目になるのが正解)

を修正しました。

Santarh
Santarh requested changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@Santarh Santarh left a comment

Choose a reason for hiding this comment

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

おつかれさまです。ヤバげな要素直接操作部分はコメントしたほうが良さそう

Assets/VRM10/vrmlib/Runtime/NumericsExtensions.cs
@@ -148,7 +148,7 @@ public static Matrix4x4 ReverseX(this Matrix4x4 m)
if (m.IsOnlyTranslation())
{
var ret = m;
ret.M43 = -ret.M43;
ret.M41 = -ret.M41;
Copy link
Contributor

Choose a reason for hiding this comment

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

コメントを書いたほうが良さそう

Assets/VRM10/Runtime/UnityExtensions.cs
@@ -106,9 +106,9 @@ public static Matrix4x4 Matrix4x4FromRotation(Quaternion q)

public static Matrix4x4 RotateY180(this Matrix4x4 m)
{
var t = m.GetColumn(3);
var t = m.GetRow(3);
Copy link
Contributor

Choose a reason for hiding this comment

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

ここもできればなんらか書いておいたほうが良さそう

Assets/VRM10/vrmlib/Runtime/SpanLike.cs
BitConverter.ToSingle(array, start + 52),
BitConverter.ToSingle(array, start + 56),
BitConverter.ToSingle(array, start + 60))
BitConverter.ToSingle(array, start + 16),
Copy link
Contributor

Choose a reason for hiding this comment

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

ここも…ｗ

@ousttrue
Copy link
Contributor Author

UnityEngine.Matrix4x4

行ベクトル

Mxx, GetRow, GetCol の概念
R, R, R, 0
R, R, R, 0
R, R, R, 0
T, T, T, 1

メモリレイアウト

列優先メモリレイアウト(col0, col1, col2, col3)

これは GLTF のメモリレイアウトと同じです。

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#data-alignment

@ousttrue ousttrue requested a review from Santarh February 19, 2021 11:22
Santarh
Santarh approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@Santarh Santarh left a comment

Choose a reason for hiding this comment

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

よさそう

@ousttrue ousttrue merged commit 59129af into vrm-c:master Feb 19, 2021
@ousttrue ousttrue deleted the feature10/z_plus_forward branch April 2, 2021 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Santarh Santarh Santarh approved these changes

@hiroj hiroj Awaiting requested review from hiroj

Assignees
No one assigned
Labels
1.0
Projects
None yet
Milestone
v0.68
Development

Successfully merging this pull request may close these issues.
2 participants
@ousttrue @Santarh