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.

Sign up for GitHub

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

Jump to bottom

Adding red, green, blue field support to the binary, and ascii pcd lo… #28179

Closed
wants to merge 1 commit into from
Closed

Adding red, green, blue field support to the binary, and ascii pcd lo… #28179

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 32 additions & 1 deletion examples/jsm/loaders/PCDLoader.js
View file
Open in desktop
Expand Up @@ -120,7 +120,7 @@ class PCDLoader extends Loader {
PCDheader.data = result2[ 1 ];
PCDheader.headerLen = result2[ 0 ].length + result1;
PCDheader.str = data.slice( 0, PCDheader.headerLen );

console.log(PCDheader.str);
// remove comments

PCDheader.str = PCDheader.str.replace( /#.*/gi, '' );
Expand Down Expand Up @@ -283,6 +283,20 @@ class PCDLoader extends Loader {

color.push( c.r, c.g, c.b );

} else if ( (offset.red !== undefined) && (offset.green !== undefined) && (offset.blue !== undefined) ) {

const red = parseFloat( line[ offset.red ] );
const green = parseFloat( line[ offset.green ] );
const blue = parseFloat( line[ offset.blue ] );

const r = ( red >> 8 ) / 255;
const g = ( green >> 8 ) / 255;
const b = ( blue >> 8 ) / 255;

c.set( r, g, b ).convertSRGBToLinear();

color.push( c.r, c.g, c.b );

}

if ( offset.normal_x !== undefined ) {
Expand Down Expand Up @@ -387,6 +401,7 @@ class PCDLoader extends Loader {

const dataview = new DataView( data, PCDheader.headerLen );
const offset = PCDheader.offset;
console.log(offset);

for ( let i = 0, row = 0; i < PCDheader.points; i ++, row += PCDheader.rowSize ) {

Expand All @@ -410,6 +425,22 @@ class PCDLoader extends Loader {

}

if ( offset.red !== undefined ) {

const red = dataview.getFloat64( row + offset.red , true);
const green = dataview.getFloat64( row + offset.green , true);
const blue = dataview.getFloat64( row + offset.blue , true);
Copy link
Collaborator

@Mugen87 Mugen87 Apr 25, 2024 •
edited

Choose a reason for hiding this comment

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

The problem is your code is application specific since it does not implement a real standard. You are assuming here that R,G,B fields are floats but what if other tools export uint8 values?

Like mentioned above, such modifications are not reliable in library code. PCDLoader isn't a large module so it's best to copy the module and add custom properties on application level.

Copy link
Author

Choose a reason for hiding this comment

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

You are right, I can change this. But you are willing to add this capability in general aren't you?. Your first response read more like a hard no. Hence, I am asking.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still not in favor of this change, no. If we merge this PR and similar ones, we end up with an overly complex PCDLoader supporting a huge number of potentially redundant fields.

Copy link
Collaborator

@Mugen87 Mugen87 Apr 25, 2024 •
edited

Choose a reason for hiding this comment

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

Um, I wonder if we eventually need a more generic approach in PCDLoader similar to what #27901 has suggested.

The loader could parse the existing default fields like x, y, z and return a geometry as before. But now the loader also parses all other fields it finds in the PCD asset. The related data could be returned as a simple map (field name -> typed array) in the userData field of the geometry. Ideally the geometry only holds fields as attributes when they are relevant for rendering.


const r = ( red >> 8 ) / 255;
const g = ( green >> 8 ) / 255;
const b = ( blue >> 8 ) / 255;

c.set( r, g, b ).convertSRGBToLinear();

color.push( c.r, c.g, c.b );

}

if ( offset.normal_x !== undefined ) {

normal.push( dataview.getFloat32( row + offset.normal_x, this.littleEndian ) );
Expand Down