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

Fix PolygonMesh::concatenate and its unit test #4745

Merged
merged 7 commits into from
May 13, 2021

Conversation

taehongjeong
Copy link
Contributor

@taehongjeong taehongjeong commented May 6, 2021

It is an addendum to and closes #4572.

Please also refer to the issue #4743.

duhuanxianzi and others added 2 commits January 13, 2021 16:27
ensure that each vertex id in a polygon is always in the correct range
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test would be simpler if (in lines 117-127) we were to use EXPECT_EQ_VECTORS from common/pcl_tests.h

use EXPECT_EQ_VECTORS for simplicity
@taehongjeong
Copy link
Contributor Author

@kunaltyagi Thank you for your great suggestion! I've made changes accordingly.

test/common/test_polygon_mesh.cpp Show resolved Hide resolved
test/common/test_polygon_mesh.cpp Outdated Show resolved Hide resolved
Hugh (Taehong Jeong) added 2 commits May 12, 2021 13:09
- refectored to copy gnd truth for modification
- added comments
- moved the temp vector out of the loop
- removed redundant comments
- used assignment operator instead
- put brackets in one-liner foreach loop
@taehongjeong
Copy link
Contributor Author

The build process failed because of two failing tests which are irrelevant to this issue:

106 - octree_search (Failed)
108 - organized_index (Failed)

What can I do for this?

@taehongjeong taehongjeong requested a review from kunaltyagi May 13, 2021 04:20
@kunaltyagi
Copy link
Member

kunaltyagi commented May 13, 2021

Tagging @mvieth (Our first seeds for failing tests)

@kunaltyagi kunaltyagi changed the title Fix PolygonMesh::concatenate(...) and its unit test Fix PolygonMesh::concatenate and its unit test May 13, 2021
@kunaltyagi kunaltyagi changed the title Fix PolygonMesh::concatenate and its unit test Fix PolygonMesh::concatenate and its unit test May 13, 2021
@kunaltyagi kunaltyagi added changelog: fix Meta-information for changelog generation module: common module: test needs: code review Specify why not closed/merged yet labels May 13, 2021
@mvieth
Copy link
Member

mvieth commented May 13, 2021

Tagging @mvieth (Our first seeds for failing tests)

Great, I will investigate those. Although they failed on Windows, and the rand and srand implementations seem to be system dependent, so knowing the seeds might not help me with testing on Ubuntu. But I think I can still find out the causes.

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @taehongjeong !

@mvieth mvieth merged commit 1ae7660 into PointCloudLibrary:master May 13, 2021
mvieth pushed a commit to mvieth/pcl that referenced this pull request Dec 27, 2021
)

* Update PolygonMesh.h

* Update test_polygon_mesh.cpp

ensure that each vertex id in a polygon is always in the correct range

* Update test_polygon_mesh.cpp

use EXPECT_EQ_VECTORS for simplicity

* Update test_polygon_mesh.cpp

fix for the lambda capture issue (https://dev.azure.com/PointCloudLibrary/pcl/_build/results?buildId=18956&view=logs&j=8042da28-3549-5cef-c93d-1a000d12f42a&t=118ad2c4-6739-5a83-709b-f149f9853975&l=192)

* Update test_polygon_mesh.cpp

- refectored to copy gnd truth for modification
- added comments

* Update test_polygon_mesh.cpp

- moved the temp vector out of the loop
- removed redundant comments

* Update test_polygon_mesh.cpp

- used assignment operator instead
- put brackets in one-liner foreach loop

Co-authored-by: duhuanxianzi <duhuanncepu@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Meta-information for changelog generation module: common module: test needs: code review Specify why not closed/merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PolygonMesh::concatenate(...)] indices are improperly handled
4 participants