Skip to content

Conversation

@mqp
Copy link
Contributor

@mqp mqp commented Jun 27, 2018

This is an interesting function to improve because it's a small hotspot for the three-pathfinding library's functionality for clamping positions onto a navigation mesh.

The algorithm I implemented is very well-explained in Real-Time Collision Detection by Christer Ericson. You may be able to preview the relevant part of the book on Google Books, starting on page 136 (I don't know exactly how Google Books decides who can see what parts of what books.)

How much faster this is depends on the inputs. In the benchmark I wrote, I tried to make a representative sample of points and triangles and observed approximately an 8x speedup on both desktop Chrome and Firefox, which seems plausible. If you really want, I can produce more performance numbers, but it should clearly be much faster in all situations.

For correctness testing, I tested it alongside the old version on all of the point/triangle pairs in the benchmark and verified that the two produced the same output modulo floating-point error.

@mrdoob
Copy link
Owner

mrdoob commented Jun 27, 2018

@WestLangley looks good?

@mrdoob mrdoob added this to the r95 milestone Jun 27, 2018
@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 27, 2018

You might want to consider #8493 (comment) as well.

@WestLangley
Copy link
Collaborator

verified that the two produced the same output modulo floating-point error.

Thanks @mquander.

Let's /ping @bhouston for 2nd opinion.

@bhouston
Copy link
Contributor

Amazing improvement!

I am pretty sure @sxenos's code that @Mugen87 linked to even more efficient but probably no more than 2x faster that this optimization, so lets at least merge this and go with it but remember @sxenos' code if you need to get even faster.

@WestLangley
Copy link
Collaborator

@mrdoob It's a go!

@mrdoob
Copy link
Owner

mrdoob commented Jun 29, 2018

Hmm... @mquander any chance you could do some benchmark comparison against @sxenos' code?

@bhouston
Copy link
Contributor

I think I was wrong, I believe this algorithm is actually the same as @sxenos's.

@mqp
Copy link
Contributor Author

mqp commented Jun 29, 2018

It's clearly closely related. I personally strongly prefer using an algorithm where I can read something that tells me how it was derived and how it works, so I prefer this implementation (unless there is an explanation of sxenos's hanging around somewhere.) But if you really feel strongly, I'll benchmark the other one sometime soon.

@WestLangley
Copy link
Collaborator

Agreed. I prefer an algorithm with a published reference. Edge cases and numerical stability can be tricky.

@mrdoob
Copy link
Owner

mrdoob commented Jul 1, 2018

Sounds good!

@mrdoob mrdoob merged commit 036d9a8 into mrdoob:dev Jul 1, 2018
@mrdoob
Copy link
Owner

mrdoob commented Jul 1, 2018

Thanks!

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.

5 participants