PointCloudLibrary / PointCloudLibrary/pcl

PCLPointCLoud2 needs functionality to drop fields marked as skip (name == "_")

Open
#3,336 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind: todo module: common
Dominant language
C++
Stars
11.1k
Forks
4.7k
Avg merge
4d 10h
Merged PRs (30d)
6

Description

PR #3320 is removing functionality of dropping the skip fields on concatenation IFF the two point clouds are the same. There needs to be a replacement for the same

Expected Behavior

A function to remove unneded fields

Possible Solution

A decent starting point is

bool
pcl::PCLPointCloud2::squeeze ()
{
  const auto size = width * height;
  struct offset
  {
    std::size_t from = 0, to = 0, size = 0;
    explicit offset(std::size_t f = 0, std::size_t t = 0, std::size_t s = 0): from(f), to(t), size(s) {}
  };
  std::vector<offset> memcpy_offsets;
  // worst case: every second field needs to be moved
  memcpy_offsets.reserve (fields.size ()/2 + 1);

  offset running_offset;
  bool skipping = false;
  for (const auto& field: fields)
  {
    const auto& size = field.count * pcl::getFieldSize (field.datatype);

    if (field.name != "_")
    {
      skipping = false;
      running_offset.size += size;
      continue;
    }
    if (skipping)
    {
      running_offset.from += size;
      continue;
    }
    skipping = true;
    memcpy_offsets.emplace_back(running_offset);
    running_offset.from += running_offset.size;
    running_offset.to += running_offset.size;
    running_offset.size = 0;
  }
  if (!skipping)
  {
    memcpy_offsets.emplace_back(running_offset);
  }

  for (std::size_t cp = 0; cp < size; ++cp)
  {
    bool modified = false;
    for (const auto& memloc: memcpy_offsets)
    {
      if (memloc.to == memloc.from)
      {
        continue;
      }
      modified = true;
      memcpy (reinterpret_cast<char*> (&data[cp * point_step + memloc.to]),
              reinterpret_cast<const char*> (&data[cp * point_step + memloc.from]),
              memloc.size);
    }
    if (!modified)
    {
      break;
    }
  }

  const auto data_size = std::accumulate(memcpy_offsets.begin (), memcpy_offsets.end (), 0,
    [](const auto& sum, const auto& offset)
    {
      return sum + offset.size;
    });
  data.resize (data_size);

  // Removal by indices would only save 1 comparison and require memory for index vector
  fields.erase (std::remove_if(fields.begin (), fields.end (), [](const auto& field)
                {
                  return field.name == "_";
                }),
                fields.end ());
  return true;
}

This has error in the memcpy region of code

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing PR #3320 and the proposed PCLPointCloud2::squeeze implementation in this issue, then reproduce or isolate the reported error in the memcpy region. Done means providing functionality that removes fields named "_" while preserving the remaining point-cloud data and field metadata.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-vision
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.