-
Notifications
You must be signed in to change notification settings - Fork 256
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
Suggestion: Returning intersection points too when return_cell_id is true #1103
Comments
Hi, i-m a bit hesitant on this because this is very widely used ... in principle it should not be a significant overhead to recover the point from the index with |
Well, you can get the element vertices that way, but the intersection is not always at the 'midpoint', it can be anywhere, so, if the cells are "big", it may be an issue. The idea came to me cause i needed both the intersection point and the cell index and i was basically running the exact method twice just adding the return_cell_id arg. But no worries!! If it's too big an issue i can just use the face index, assume the intersection is at the midpoint and work with that, no big deal :)! I was just asking in case the method was used very locally and you wouldn't need to change much of it. Thanks! |
Yes you've got a very valid point .. I need to think if there is some way to make the change so that it is not disruptive of existing code and examples. |
If it's too hard/complicated/bitchy, don't worry, i was asking just in case it was simple. So far, in my library, i changed that line and like 1 or 2 more when it's used in the same file just adding a placeholder for the 2nd value in the unpack and haven't had any issue so far. Thanks a lot! |
Hi Marco!
Would it be possible for the pointcloud method
closest_point
to also return the intersection coordinates whenreturn_cell_id
is True?It's basically changing the line 1536 in pointcloud.py from
To:
And adding the corresponding placeholder wherever it's being called.
I've been using it in a couple projects but i basically have to change it every time vedo gets an update and risking breaking functionality somewhere else.
Thanks in advance!
The text was updated successfully, but these errors were encountered: