[VRMC_springBone_limit] fix clamp - #2832
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates UniVRM’s VRMC_springBone_limit handling to better match the specification by clamping parsed limit angles and adding/adjusting runtime angle-limit behavior (including singularity fallbacks) in the SpringBone job angle-limit implementations.
Changes:
- Clamp imported Cone/Hinge angles to
[0, π], and Spherical pitch/yaw to spec maxima during import. - Update runtime Anglelimit implementations (Cone/Hinge/Spherical) to clamp inputs and apply singularity-aware fallback directions.
- Refactor
Anglelimit.Applyto share common computations across limit types and tweak axis-rotation special-casing.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Packages/VRM10/Runtime/IO/Vrm10Importer.cs | Clamp VRMC_springBone_limit values during import to spec bounds. |
| Packages/UniGLTF/Runtime/SpringBoneJobs/Anglelimit/AnglelimitSpherical.cs | Implement pitch/yaw clamping and singularity fallback handling for spherical limits. |
| Packages/UniGLTF/Runtime/SpringBoneJobs/Anglelimit/AnglelimitHinge.cs | Add clamping and singularity fallback handling for hinge limits. |
| Packages/UniGLTF/Runtime/SpringBoneJobs/Anglelimit/AnglelimitCone.cs | Add clamping and singularity fallback handling for cone limits. |
| Packages/UniGLTF/Runtime/SpringBoneJobs/Anglelimit/Anglelimit.cs | Refactor shared computations and adjust axis-rotation singularity handling. |
Suppressed comments (1)
Packages/UniGLTF/Runtime/SpringBoneJobs/Anglelimit/AnglelimitSpherical.cs:26
math.abs(tailDir.x) == 1.0fの完全一致判定は誤差で成立しづらく、±X 特異点フォールバックが効かないことがあります。しきい値比較にしてください。
else if (math.abs(tailDir.x) == 1.0f)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| using System; | ||
| using Unity.Mathematics; | ||
| using UnityEngine; |
| if (math.abs(phi) > limitAnglePhi) | ||
| // tailDirのpitch・yawを計算する | ||
| float pitch; | ||
| if (tailDir.y == -1.0) |
There was a problem hiding this comment.
| var projectedLengthSquared = tailDir.y * tailDir.y + tailDir.z * tailDir.z; | ||
| if (projectedLengthSquared == 0.0f) | ||
| { |
| // tailDirがy軸負方向の場合、z軸正方向側を選択する | ||
| var zSign = (tailDir.z < 0.0f) ? -1.0f : 1.0f; | ||
| tailDir.y = cosLimitAngle; | ||
| tailDir.z = sinLimitAngle * zSign; |
| var horizontalLengthSquared = 1.0f - tailDir.y * tailDir.y; | ||
|
|
||
| if (horizontalLengthSquared == 0.0) | ||
| { |
| if (dot == -1.0) | ||
| { | ||
| return new quaternion(1f, 0f, 0f, 0f); | ||
| // headからtailに向かうベクトルがY-方向の場合、X軸周りに180度回転させた回転を設定する | ||
| return new quaternion(1, 0, 0, 0); | ||
| } |
| // tailDirがy軸負方向の場合、Z軸正方向側の境界を選択するため、pitchをπとする | ||
| pitch = math.PI; | ||
| } | ||
| else if (math.abs(tailDir.x) == 1.0f) |
There was a problem hiding this comment.
[MAY] こちらもthree-vrmではEPSILONを入れた特異点処理としています。こちらは誤差付きで比較しなくてもそこまで問題にならないかも
| { | ||
| pitch = math.atan2(tailDir.z, tailDir.y); | ||
| } | ||
| var yaw = math.asin(tailDir.x); |
There was a problem hiding this comment.
SHOULD: tailDir.x の範囲が [-1.0, 1.0] の外に行くと、math.asinが NaN を返し得ます。おそらく、-1, 1でclampしたほうが良いです。
| var cosAngle = math.cos(limitAngle); | ||
| if (tailDir.y < cosAngle) | ||
| var projectedLengthSquared = tailDir.y * tailDir.y + tailDir.z * tailDir.z; | ||
| if (projectedLengthSquared == 0.0f) |
There was a problem hiding this comment.
MAY: こちらも、three-vrmでは、誤差を考慮した比較を行っています。
| tailDir.z *= ratio; | ||
| var horizontalLengthSquared = 1.0f - tailDir.y * tailDir.y; | ||
|
|
||
| if (horizontalLengthSquared == 0.0) |
There was a problem hiding this comment.
SHOULD: 誤差を考慮した比較を行ったほうが良さそうです。three-vrm側はこんな感じです:
|
私からの指摘事項は、Copilotによる指摘と同じく、全体的に浮動小数点誤差周りの取扱に関するもののみです。それ以外は良いと思います！ |
fixed #2827