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

avatar 代入時に姿勢が変わる対策 #2208

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

ousttrue
Copy link
Contributor

@ousttrue ousttrue commented Jan 4, 2024

fixed #2201

animator.avatar = newAvatar

としたときに元の値を維持する?挙動があります。
そのため 旧avatar と newAvatar で元になる姿勢が異なる(例えば正規化)場合に姿勢が壊れます。
対策として、 Destory(Animator) して作り直します。

v0.116 より前はヒエラルキーをコピーしていたため代入時の元の値を維持する挙動をたまたま回避できていたようです。
v0.116 はコピーせずに同一のヒエラルキーで実行します。

@ousttrue ousttrue requested a review from Santarh January 4, 2024 07:19
@ousttrue
Copy link
Contributor Author

ousttrue commented Jan 4, 2024

#2207 merge 後に rebase します 🙏

@Santarh
Copy link
Contributor

Santarh commented Jan 5, 2024

むしろここが気になったんですが、現状の実装では Export 対象の GameObject に破壊的変更を加えるようにしたということですか?

v0.116 はコピーせずに同一のヒエラルキーで実行します。

出力操作をするとモデルが変更されるというのは、ユーザから見ると予想しづらい(困った)挙動だと思えます。
すくなくとも Editor での Export の場合、素直にコピーした方が実装も挙動もシンプルになりそうですがどうなんでしょう?

@Santarh
Copy link
Contributor

Santarh commented Jan 5, 2024

さすがにそれはなくて、元の GameObject 自体は不変で、コピーしたものを操作して出力しているか。
なので v0.116 も ヒエラルキーをコピーして はいて、シーンが破壊されるようなことはないですよ、ね?

target = GameObject.Instantiate(target);

@Santarh
Copy link
Contributor

Santarh commented Jan 5, 2024

なるほど。ここでいう ヒエラルキーをコピーして いたのは、以前の BoneNormalizer の中のこの実装のことですか。
理解しました。

https://github.com/vrm-c/UniVRM/pull/2179/files#diff-1fdf808babd75944390d7cde67671e618c54d4e4c49358e9d3ec81f2b24508b4L514

https://github.com/ousttrue/UniVRM/blob/f8ac8075e7efffad062def03eeb54f680cadc57c/Assets/UniGLTF/Runtime/MeshUtility/BoneNormalizer.cs#L14

以前は2回も GameObject を Duplicate していたんですねえ

@ousttrue ousttrue force-pushed the fix/avatar_assign_vrm0_normalize branch from cf21e15 to 30f893f Compare January 5, 2024 06:08
@ousttrue
Copy link
Contributor Author

ousttrue commented Jan 5, 2024

2回も GameObject を Duplicate

うろ覚えですが、
prefab で動くようにしたあたりで、2つめのコピーが増えたのかもしれない。
editor なので安全に倒してあるのだと思います。

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.

LGTM、コメント追従だけ

@@ -45,7 +45,7 @@ public static void EnforceTPose(GameObject go)
/// <param name="go">対象モデルのルート</param>
/// <param name="forceTPose">強制的にT-Pose化するか</param>
/// <returns>正規化済みのモデル</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

コメント更新が追い付いてないですね。
ついでに破壊的操作であることも書いておくといいのかも

@ousttrue ousttrue requested a review from Santarh January 9, 2024 04:41
@ousttrue ousttrue merged commit 125e42c into vrm-c:master Jan 9, 2024
1 check was pending
@Santarh Santarh mentioned this pull request Jan 9, 2024
@FujiSunflower
Copy link
Contributor

報告が2つあります。

  1. Destory(Animator)とDestroyImmediate(Animator)の分岐の箇所がありましたが、Destory(Animator)の場合ランタイムで次フレームに移らないと削除されないので、DestroyImmediate(Animator)の方が良さそうでした。
  2. 表情の正規化が上手く行っていないような気がしました。これに関しては自分がややこしい手順をしてる可能性もあります。

@ousttrue ousttrue deleted the fix/avatar_assign_vrm0_normalize branch January 24, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A normalization bug on Unity2022.
3 participants