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

Extend _bvhClosestPointToPoint to accept maxDistance #732

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TheBlek
Copy link

@TheBlek TheBlek commented Dec 26, 2024

Recently I needed to find a closest mesh to the point on the gpu. It was pretty fast, considering the complexity. But it could be faster, if we allowed setting lower starting closestDistanceSquared thus skipping a lot of useless variants. Also, this would allow finding distances farther than 100000 (if this could in fact happen at all)

The change will not disrupt existing usage, but add a new possibility for situational performance improvement.

If you are ok with the change, should I document this somewhere? I couldn't find explicit documentation for shader function

@TheBlek
Copy link
Author

TheBlek commented Dec 28, 2024

@gkjohnson Just in case you didn't get the notification. I don't have ability to set you as a reviewer

@gkjohnson
Copy link
Owner

Hello! Thanks for the contribution. Is it possible to use two functions with the same name but different function signatures? Otherwise I feel inclined to just replace the old signature with the new one you've proposed.

@TheBlek
Copy link
Author

TheBlek commented Jan 9, 2025

Is it possible to use two functions with the same name but different function signatures?

Unfortunately, I believe it's not possible to have different macros with the same name. Shall I replace the signatures then? It's a breaking change after all.

@gkjohnson
Copy link
Owner

Shall I replace the signatures then? It's a breaking change after all.

I thought that might be the case - yeah let's just change the signature of the original function, then. It's okay that it's breaking. Lets just make sure the demos that use the function continue to work, as well.

@TheBlek
Copy link
Author

TheBlek commented Jan 14, 2025

l replaced the function and fixed some broken file links in the GPU section of README. @gkjohnson can you take a look?

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.

2 participants