Set default alpha value to 255

[stefanbuettner]

Hey there,
this is my suggestion to issue #1141. Some tests are failing compared to the master branch these commits are based on:

• PCL.GroundBasedPeopleDetectionApp
• PCL.CopyIfFieldExists
• PCL.PLYPolygonMeshColoredIO (SegFault)

At first I looked at CopyIfFieldExists where the expected value has to change. But the value I was expecting based on my calculations still was not the result of the test. For the other two, I don't have any idea. So perhaps you guys do.

Cheers,
Stefan

[taketwo]

In the unit test, CopyIfFieldExists functor is instantiated with the assumption that "rgb" field has type float:

CopyIfFieldExists<PointXYZRGBNormal, float> (p, "rgb", is_rgb, rgb_val)

Then when the functor is applied in the for_each_type loop, the following operation is carried out (lines 261 through 267):

if (name_ == pcl::traits::name<PointInT, Key>::value)
{
  exists_ = true;
  typedef typename pcl::traits::datatype<PointInT, Key>::type T;
  const uint8_t* data_ptr = reinterpret_cast<const uint8_t*>(&pt_) + pcl::traits::offset<PointInT, Key>::value;
  value_ = static_cast<OutT> (*reinterpret_cast<const T*>(data_ptr));
}

Here OutT is float and T is uint32_t (after your change). Therefore *reinterpret_cast<const T*>(data_ptr) is an unsigned integer, which is then cast to float using normal casting rules (instead of reinterpretation), which produces nonsense results. Note that before your change both types were float and therefore this worked as expected.

I think we can not apply your change, because it may affect existing user code the same way it affects unit tests. Your "second option" proposed in the previous discussion was only about changing IO functions, guess we should go that way.

[VictorLamoine]

There are still some tests failing, can you fix them?

12: /home/travis/build/PointCloudLibrary/pcl/test/common/test_common.cpp:416: Failure
12: Value of: 0xffef40fe
12:   Actual: 4293869822

12: Expected: rgb
12: Which is: -8437506

12: [  FAILED  ] 1 test, listed below:
12: [  FAILED  ] PCL.CopyIfFieldExists
12:  1 FAILED TEST
 12/101 Test  #12: common_common ..........................***Failed    0.01 sec
test 13
        Start  13: common_copy_point

#1040 should be fixed after this pull request is merged.

[VictorLamoine]

We're good, aren't we?

[stefanbuettner]

I didn't manage to check what is failing in the test Hough3DGrouping but if it's not related to the error, we're good.

[SergioRAgostinho]

It shouldn't be. That test is failing for the majority (if not all) of the (current) PRs.

[SergioRAgostinho]

Squash?

[VictorLamoine]

I think we are good to merge this one and close #1141.