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 incorrect offset calculation for PointCloud2 PointField fields #29

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

tokuda99
Copy link
Contributor

Description

While using the library to convert PCD data into ROS PointCloud2 messages, I encountered an issue where the offset values for the PointField fields were calculated incorrectly. This resulted in fields like intensity or other attributes being misaligned in the generated PointCloud2 message, causing visualization tools like RViz to display incorrect or missing data.

Expected Behavior

The offset for each PointField should reflect the cumulative byte position of the field within a point, considering the size and count of all preceding fields.

Actual Behavior

The current implementation calculates offset using i * type_.itemsize, which assumes a fixed-size field alignment without considering the actual data layout or field sizes. This leads to incorrect offset values for fields that do not align sequentially (e.g., fields with different sizes or counts).

Steps to Reproduce

  1. Use the library to generate a PointCloud2 message from a PCD file with fields of varying sizes and counts, such as:
  • x, y, z (float32)
  • intensity (uint8)
  • return_type (uint8)
  • channel (uint16)
  1. Publish the PointCloud2 message in ROS.
  2. Attempt to visualize the data in RViz or CloudCompare.

Example Code

Here is the problematic snippet:

for i, (field, type_, count) in enumerate(zip(self.fields, self.types, self.counts)):
    type_ = np.dtype(type_)

    fields.append(
        PointField(
            name=field,
            offset=i * type_.itemsize,  # Incorrect offset calculation
            datatype=NPTYPE_TO_PFTYPE[type_],
            count=count,
        )
    )

Suggested Fix

The offset should be calculated as the cumulative sum of the sizes of all preceding fields, adjusted for the field's count. For example:

offset = 0

for field_name, type_str, count in zip(self.fields, self.types, self.counts):
    np_type = np.dtype(type_str)
    
    fields.append(
        PointField(
            name=field_name,
            offset=offset,  # Correct cumulative offset
            datatype=NPTYPE_TO_PFTYPE[np_type],
            count=count,
        )
    )
    
    offset += np_type.itemsize * count  # Accumulate size

Impact

This issue causes data misalignment in the PointCloud2 message, leading to:

  • Visualization tools misinterpreting fields (e.g., intensity not showing up or being assigned wrong values).
  • Potential bugs in downstream processing nodes that rely on correctly aligned fields.

Testing

Tested with PCD files containing fields of varying sizes (float32, uint8, uint16).
Verified the corrected offsets using RViz and CloudCompare.

@urasakikeisuke urasakikeisuke self-requested a review January 27, 2025 22:01
@urasakikeisuke urasakikeisuke added rd/fix Bug fix for the user, not a fix to a build script rd/patch labels Jan 27, 2025
@urasakikeisuke
Copy link
Collaborator

LGTM! Thanks for your great work. We really appreciate it!

@urasakikeisuke urasakikeisuke merged commit 6fb7210 into MapIV:main Jan 27, 2025
5 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rd/fix Bug fix for the user, not a fix to a build script rd/patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect offset calculation in PointField when creating PointCloud2 messages
2 participants