Unity-Technologies / com.unity.demoteam.hair

An integrated solution for authoring / importing / simulating / rendering strand-based hair in Unity.
Other
745 stars 101 forks source link

Resolve From Mesh - BVH Returning Incorrect Triangles #98

Closed Kleptine closed 2 months ago

Kleptine commented 3 months ago

Thanks for the great package! The hair package has been a joy to use.

I've noticed that the "Closest Triangle" calculation seems to return slightly incorrect results, when using the "Resolve from Mesh" option to construct root UVs. For example, here I'm showing the UVs on the hair and the mesh. The hair UVs occasionally grab from the wrong triangle:

Screenshot 2024-07-26 at 1 36 59 PM

It seems to select a nearby triangle, so it's most noticeable on the seams between UV islands. I've attached an alembic file with the groom and mesh, and the debug shaders so you can reproduce it.

ReproAlembicGroom.zip

Kleptine commented 3 months ago

Ok, it took me a moment, but I figured out the BVH bug. The issue is due to the heuristic used for selecting the Left/Right nodes in FindClosestLeaf. This uses the Squared Signed Distance from the point to the node. This is correct for points outside the node, but for a point inside the node, the Signed Distance will be negative. Squaring that will then produce a positive distance, treating points inside the node as if they are further away.

Since a point can only be in one node or the other, we can solve this by clamping the SDF value before squaring:

public static float SqDistanceNodePoint(Node* nodePtr, in float3 p)
{
    // see: "distance functions" by Inigo Quilez
    // https://www.iquilezles.org/www/articles/distfunctions/distfunctions.htm

    var b = 0.5f * (nodePtr->data.max - nodePtr->data.min);
    var c = 0.5f * (nodePtr->data.max + nodePtr->data.min);
    var q = abs(p - c) - b;
    var d = length(max(q, 0.0f)) + min(max(q.x, max(q.y, q.z)), 0.0f);

    // fix: clamp the signed distance so it doesn't ever go negative.
    d = max(0, d);

    return (d * d);
}

This results in the correct triangles and UVs:

Screenshot 2024-07-26 at 3 17 34 PM

Happy to submit a PR, but the fix is just one line. Cheers!

fuglsang commented 2 months ago

Thank you for the feedback and detailed report! I've now pushed a fix for this, which also reduces the cost of SqDistanceNodePoint a bit.