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 for issue #905. #912

Merged
merged 2 commits into from
Sep 18, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion octree/include/pcl/octree/octree_pointcloud_adjacency.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ namespace pcl

inline iterator begin () { return (leaf_vector_.begin ()); }
inline iterator end () { return (leaf_vector_.end ()); }

inline LeafContainerT* at (size_t idx) { return leaf_vector_.at (idx); }

// Size of neighbors
inline size_t size () const { return leaf_vector_.size (); }

Expand Down
16 changes: 10 additions & 6 deletions octree/include/pcl/octree/octree_pointcloud_adjacency_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace pcl
class OctreePointCloudAdjacencyContainer : public OctreeContainerBase
{
public:
typedef std::set<OctreePointCloudAdjacencyContainer*> NeighborSetT;
typedef std::list<OctreePointCloudAdjacencyContainer*> NeighborSetT;
Copy link
Member

Choose a reason for hiding this comment

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

By switching from set to list you give up the invariant that there are no duplicate entries. Just to make sure: was this assumption used in code? And if so, do we now have checks to make sure it holds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@taketwo No, there are no checks at the moment to make sure no duplicate neighbors are added.
Neighbors are added quite simply - we just check the 26 neighborhood by octree keys and add them if they exist.

There is no way to add new points into the octree after it is constructed, so the only function that modifies the list of neighbors is computeNeighbors in octree_pointcloud_adjacency - meaning that the only way to get duplicates is if a user manually adds in neighbors.

I'm thinking I should modify the OctreePointCloudAdjacencyContainer so that only const_iterators and getData() can be used from outside of OctreePointCloudAdjacency. It doesn't really make sense for users to be messing with anything else.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll rename that typedef too...

//iterators to neighbors
typedef typename NeighborSetT::iterator iterator;
typedef typename NeighborSetT::const_iterator const_iterator;
Expand All @@ -64,8 +64,6 @@ namespace pcl
inline const_iterator end () const { return (neighbors_.end ()); }
//size of neighbors
inline size_t size () const { return neighbors_.size (); }
//insert for neighbors
inline std::pair<iterator, bool> insert (OctreePointCloudAdjacencyContainer* neighbor) { return neighbors_.insert (neighbor); }

/** \brief Class initialization. */
OctreePointCloudAdjacencyContainer () :
Expand Down Expand Up @@ -131,7 +129,7 @@ namespace pcl
void
addNeighbor (OctreePointCloudAdjacencyContainer *neighbor)
{
neighbors_.insert (neighbor);
neighbors_.push_back (neighbor);
}

/** \brief Remove neighbor from neighbor set.
Expand All @@ -140,8 +138,14 @@ namespace pcl
void
removeNeighbor (OctreePointCloudAdjacencyContainer *neighbor)
{
neighbors_.erase (neighbor);

for (iterator neighb_it = neighbors_.begin(); neighb_it != neighbors_.end(); ++neighb_it)
{
if ( *neighb_it == neighbor)
{
neighbors_.erase (neighb_it);
return;
}
}
}

/** \brief Returns the number of neighbors this leaf has
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ pcl::SupervoxelClustering<PointT>::extract (std::map<uint32_t,typename Supervoxe

//double t_prep = timer_.getTime ();
//std::cout << "Placing Seeds" << std::endl;
std::vector<PointT, Eigen::aligned_allocator<PointT> > seed_points;
selectInitialSupervoxelSeeds (seed_points);
std::vector<int> seed_indices;
selectInitialSupervoxelSeeds (seed_indices);
//std::cout << "Creating helpers "<<std::endl;
createSupervoxelHelpers (seed_points);
createSupervoxelHelpers (seed_indices);
//double t_seeds = timer_.getTime ();


Expand Down Expand Up @@ -340,15 +340,15 @@ pcl::SupervoxelClustering<PointT>::makeSupervoxels (std::map<uint32_t,typename S

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
template <typename PointT> void
pcl::SupervoxelClustering<PointT>::createSupervoxelHelpers (std::vector<PointT, Eigen::aligned_allocator<PointT> > &seed_points)
pcl::SupervoxelClustering<PointT>::createSupervoxelHelpers (std::vector<int> &seed_indices)
{

supervoxel_helpers_.clear ();
for (size_t i = 0; i < seed_points.size (); ++i)
for (size_t i = 0; i < seed_indices.size (); ++i)
{
supervoxel_helpers_.push_back (new SupervoxelHelper(i+1,this));
//Find which leaf corresponds to this seed index
LeafContainerT* seed_leaf = adjacency_octree_->getLeafContainerAtPoint (seed_points[i]);
LeafContainerT* seed_leaf = adjacency_octree_->at(seed_indices[i]);//adjacency_octree_->getLeafContainerAtPoint (seed_points[i]);
if (seed_leaf)
{
supervoxel_helpers_.back ().addLeaf (seed_leaf);
Expand All @@ -362,7 +362,7 @@ pcl::SupervoxelClustering<PointT>::createSupervoxelHelpers (std::vector<PointT,
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
template <typename PointT> void
pcl::SupervoxelClustering<PointT>::selectInitialSupervoxelSeeds (std::vector<PointT, Eigen::aligned_allocator<PointT> > &seed_points)
pcl::SupervoxelClustering<PointT>::selectInitialSupervoxelSeeds (std::vector<int> &seed_indices)
{
//TODO THIS IS BAD - SEEDING SHOULD BE BETTER
//TODO Switch to assigning leaves! Don't use Octree!
Expand All @@ -379,7 +379,7 @@ pcl::SupervoxelClustering<PointT>::selectInitialSupervoxelSeeds (std::vector<Poi

std::vector<int> seed_indices_orig;
seed_indices_orig.resize (num_seeds, 0);
seed_points.clear ();
seed_indices.clear ();
std::vector<int> closest_index;
std::vector<float> distance;
closest_index.resize(1,0);
Expand All @@ -398,7 +398,7 @@ pcl::SupervoxelClustering<PointT>::selectInitialSupervoxelSeeds (std::vector<Poi

std::vector<int> neighbors;
std::vector<float> sqr_distances;
seed_points.reserve (seed_indices_orig.size ());
seed_indices.reserve (seed_indices_orig.size ());
float search_radius = 0.5f*seed_resolution_;
// This is number of voxels which fit in a planar slice through search volume
// Area of planar slice / area of voxel side
Expand All @@ -409,7 +409,7 @@ pcl::SupervoxelClustering<PointT>::selectInitialSupervoxelSeeds (std::vector<Poi
int min_index = seed_indices_orig[i];
if ( num > min_points)
{
seed_points.push_back (voxel_centroid_cloud_->points[min_index]);
seed_indices.push_back (min_index);
}

}
Expand Down Expand Up @@ -437,11 +437,15 @@ pcl::SupervoxelClustering<PointT>::reseedSupervoxels ()
sv_itr->getXYZ (point.x, point.y, point.z);
voxel_kdtree_->nearestKSearch (point, 1, closest_index, distance);

LeafContainerT* seed_leaf = adjacency_octree_->getLeafContainerAtPoint (voxel_centroid_cloud_->points[closest_index[0]]);
LeafContainerT* seed_leaf = adjacency_octree_->at (closest_index[0]);
if (seed_leaf)
{
sv_itr->addLeaf (seed_leaf);
}
else
{
PCL_WARN ("Could not find leaf in pcl::SupervoxelClustering<PointT>::reseedSupervoxels - supervoxel will be deleted \n");
}
}

}
Expand Down Expand Up @@ -916,7 +920,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::removeLeaf (LeafContainerT*
template <typename PointT> void
pcl::SupervoxelClustering<PointT>::SupervoxelHelper::removeAllLeaves ()
{
typename std::set<LeafContainerT*>::iterator leaf_itr;
typename SupervoxelHelper::iterator leaf_itr;
for (leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr)
{
VoxelData& voxel = ((*leaf_itr)->getData ());
Expand All @@ -935,7 +939,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::expand ()
std::vector<LeafContainerT*> new_owned;
new_owned.reserve (leaves_.size () * 9);
//For each leaf belonging to this supervoxel
typename std::set<LeafContainerT*>::iterator leaf_itr;
typename SupervoxelHelper::iterator leaf_itr;
for (leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr)
{
//for each neighbor of the leaf
Expand Down Expand Up @@ -976,7 +980,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::expand ()
template <typename PointT> void
pcl::SupervoxelClustering<PointT>::SupervoxelHelper::refineNormals ()
{
typename std::set<LeafContainerT*>::iterator leaf_itr;
typename SupervoxelHelper::iterator leaf_itr;
//For each leaf belonging to this supervoxel, get its neighbors, build an index vector, compute normal
for (leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr)
{
Expand Down Expand Up @@ -1019,7 +1023,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::updateCentroid ()
centroid_.normal_ = Eigen::Vector4f::Zero ();
centroid_.xyz_ = Eigen::Vector3f::Zero ();
centroid_.rgb_ = Eigen::Vector3f::Zero ();
typename std::set<LeafContainerT*>::iterator leaf_itr = leaves_.begin ();
typename SupervoxelHelper::iterator leaf_itr = leaves_.begin ();
for ( ; leaf_itr!= leaves_.end (); ++leaf_itr)
{
const VoxelData& leaf_data = (*leaf_itr)->getData ();
Expand All @@ -1041,10 +1045,8 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::getVoxels (typename pcl::Po
voxels->clear ();
voxels->resize (leaves_.size ());
typename pcl::PointCloud<PointT>::iterator voxel_itr = voxels->begin ();
//typename std::set<LeafContainerT*>::iterator leaf_itr;
for (typename std::set<LeafContainerT*>::const_iterator leaf_itr = leaves_.begin ();
leaf_itr != leaves_.end ();
++leaf_itr, ++voxel_itr)
typename SupervoxelHelper::const_iterator leaf_itr;
for ( leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr, ++voxel_itr)
{
const VoxelData& leaf_data = (*leaf_itr)->getData ();
leaf_data.getPoint (*voxel_itr);
Expand All @@ -1058,7 +1060,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::getNormals (typename pcl::P
normals.reset (new pcl::PointCloud<Normal>);
normals->clear ();
normals->resize (leaves_.size ());
typename std::set<LeafContainerT*>::const_iterator leaf_itr;
typename SupervoxelHelper::const_iterator leaf_itr;
typename pcl::PointCloud<Normal>::iterator normal_itr = normals->begin ();
for (leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr, ++normal_itr)
{
Expand All @@ -1073,7 +1075,7 @@ pcl::SupervoxelClustering<PointT>::SupervoxelHelper::getNeighborLabels (std::set
{
neighbor_labels.clear ();
//For each leaf belonging to this supervoxel
typename std::set<LeafContainerT*>::const_iterator leaf_itr;
typename SupervoxelHelper::const_iterator leaf_itr;
for (leaf_itr = leaves_.begin (); leaf_itr != leaves_.end (); ++leaf_itr)
{
//for each neighbor of the leaf
Expand Down
26 changes: 21 additions & 5 deletions segmentation/include/pcl/segmentation/supervoxel_clustering.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,16 +329,16 @@ namespace pcl
prepareForSegmentation ();

/** \brief This selects points to use as initial supervoxel centroids
* \param[out] seed_points The selected points
* \param[out] seed_indices The selected leaf indices
*/
void
selectInitialSupervoxelSeeds (std::vector<PointT, Eigen::aligned_allocator<PointT> > &seed_points);
selectInitialSupervoxelSeeds (std::vector<int> &seed_indices);

/** \brief This method creates the internal supervoxel helpers based on the provided seed points
* \param[in] seed_points The selected points
* \param[in] seed_indices Indices of the leaves to use as seeds
*/
void
createSupervoxelHelpers (std::vector<PointT, Eigen::aligned_allocator<PointT> > &seed_points);
createSupervoxelHelpers (std::vector<int> &seed_indices);

/** \brief This performs the superpixel evolution */
void
Expand Down Expand Up @@ -399,6 +399,22 @@ namespace pcl
class SupervoxelHelper
{
public:
/** \brief Comparator for LeafContainerT pointers - used for sorting set of leaves
* \note Compares by index in the overall leaf_vector. Order isn't important, so long as it is fixed.
*/
struct compareLeaves
{
bool operator() (LeafContainerT* const &left, LeafContainerT* const &right) const
{
const VoxelData& leaf_data_left = left->getData ();
const VoxelData& leaf_data_right = right->getData ();
return leaf_data_left.idx_ < leaf_data_right.idx_;
}
};
typedef std::set<LeafContainerT*,SupervoxelHelper::compareLeaves> LeafSetT;
typedef typename LeafSetT::iterator iterator;
typedef typename LeafSetT::const_iterator const_iterator;

SupervoxelHelper (uint32_t label, SupervoxelClustering* parent_arg):
label_ (label),
parent_ (parent_arg)
Expand Down Expand Up @@ -479,7 +495,7 @@ namespace pcl
size () const { return leaves_.size (); }
private:
//Stores leaves
std::set<LeafContainerT*> leaves_;
LeafSetT leaves_;
uint32_t label_;
VoxelData centroid_;
SupervoxelClustering* parent_;
Expand Down