-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
computeTriangleMeshArea() : calculate the area of a triangle mesh #5804
base: master
Are you sure you want to change the base?
computeTriangleMeshArea() : calculate the area of a triangle mesh #5804
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A few general comments:
- It seems you are using
PCL_NO_PRECOMPILE
for testing? Putting the declaration in the .h file and the definition in the .hpp file like this will only work if there are explicit instantiation in the .cpp file, or ifPCL_NO_PRECOMPILE
is defined. Here I would suggest to put definition and declaration together in the .h file, since the functions are not too large. - Please add some simple test in test/surface/test_gp3.cpp. Something very simple like computing the area of a tetrahedron/pyramid is fine
- I would prefer to use
const pcl::PointCloud<PointT>&
as the parameter, then the pointer doesn't have to be dereferenced inside the function repeatedly - To be honest, I don't quite understand the second overload with the
indices
parameter. In which use case would the indices fromtriangleMesh
be used for a look-up inindices
?
@mvieth thanks for the feedback. I will come back to you later because these days, week end included, I am extremely busy. |
@mynickmynick Hi, I just wanted to ask whether you are still working on this pull request? |
yes but i am moving to switzerland the thing is on hold i have no time now
…On Sat, 2 Dec 2023, 17:42 Markus Vieth, ***@***.***> wrote:
@mynickmynick <https://github.com/mynickmynick> Hi, I just wanted to ask
whether you are still working on this pull request?
—
Reply to this email directly, view it on GitHub
<#5804 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD5EDZJZRABZBXVWRBMSGYLYHNLAZAVCNFSM6AAAAAA4RUTJPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZXGE4TSMBSGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
okay |
@mvieth please some more time, i just moved to a new country, new job and i am very busy. by the way i need to investigate on implementations of stereoscopic (binocular) 3d reconstruction for a (very) custom application (epipolar lines etc.) do we have something in PCL or can you suggest something about it? I am now using Halcon which works fine but not enough. thanks |
i am having a very hard time rebuilding and rerunning this branch because i am using here another PC where I don't think I have the original test app using it , a lot of time has passed and the mod here you made me do very different from my original modifications prepared for issue #5735 and my branch https://github.com/mynickmynick/pcl/tree/fork_getArea :)) |
I am sorry but I don't understand here why I should implement it not uniform with the other functions, they are all declared in .h and implemented in .hpp I honestly forgot the reason for PCL_NO_PRECOMPILE. I think it was some side issue with openMP if I remember correctly. openMP would not work fine without this, we also opened an issue about it also why all old notifications have disappeared from github?? that makes it even more difficult for me to reconstruct this process and find that discussion we had with Lars about PCL_NO_PRECOMPILE this is it #5717
in my original modifications differently structured in https://github.com/mynickmynick/pcl/tree/fork_getArea this case was used. now I am having a hard time reconstructing it. may be you can help. |
you have this dualism all over the PCL, make up your mind! ;)) |
minor test_gp3 test sample pcd correction minor correction minor minor
96d6388
to
3de7937
Compare
@mvieth i think now it's ready to merge. i did all you asked |
Thanks for the modifications, I have three minor additional requests:
If the cloud has to be stored, e.g. inside a class, a shared pointer is used (the most prominent example would be |
all right I cannot do it before Feb 3rd |
hi sorry i am very busy also on week ends, i will try to finalize this in the next weeks |
implements the functions computeTriangleMeshArea() : part of what discussed in issue [surface] getArea(point cloud segment) #5735
to calculate the area of a Triangles mesh
depending on cloud surface smoothness it might be advisable some preprocessing (like voxel filtering) to smooth up the surface
i tested it on different point clouds, both global or cluster-segmented and it worked fine