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

Implements expression overriding #714

Merged
merged 9 commits into from
Feb 2, 2021
Merged

Conversation

Santarh
Copy link
Contributor

@Santarh Santarh commented Feb 1, 2021

Implements expression overriding specification without GCAlloc.

vrm-c/vrm-specification#196

@Santarh Santarh requested a review from ousttrue February 1, 2021 13:30
Comment on lines 43 to 54
if (!key.IsBlink)
{
blinkOverrideRate = Mathf.Max(blinkOverrideRate, GetOverrideRate(expression.OverrideBlink, weight));
}
if (!key.IsLookAt)
{
lookAtOverrideRate = Mathf.Max(lookAtOverrideRate, GetOverrideRate(expression.OverrideLookAt, weight));
}
if (!key.IsMouth)
{
actualWeights.Add(key, weight);
mouthOverrideRate = Mathf.Max(mouthOverrideRate, GetOverrideRate(expression.OverrideMouth, weight));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

[COMMENT] 例えば、Blink系にOverrideBlinkが付いていた場合は自分自身のweightはoverrideの影響からは無視する、というような実装になっていそうですね。同意します。この挙動は仕様として記録しておいたほうが良さそうですね。

Copy link
Contributor Author

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.

ちなみに、実際そういう設定をすることってありますかね?Blink vs BlinkLeftで、BlinkLeftをBlinkでオーバーライドさせるとか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

それを実現する場合は BlinkLeft は IsBlink ではない、とする必要があると思います。
そうすると、もともとの Override をやろうとする意義に反しそう。

// Override rate without targeting myself.
if (!key.IsBlink)
{
blinkOverrideRate = Mathf.Max(blinkOverrideRate, GetOverrideRate(expression.OverrideBlink, weight));
Copy link
Contributor

@0b5vr 0b5vr Feb 2, 2021

Choose a reason for hiding this comment

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

自分がイメージしていた仕様では、ここはここまでで得られたoverrideRateとのmaxを取るのではなく、ここまでで得られたoverrideRateと加算( および1.0以下へのクランプ L58-L60で既にやってそう)を行う感じでした。
目を閉じる方向に動く表情2つを組み合わせた際にBlinkとの干渉がどう発生するかをイメージすると、加算のほうが良さそうだと思っています。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

よさそう

}
if (!key.IsMouth)
{
mouthOverrideRate = Mathf.Max(mouthOverrideRate, GetOverrideRate(expression.OverrideMouth, weight));
mouthOverrideRate += GetOverrideRate(expression.OverrideMouth, weight);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

加算にした

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.

3 participants