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

[meyerjo]

Description

This adds the ability to load the color information from pcd files which do not specify an "rgb" field but the color information split up to the fields "red", "green", "blue".
I tested it with a pcd files converted from a las-file via pdal with compression: binary, ascii.

[Mugen87]

I've always struggled with such enhancements for PCDLoader because the file format lacks a real specification document. The only one that we have used so far is the following which is also referenced by MathWorks/MATLAB:

https://pointclouds.org/documentation/tutorials/pcd_file_format.html

According to this document, separate RGB fields are not supported.

Considering this document, it's fair to ask why a PCD exporter has to report separate RGB fields if the exporter devs should know this approach is uncommon for PCD. Design decisions like that only make compatibility between different software harder. Unless there is a very good reason for having separate "red", "green", "blue" fields, I'm not willing to vote for this change.

[mrdoob]

@meyerjo Could you explain where these values come from and what's the PCD ecosystem support for them?

[meyerjo]

I find the whole situation around the PCD file definition a bit messy. I read the definition of the file format header as "Examples:" but you could always add new columns to it.

In my case, I have data which is originally stored in a las file which contains many additional information such as the intensity, number of returns etc and the color specified in three different fields red, green, blue (format definitions in: https://www.asprs.org/wp-content/uploads/2010/12/asprs_las_format_v12.pdf see i.e. page 10).

When processing and converting them to pcd using libraries such as pdal pcd-headers such as

VERSION 0.7
FIELDS x y z intensity returnnumber numberofreturns scandirectionflag edgeofflightline classification scananglerank userdata pointsourceid red green blue
SIZE 4 4 4 8 8 8 8 8 8 8 8 8 8 8 8
TYPE F F F F F F F F F F F F F F F
COUNT 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
WIDTH 731114
HEIGHT 1
VIEWPOINT 0.0 0.0 0.0 1.0 0.0 0.0 0.0
POINTS 731114
DATA ascii

are generated. However, it is not possible at the moment to read these color information into your nice library. Thus, I decided to add this behavior to the PCDLoader and contribute it back.

[Mugen87]

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.

[meyerjo]

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.

[Mugen87]

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.

[Mugen87]

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.

[WestLangley]

When processing and converting them to pcd using libraries such as pdal

Some pdal links that discuss the additional fields (referred to as "dimensions"):

https://pdal.io/en/2.7.1/about.html
https://pdal.io/en/2.7.1/dimensions.html#dimensions
https://pdal.io/en/2.7.1/stages/writers.pcd.html#writers-pcd

[Mugen87]

Closing,see #28179 (comment).