[WIP] Set default alpha value to 255

[VictorLamoine]

I think we should set the alpha value to 255 in the pcl::RGB struct.
This fixes the second test case of #1040.

[taketwo]

Yes, it is logical. I wondered why it was not done before and found the following two commits in the history. The first one defaulted alpha to 255 everywhere, and the second one reverted it. I am not sure why the float <-> integer cast mentioned in the commit comment is important and how exactly it affects user code. Perhaps @rbrusu could elaborate on that...

[jspricke]

Afair the problem was really the "float<->integer RGB casts precision" as Radu wrote in the commit message. The best thing would be to save RGB as three int...

[stefanbuettner]

Hello there,

I just came across this issue, since some of my saved point clouds strangely contain huge amounts of red points. I dug around a bit and here's what I found out so far:

• Default values of alpha channel:
  • RGB: 0
  • PointXYZRGB: 0
  • PointXYZRGBA: 255
  • PointXYZRGBL: 0
  • PointXYZRGBNormal: 0
  • PointSurfel: 0
• The bit pattern of the integer RGBA field is sometimes interpreted as float for saving point clouds.
  Bit layout of the RGBA field:
  
  a(8) | r(8) | g(8) | b(8)
• The IEEE single precision bit layout is the following see page 8, Figure 1:
  
  sign(1) | exponent(8) | mantissa(23)
• NaNs are encoded by the exponent being all ones (255) and the mantissa being non-zero!

This leads to the problem that a whole bunch of values are being mapped to NaNs, namely fully opaque and half opaque values with red bigger than or equal to128 (0xff8..., 0x7f8...).
So I would suggest setting the alpha to 254 (0xfe) by default in every structure containing color information. This way, it's almost fully opaque and the point cloud should be stored correcty when using the float representation.

Cheers,
Stefan

PS.: Here are some tests on the issue:

#include <iostream>
#include <pcl/point_cloud.h>
#include <pcl/point_types.h>
#include <pcl/io/pcd_io.h>

using namespace std;
using namespace pcl;

int main (int argc, char* argv[])
{
    // Use PointXYZRGB because all of its fields are written as floats by default by PCDWriter
    PointCloud<PointXYZRGB> cloud;
    cloud.resize(9);

    cloud.points[0].rgba = 0x00FF0000; // finite value
    cloud.points[1].rgba = 0x0000FF00; // finite value
    cloud.points[2].rgba = 0x000000FF; // finite value
    cloud.points[3].rgba = 0xFF800000; // 255, 128,   0,   0 → -inf
    cloud.points[4].rgba = 0xFF80F0F0; // 255, 128, 240, 240 →  nan
    cloud.points[5].rgba = 0xFFC00000; // 255, 192,   0,   0 →  nan
    cloud.points[6].rgba = 0xFFC0F0F0; // 255, 192, 240, 240 →  nan
    cloud.points[7].rgba = 0x7F800000; // 127, 128,   0,   0 → +inf
    cloud.points[8].rgba = 0x7F80F0F0; // 127, 128, 240, 240 →  nan

    PCDWriter writer;
    writer.writeASCII("xyzrgb.pcd", cloud);
    return 0;
}

The saved xyzrgb.pcd file:

# .PCD v0.7 - Point Cloud Data file format
VERSION 0.7
FIELDS x y z rgb
SIZE 4 4 4 4
TYPE F F F F
COUNT 1 1 1 1
WIDTH 9
HEIGHT 1
VIEWPOINT 0 0 0 1 0 0 0
POINTS 9
DATA ascii
0 0 0 2.3418052e-38
0 0 0 9.1476764e-41
0 0 0 3.5733111e-43
0 0 0 -inf
0 0 0 nan
0 0 0 nan
0 0 0 nan
0 0 0 inf
0 0 0 nan

[stefanbuettner]

Oooor, one could initialize the alpha with 255 in every class/struct and write RGB(A) only data as uint32_t such that newly written pcd-files don't suffer the information loss and the precision issue.

The feature of reading RGB(A) data as floats should be kept for backward compatibility.

What do you think?

Cheers,
Stefan

[taketwo]

Stefan, nice analysis. I prefer the first option, it's a bit hacky, but elegant.
@jspricke What's your opinion?

[taketwo]

@jspricke ... and here. Thanks!

[jspricke]

I would love to see the second option implemented. @stefanbuettner if you have time to do that, it would be really great. Otherwise, I'm ok with the first option as well. @VictorLamoine do you want to update this pull request, or should we do it in a new one?

[VictorLamoine]

Stefan should open a new pull request: it's his work, not mine!

[stefanbuettner]

Okay, I'll try.