Yarp: PontCloudTypes: Possible error in DataRGBA

Created on 19 Nov 2018  路  11Comments  路  Source: robotology/yarp

The order of the bytes in DataRGBA seems wrong (b, g, r, a).

https://github.com/robotology/yarp/blob/ff713f369ff3a00aafd64d8105597e203f58b65a/src/libYARP_sig/include/yarp/sig/PointCloudTypes.h#L483-L490

Is this a typo? If it is not, please add a comment to explain...

Library - YARP_sig Reminder - Documentation Needed

Most helpful comment

Actually I was wrong, being on little endian system, this is B G R _.

Yes, I think the definition leaverage on the little endiannes... at this point the shouldn't the int be Argb?
Otherwise I maybe tempted to do:
rgba = (r) << 24 | (g) << 16 | (b) <<8 | (a) ;
which is incorrect, right?

All 11 comments

CC @Nicogene

The point cloud types have been taken from PCL, see their documentation at line 224.
Sincerely I have no idea if they did with some rationale or it is a typo

Interesting observation. PCL defines PCL_ADD_UNION_RGB as a BGR union, then it defines RGB vectors reordering the data in the union. I guess that PCL_ADD_UNION_RGB could be simply renamed PCL_ADD_UNION_BGR. As long as there is an agreement on data alignment, our structure is good to go, isn't it?

By the way, I found a lot of this data structures in OpenCV, PCL, ROS, but I have never found a reference describing in detailed all of this.

As long as there is an agreement on data alignment, our structure is good to go, isn't it?

The problem that I see here is that if I access to r, g, b, a through the NetInt32 rgba, I could think that its first byte is the r and it is the b instead. Am I right?
Obviously I don't think that so many people use this struct as NetInt32 but it could cause issues

You are definitely right. I'm not sure what would be the proper way to tackle this. PCL defines RGB types as BGR, providing proper documentation here.
If we are using that structure to be a replica/compatibility layer of PCL, then I guess we need to leave the structure as it is.

It doesn't look like bgr to me, r is shifted by 16 and g is shifted by 8...

Actually I was wrong, being on little endian system, this is B G R _.

Actually I was wrong, being on little endian system, this is B G R _.

Yes, I think the definition leaverage on the little endiannes... at this point the shouldn't the int be Argb?
Otherwise I maybe tempted to do:
rgba = (r) << 24 | (g) << 16 | (b) <<8 | (a) ;
which is incorrect, right?

Either BGRA or ARGB, but for sure not RGBA

Closing this. The data is correct (the struct comes from PCL), the name is confusing.

Was this page helpful?
0 / 5 - 0 ratings