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

PCDLoader: Get the byte length of the field correctly #24278

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

luoxuhai
Copy link
Contributor

@luoxuhai luoxuhai commented Jun 27, 2022

Description

Get the byte length of the field correctly in binary compressed PCD files.

PCD file format header Examples:

FIELDS x y z normal_x normal_y normal_z     # XYZ + surface normals

Current version of PCDLoader, the correct byte length index of normal are 3, 4 and 5:

normal.push( dataview.getFloat32( PCDheader.points * offset.normal_x + PCDheader.size[ 4 ] * i, this.littleEndian ) );
normal.push( dataview.getFloat32( PCDheader.points * offset.normal_y + PCDheader.size[ 5 ] * i, this.littleEndian ) );
normal.push( dataview.getFloat32( PCDheader.points * offset.normal_z + PCDheader.size[ 6 ] * i, this.littleEndian ) );

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 27, 2022

Can you please share a PCD files that breaks with the current version of PCDLoader but loads correctly with your PR?

@luoxuhai
Copy link
Contributor Author

luoxuhai commented Jun 28, 2022

Can you please share a PCD files that breaks with the current version of PCDLoader but loads correctly with your PR?

Sorry, I can't share my PCD file.
If a PCD file does not have RGB but has Normal it cannot be parsed correctly in the current version of PCDLoader

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

The PR seems to be okay. The existing test file pcl_logo.pcd works with this change, too. Let's give it a try!

@mrdoob mrdoob added this to the r142 milestone Jun 28, 2022
@mrdoob mrdoob merged commit 911db47 into mrdoob:dev Jun 28, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jun 28, 2022

Thanks!

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
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.

3 participants