Three.js: PCDLoader incorrect colors from ascii PCD files

[focaldepth]

Description of the problem

The decoding of RGB colors in incorrect in PCDLoader when decoding ascii files..

three.js/examples/js/loaders/PCDLoader.js

I am basing this fact on PCD files that display correctly using Debian version of pcl_viewer.

We need to treat the data as a 32 bit integer and the color byte order is reversed.

Here is a code fix for the problem:

  //var c = new Float32Array( [ parseFloat( line[ offset.rgb ] ) ] );
  //var dataview = new DataView( c.buffer, 0 );
  //color.push( dataview.getUint8( 0 ) / 255.0 );
  //color.push( dataview.getUint8( 1 ) / 255.0 );
  //color.push( dataview.getUint8( 2 ) / 255.0 );

   var c = new Int32Array( [ parseInt( line[ offset.rgb ] ) ] );
   var dataview = new DataView( c.buffer, 0 );
   color.push( dataview.getUint8( 2 ) / 255.0 );
   color.push( dataview.getUint8( 1 ) / 255.0 );
   color.push( dataview.getUint8( 0 ) / 255.0 );

Three.js version

Latest as of 2017 - 09 -15

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [] All of them
• [ ] Windows
• [ ] macOS
• [x ] Linux
• [ ] Android
• [ ] iOS

Hardware Requirements (graphics card, VR Device, ...)

N/A

[Mugen87]

The mentioned code section is currently not executed by the official example because the corresponding file is in binary format. So maybe this code was never tested before.

Would you like to do a PR with your changes? Besides, it would be cool if you could add a simple PCD ascii file for testing purposes to /examples/models/pcd/.

[Mugen87]

I've verified the issue with a simple ascii file. The current implementation seems to be correct, the file loads with nice vertex colors. see #12213

Tested on mac with this file (see bottom of the page): http://pointclouds.org/documentation/tutorials/pcd_file_format.php

[Mugen87]

@focaldepth BTW: If I implement your proposed fix, the loader does not parse the color of the mentioned PCD file correctly.

[Mugen87]

@focaldepth Can you please share one of your PCD files that does not load with PCDLoader on your system?

[Mugen87]

Closing. We can reopen the issue if @focaldepth provides a file for testing purposes.

[baba1472]

@Mugen87 is it possible to re-open this issue? I am building on top of ThreeJS using the PCDLoader and I am noticing also that the colors are not coming out correctly. I have attached two screenshots:

• The first is a full window in Ubuntu showing the PCD in the correct colors using the installed PCL Viewer

• The second shows a half window in Chrome using ThreeJS showing the incorrect colors

• The third is the PCD file used in the screenshots. The file extension was changed to .txt to be able to upload to Github

pcd_0000000 copy.txt

[baba1472]

I also noticed that the colors are different for the mentioned pcd example file from PCL when displayed in the installed PCL Viewer vs the ThreeJS PCDLoader. See below:

[WestLangley]

The loader is not decoding the colors correctly. This appears to work:

var rgb = parseFloat( line[ offset.rgb ] );
var r = ( rgb >> 16 ) & 0x0000ff;
var g = ( rgb >> 8 ) & 0x0000ff;
var b = ( rgb >> 0 ) & 0x0000ff;
color.push( r / 255, g / 255, b / 255 );

There may be a more-elegant solution.

Plus, it should be:

material.vertexColors = THREE.VertexColors;

[baba1472]

Yup, that seems to fix it for ascii files, thanks!

[Mugen87]

Does this fix also work with the PCD file shown at the bottom of the following website?

http://pointclouds.org/documentation/tutorials/pcd_file_format.php

@focaldepth's proposed solution was not able to load it.

[baba1472]

@Mugen87 Yes it works, they both now show the Pointcloud logo in purple and green.

[baba1472]

Note
I copied @WestLangley code into the ascii conditional of the PCDLoader.js file only, so I am unsure about the fix for the binary data conditional, but I assume it is similar.

[Mugen87]

@baba1472 Thanks for testing!

Um, I wonder if there is a solution which does not require bitwise operations. This stuff is always somewhat hard to read...

[baba1472]

@Mugen87 Sure no problem. To remove bitwise operations, this seems to work as well:

var rgb = parseFloat( line[ offset.rgb ] );
var r = rgb / 256 / 256 % 256;
var g = rgb / 256 % 256;
var b = rgb % 256;
color.push( r / 255, g / 255, b / 255 );

[WestLangley]

Compare with the source code of Color.setHex().

[baba1472]

I actually just noticed that the OP's solution (instead of 0, 1, 2 using 2, 1, 0) does work with my files as well as the sample file, and it works for both ascii and binary.

ASCII

var c = new Float32Array( [ parseFloat( line[ offset.rgb ] ) ] );
var dataview = new DataView( c.buffer, 0 );
color.push( dataview.getUint8( 2 ) / 255.0 );
color.push( dataview.getUint8( 1 ) / 255.0 );
color.push( dataview.getUint8( 0 ) / 255.0 );

BINARY

color.push( dataview.getUint8( row + offset.rgb + 2 ) / 255.0 );
color.push( dataview.getUint8( row + offset.rgb + 1 ) / 255.0 );
color.push( dataview.getUint8( row + offset.rgb + 0 ) / 255.0 );

[baba1472]

Actually I take that back, it works for binary but not always for ascii (works for the PCL sample PCD file for both though).

The safest solution seems to be the one @WestLangley or I posted

[Mugen87]

Would you like to do a PR with @WestLangley's suggestion? In this way, the code is at least consistent with the implementation of Color.setHex().

[baba1472]

Sure I can, or @WestLangley can since it was his suggestion.

[WestLangley]

@baba1472 Feel free. I just offered some help. This code needs to be tested by the maintainers.

[baba1472]

@Mugen87 I don't seem to have permission to publish my branch, to then submit a PR.

[Mugen87]

Strange. How does your workflow look like? Maybe the following two guides might help a bit:

https://help.github.com/articles/fork-a-repo/
https://github.com/mrdoob/three.js/wiki/How-to-contribute-to-three.js

[baba1472]

Thank you, pull request made @Mugen87