Optimize MorphTargetBindingMerger with minimum Dictionary usage
#2135
+211
−132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Santarh
commented
Aug 17, 2023
|
|
||
| public void AccumulateValue(ExpressionKey key, float value) | ||
| { | ||
| if (!_keyIndexReverseMap.TryGetValue(key, out var idx)) return; |
There was a problem hiding this comment.
毎フレーム処理での、唯一の Dictionary 呼び出し場所。
１モデルあたり、そのモデルが持つ ExpressionClip の数（デフォルト１８）だけ呼ばれる。
Santarh
commented
Aug 17, 2023
| @@ -156,7 +156,7 @@ public bool Equals(ExpressionKey other) | |||
|
|
|||
| if (Preset != other.Preset) return false; | |||
| if (Preset != ExpressionPreset.custom) return true; | |||
| return Name == other.Name; | |||
| return Name.Equals(other.Name, StringComparison.Ordinal); | |||
There was a problem hiding this comment.
string の比較は、デフォルトだと Culture を考慮しており、低速かつ他言語で予期せぬ動作になる恐れがある。
この Name はカルチャを考慮する必要はない。
Santarh
commented
Aug 17, 2023
Comment on lines
+25
to
+26
| public int TargetRendererInstanceId { get; } | ||
| public int TargetBlendShapeIndex { get; } |
There was a problem hiding this comment.
レンダラの InstanceID と BlendShape Index の組み合わせで、操作対象箇所を Identity する。
レンダラは UnityEngine.Object の == null になる可能性があるが、この struct の同値判定においては考慮しない。
notargs
approved these changes
Aug 17, 2023
| _expressions = expressionAvatar.Clips.ToDictionary( | ||
| x => expressionAvatar.CreateKey(x.Clip), | ||
| x => x.Clip, | ||
| ExpressionKey.Comparer |
There was a problem hiding this comment.
ExpressionKey.Comparerを渡す
keysは改行しただけ
| /// Dictionary を使用すると、その GetEnumerator()(foreach) や get/set を 100,1000 オーダーで呼び出すことになる。 | ||
| /// するとモバイル環境ではかなりの定常負荷になってしまうため、その使用を避けて実装する。 | ||
| /// </summary> | ||
| internal sealed class MorphTargetBindingMerger |
There was a problem hiding this comment.
Expression/MorphTargetBindingManager.cs -> Expression/MorphTargetBindingManager/MorphTargetBindingManager.cs
に移動
| /// <summary> | ||
| /// index access with [_keyOrder][*] | ||
| /// </summary> | ||
| private readonly RuntimeMorphTargetBinding[][] _bindings; |
There was a problem hiding this comment.
[質問] 2次元配列ではなくジャグ配列を用いているのは要素ごとにサイズが違う可能性があるから？
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VRM10 のランタイムの処理高速化実装です。
スマートフォンやスタンドアロン VR デバイスといったモバイル環境においては Expression に関する毎フレームの定常処理の負荷は無視できない程度に大きい。
#2130 においてそもそもの呼び出し回数を半分に減らせたので x2 Faster となったが、さらに内部の処理も高速化を行う。
ExpressionMerger.SetValues()in EditorMorphTargetBindingMerger.AccumulateValue()MorphTargetBindingMerger.Apply()この PR では MorphTarget 、つまり Unity では BlendShape を対象とした Expression 処理の負荷軽減を行った。
一般的にセットアップされた VRM モデルにおいては #2130 と合わせて x3 Faster となったと期待できる。