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

Fixed compile error related to example projects #1315

Merged
merged 1 commit into from
Sep 6, 2015

Conversation

simbaforrest
Copy link

Related issues:

#1313

#1314

@taketwo
Copy link
Member

taketwo commented Aug 30, 2015

Can you please explain the first fix (in "example_difference_of_normals.cpp")? Everything else looks good.

@simbaforrest
Copy link
Author

Without the change, the line 206 and the line 114 will lead to unresolved external symbols since only core point types were instantiated in the pcl's dll.

@taketwo
Copy link
Member

taketwo commented Aug 31, 2015

This is strange, I'll need to test this. But anyway, we should not manipulate PCL_NO_PRECOMPILE inside source files, this is something that is set by CMake when configuring the project.

@simbaforrest
Copy link
Author

I guess then the other way is to add

#include <pcl/features/impl/normal_3d_omp.hpp>

below line 19

and add

#include <pcl/segmentation/impl/extract_clusters.hpp>

below line 22

since without this "PCL_NO_PRECOMPILE", this two hpp files will not be included and in the example project, the types

typedef pcl::PointNormal PointNT;
typedef pcl::PointNormal PointOutT;

will leave some functions of

pcl::NormalEstimationOMP<PointT, PointNT> ne;

and

pcl::EuclideanClusterExtraction<PointOutT> ec;

undefined.

@taketwo
Copy link
Member

taketwo commented Sep 1, 2015

How about other example programs though? Shouldn't the same problem manifest in them as well?

@simbaforrest
Copy link
Author

Other examples are built just fine. I guess they don't use any non-default point types.

BTW, just want to note that I built with the PCL_NO_PRECOMPILE off. So maybe this example project should add some compile time switch...

@taketwo
Copy link
Member

taketwo commented Sep 1, 2015

Ok, I also get the error with PCL_NO_PRECOMPILE=OFF and PCL_ONLY_CORE_POINT_TYPES=ON.

I think the right way to solve this is to include the corresponding headers from impl/ as you suggested. However I would wrap them in #ifdef PCL_ONLY_CORE_POINT_TYPES.

@simbaforrest
Copy link
Author

Sure, I'll update and submit another pull request then.

@taketwo
Copy link
Member

taketwo commented Sep 1, 2015

You don't need to make another PR. Just rewrite your last commit locally and push with --force flag.

@taketwo
Copy link
Member

taketwo commented Sep 1, 2015

Can you actually join both #ifdefs so we have cleaner code? And one last thing will be to squash the commits in this pull request so that we do not pollute the history.

git rebase -i HEAD~2

Mark the second commit as "fixup", save and exit. Once rebasing is done, git push --force, and this PR will be automatically updated. Thank you!

@simbaforrest
Copy link
Author

This will be cleaned up code

#include <string>

#include <pcl/point_types.h>
#include <pcl/io/pcd_io.h>
#include <pcl/kdtree/kdtree_flann.h>
#include <pcl/common/point_operators.h>
#include <pcl/common/io.h>
#include <pcl/search/organized.h>
#include <pcl/search/octree.h>
#include <pcl/search/kdtree.h>
#include <pcl/features/normal_3d_omp.h>
#include <pcl/filters/conditional_removal.h>
#include <pcl/segmentation/sac_segmentation.h>
#include <pcl/segmentation/extract_clusters.h>
#include <pcl/io/vtk_io.h>
#include <pcl/filters/voxel_grid.h>

#include <pcl/features/don.h>

#ifdef PCL_ONLY_CORE_POINT_TYPES
#include <pcl/features/impl/normal_3d_omp.hpp>
#include <pcl/segmentation/impl/extract_clusters.hpp>
#endif

Sorry I'm not that familiar with git rebase, I tried "git rebase -i HEAD~2" but nothing seems to happen. Should I do this after this new commit or before?

@simbaforrest
Copy link
Author

Got it, since with the interactive rebase, I have to setup my git to work with my notepad++ like this...

@taketwo
Copy link
Member

taketwo commented Sep 2, 2015

Apparently working with terminal Git is a bit more complicated on Windows :D
Sorry again, but could you please squash these two commits into a single one? Then I'll merge.

@simbaforrest
Copy link
Author

Now there should be only one commit :)

@taketwo
Copy link
Member

taketwo commented Sep 6, 2015

Perfect! Thank you.

taketwo added a commit that referenced this pull request Sep 6, 2015
Fixed compile error related to example projects
@taketwo taketwo merged commit 7cc4806 into PointCloudLibrary:master Sep 6, 2015
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