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

Suggestion: Returning intersection points too when return_cell_id is true #1103

Open
ManuGraiph opened this issue Apr 23, 2024 · 4 comments
Open
Labels
enhancement long-term longer term tricky fix or enhancement

Comments

@ManuGraiph
Copy link

Hi Marco!

Would it be possible for the pointcloud method closest_point to also return the intersection coordinates when return_cell_id is True?

It's basically changing the line 1536 in pointcloud.py from

if return_cell_id:
    return int(cid)

To:

if return_cell_id:
    return int(cid),np.array(trgp)

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!

@marcomusy
Copy link
Owner

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 myarray[cid] ?

@ManuGraiph
Copy link
Author

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!

@marcomusy
Copy link
Owner

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.

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.

@marcomusy marcomusy added enhancement long-term longer term tricky fix or enhancement labels Apr 24, 2024
@ManuGraiph
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement long-term longer term tricky fix or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants