Capsule: "lineLineMinimumPoints" function returns global temp variables, does not use Capsule fields

[gkjohnson]

Description

I was looking through the example Capsule file and saw that the "lineLineMinimumPoints" function does not use any of the local "Capsule" members. It also returns the global temp _v1 and _v2 variables to the caller instead of taking target variables instead of making clones. It seems it's only used in the Octree file for triangle / capsule intersections and only ever takes a line representing the capsule as the first argument.

I'd fix and rename the function to something more clear but it's not clear to me exactly what it's supposed to do. Is the function just returning the closest point between the two line segments? Does it belong in the LineSegment class? Do we know where the original implementation is referenced from?

Solution

• rename function
• adjust function signature to return boolean, take target variables
• add comments for original reference function implementation

Alternatives

None

Additional context

No response

[Mugen87]

The code was added via #20836 so only @supereggbert can help here.

Bard says about the code:

The code you provided is a JavaScript function called lineLineMinimumPoints(). It takes two line objects as input and returns an array of two points, which are the points on the two lines that are closest together. The function works by first calculating the vectors r, s, and w. These vectors represent the direction vectors of the two lines and the vector between the start points of the two lines, respectively. The function then calculates the coefficients a, b, c, d, and e. These coefficients are used to calculate the intersection point of the two lines, which is the point on both lines that is closest together. The function then calculates the parameters t1 and t2. These parameters represent the distances along the two lines from the start points to the intersection point. The function then clamps the values of t1 and t2 to the range [0, 1] to ensure that they represent valid points on the two lines. Finally, the function calculates the two points that are closest together by multiplying the vectors r and s by the parameters t1 and t2 and adding the start points of the two lines, respectively.

For now, I suggest to move the code as a helper function to Octree.

[Mugen87]

Since lineLineMinimumPoints() returns module scope variables, considering this issue as a bug.

[Mugen87]

@gkjohnson Are you planning to file a PR with a fix? Otherwise I would try to fix it so the issue is resolved for r158.

[gkjohnson]

I was waiting for a reply from @supereggbert but it seems we may not get one. I can see try to make a PR in the next week that moves the function.

But as an aside I do have a couple concerns regarding the Octree example that I'm reminded of here - I've seen multiple cases of users using it and it causing massive build stalls or crashes (ref 1, ref 2, ref 3, ref 4) and I'm wondering how effectively we can maintain a utility like this in the project. An efficient octree implementation would be nice to have though generally I think properly implementing and maintaining a spatial structure like this is a fairly complex task. I'm not sure what you're thoughts are.

[Mugen87]

I'm okay with maintaining a simple spatial data structure in this repo but I see a more complex and optimized solution in an external repository. Similar to three-mesh-bvh. If there would be something like three-octree, we could replace the internal Octree class at some point.

In general, addons have different priorities/relevance for pure rendering tasks and also different complexity levels. Similar to geometry modifiers, I think spatial data structures are better maintained in external repositories.