Yarp: PointCloudUtils does not compile

Created on 6 Feb 2019 · 11Comments · Source: robotology/yarp

Describe the bug
Calling yarp::sig::utils::depthRgbToPC with T2 class type differentr from either yarp::sig::PixelRgba or yarp::sig::PixelBgra, for example

yarp::sig::DataXYZRGBA pointcloud = yarp::sig::utils::depthRgbToPC<yarp::sig::DataXYZRGBA, yarp::sig::PixelRgb>(img_depth, img_color, camera_intrinsics);

gives the following error

error C2039: 'a': is not a member of 'yarp::sig::PixelRgb'

Expected behavior
Visual Studio compiler is not able to optimize, i.e. remove with branch hints, the following if statement
https://github.com/robotology/yarp/blob/ec09c81fa2558a7f75a23b4a8e706121c18823eb/src/libYARP_sig/include/yarp/sig/PointCloudUtils-inl.h#L46-L49
while other compilers do (gcc, clang).

Configuration (please complete the following information):

OS: Windows 10
yarp version: 3.1.100+20181113.21+gitd93264183
- compiler: cl toolchain v141

cc @Nicogene

YARP v3.1.0 Library - YARP_sig YARP v3.4.1 Bug Fixed

Source

claudiofantacci

All 11 comments

@S-Dafarra was describing a simiilar problem during lunch yesterday.

traversaro on 6 Feb 2019

👍1

The only solution for now is to either copy-paste the content of PointCloudUtils-inl.h (that is visible to users) and customize/specialize it for the particular cases Windows does not work properly.
This may also be a possible solution on YARP side.

Unfortunately if constexpr is a C++17 feature, otherwise we would already have a solution.

claudiofantacci on 6 Feb 2019

I think this is a slightly different issue. What I experienced was that Visual Studio was failing to substitute the following template (https://github.com/S-Dafarra/levi/blob/e5ad65b5a6/include/levi/OperatorsEvaluables.h#L76-L77):

template<typename Scalar_lhs, int lhsRows, int lhsCols, typename Scalar_rhs, int rhsRows, int rhsCols>
struct levi::matrix_product_return<Eigen::Matrix<Scalar_lhs, lhsRows, lhsCols>, Eigen::Matrix<Scalar_rhs, rhsRows, rhsCols>

The problem was that Eigen::Matrix has some additional template parameters which can be defaulted. When trying to call matrix_product_return<Matrix1, Matrix2> Visual Studio was assuming that all the parameters I was not setting should have been all equal (I actually don't know what the standard says in this regard). The fix was to add also those parameters in the template specialization.

Unfortunately if constexpr is a C++17 feature, otherwise we would already have a solution.

Once I saw an answer on stackoverflow that I am not able to find, but it worked basically in this way.

Define a struct templated with a bool

template<bool value>
struct bool_value{}

Define a function which takes bool_value as a parameter. By specializing the true and false case you have the same effect of the if

template<bool value>
void copyA(bool_value<value> /*, other parameters*/);

void copyA(bool_value<true> /*, other parameters*/) {
// True case
}

void copyA(bool_value<value> /*, other parameters*/) {
// False case
}

Use the function

copyA(bool_value</*condition to check*/>(), /*other parameters*/)

S-Dafarra on 6 Feb 2019

👍1

@S-Dafarra I agreee that they are two different problems.

What you suggest is useful, but will lead to two different implementation of a function and in the end we could just implement a template specialization for PixelRgb types (without the alpha channel).

claudiofantacci on 6 Feb 2019

👍1

We analyzed with godbolt the compilation result of this code snippet:

// Type your code here, or load an example.
int square(int num) {
    int pippo = 2;
    if(false) {
        pippo=5;
    }
    return num * num;
}

And the code inside the if is removed by clang and gcc by default, on the other hand msvc removes it only with the compiler optimization /Ox.