1

Assuming I have a 4 value vector union, used to hold either a spatial coordinate or colour and I wish to use one of two functions to convert between integer and real format, how would I construct the functions?

My (failed) attempt is:

    template<class T, 
             class S, 
             typename = std::enable_if_t<std::is_floating_point<T>>,
             typename = std::enable_if_t<std::is_integral<S>>>
    Math::Vec4<T> Colour(S r, S g, S b, S a)
    {
        return Vec4<T>(r / static_cast<T>(255), 
                       g / static_cast<T>(255), 
                       b / static_cast<T>(255), 
                       a / static_cast<T>(255));
    };

    template<class T, 
             class S, 
             typename = std::enable_if_t<std::is_integral<T>>,
             typename = std::enable_if_t<std::is_floating_point<S>>>
    Math::Vec4<T> Colour(S r, S g, S b, S a)
    {
        return Vec4<T>(static_cast<T>(r * 255), 
                       static_cast<T>(g * 255), 
                       static_cast<T>(b * 255), 
                       static_cast<T>(a * 255));
    };

The intention here is to instantiate 1 for cases where T is real and S is integer (convert from integer to real) and 2 for cases where T is integer and S is real. Ideally I'd like to use the same name for both and for the compiler to decide which to use based on the input type. At the moment I get a compiler error, "function template has already been defined".

Also, is this a bad idea?

Update: I've produced a minimal repro based on Tartan's answer, that doesn't compile (can't deduce template argument). Where did I err?

#include <limits>
#include <type_traits>

template<class T>       
union Vec3
{ 
    typedef T value_type;

    struct 
    {                   
        T r, g, b;
    };

    Vec3() = default;
    Vec3(T R, T G, T B) : r(R), g(G), b(B) {}
};

template<class T,
         class S,
         std::enable_if_t<std::is_floating_point<T>::value && std::is_integral<S>::value> * = nullptr>
Vec3<T> Colour(Vec3<S> const & rgb)
{
    return Vec3<T>(static_cast<T>(rgb.r) / static_cast<T>(std::numeric_limits<S::value_type>::max()),
                   static_cast<T>(rgb.g) / static_cast<T>(std::numeric_limits<S::value_type>::max()),
                   static_cast<T>(rgb.b) / static_cast<T>(std::numeric_limits<S::value_type>::max()));
}

template<class T,
         class S,
         std::enable_if_t<std::is_integral<T>::value && std::is_floating_point<S>::value> * = nullptr>
Vec3<T> Colour(Vec3<S> const & rgb)
{
    return Vec3<T>(static_cast<T>(rgb.r * static_cast<S::value_type>(std::numeric_limits<T>::max())),
                   static_cast<T>(rgb.g * static_cast<S::value_type>(std::numeric_limits<T>::max())),
                   static_cast<T>(rgb.b * static_cast<S::value_type>(std::numeric_limits<T>::max())));
}

int main(void)
{
    Vec3<float> a(1.0f, 0.5f, 0.25f);
    Vec3<char> b;

    b = Colour(a);
}
Robinson
  • 9,666
  • 16
  • 71
  • 115

2 Answers2

4

Your issue is that differences in default template arguments don't declare distinct templates. That is why you are getting the redefinition error.

An additional issue is that you need to take the ::value of the std::is_integral and std::is_floating_point traits, as std::enable_if_t expects a bool, not a std::integral_constant.

To fix this, you can use a standard SFINAE trick to declare a template argument of a type which depends on the enable_if_t result:

template<class T, 
         class S, 
         std::enable_if_t<
              std::is_floating_point<T>::value && std::is_integral<S>::value
         >* = nullptr>
Math::Vec4<T> Colour(S r, S g, S b, S a)
{
    return Vec4<T>(r / static_cast<T>(255), 
                   g / static_cast<T>(255), 
                   b / static_cast<T>(255), 
                   a / static_cast<T>(255));
};

template<class T, 
         class S, 
         std::enable_if_t<
              std::is_integral<T>::value && std::is_floating_point<S>::value
         >* = nullptr>
Math::Vec4<T> Colour(S r, S g, S b, S a)
{
    return Vec4<T>(static_cast<T>(r * 255), 
                   static_cast<T>(g * 255), 
                   static_cast<T>(b * 255), 
                   static_cast<T>(a * 255));
};
TartanLlama
  • 63,752
  • 13
  • 157
  • 193
  • Well, I suppose I need to add SFINAE to the list of things to try to understand about templates. It seems to work however, so thank you. – Robinson Jun 17 '15 at 08:38
  • You could also look into tag-dispatching, which might be a bit prettier. YMMV. – TartanLlama Jun 17 '15 at 08:39
  • I think there is a `typename =` missing in your code. Further, as a suggestion, I would drop the `* = nullptr` part after `enable_if_t` -- I guess that has been copy-and-pasted :) – davidhigh Jun 17 '15 at 08:58
  • @davidhigh Nope, the whole point of this is to avoid the `typename =`, because then you are relying on default arguments and you have the redefinition error. The `* = nullptr` is there because a valid `std::enable_if_t` will collapse to `void`, so the overall expression will be `void* = nullptr`. – TartanLlama Jun 17 '15 at 09:00
  • ok, thanks, I see that for the first time. Can you please elaborate a bit on that? What does it mean to replace `typename` by `void*` (or more generally an arbitrary pointer) in the parameter list? – davidhigh Jun 17 '15 at 09:14
  • It's a way of sidestepping the redefinition error. If the `std::enable_if_t` construct is valid, we get a `void*` template argument with `nullptr` as a default value (so that the user doesn't need to supply one), and if the construct is invalid, we get a substitution failure. – TartanLlama Jun 17 '15 at 09:15
  • ok, that SFINAE mechanism basically clear to me, I just never saw pointers as non-type parameters. It seems to me that is more or less a shorthand for the alternative approach where a specialization of an integral or bool template parameter is done. I'd suggest you directly go [here](http://stackoverflow.com/questions/13220502/uses-of-pointers-non-type-template-parameters) and post another answer (where, in the best case, you show both mentioned alternatives). And then count on my upvote :) – davidhigh Jun 17 '15 at 09:25
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/80760/discussion-between-tartanllama-and-davidhigh). – TartanLlama Jun 17 '15 at 09:40
  • Can you look at the repro? I've updated my question as it doesn't compile. – Robinson Jun 17 '15 at 11:47
  • Where you say `S::value_type` you actually mean just `S`. Also, you can't just say `b = Color(a)` because `Color` doesn't know what type to return. You need to say `b = Color(a)`, or `b = Color(a);` – TartanLlama Jun 17 '15 at 12:37
  • Ah, you're quite right. It's working now. Wonderful. – Robinson Jun 17 '15 at 13:40
4

The answer of @TartanLlama presents the technical way to go. Here I give my opinion on your more fundamental question

Also, is this a bad idea?

Yes, I think so.

To me it seems your two overloads of Coulour are inverse to each other. Thus, you should express that and give them proper names.

Nobody would expect Colour(Colour(r,g,b,s)) to be the identity operation (such a function doesn't exists here but the interpretation is clear).

Something like colour_to_spatial/spatial_to_colour or colour/coulor_invert would be much more appropriate imo.

Next, there might be technical issues. The conversion from Integer to Floating-point back to Integer is in general not deterministic, as rounding errors might occur which lead to different results. So I'd suggest to be careful here and use proper rounding routines, or better, a fixed-width decimal type.

davidhigh
  • 14,652
  • 2
  • 44
  • 75
  • To add to these excellent points, I just realised I need to divide and multiply by std::numeric_limits::max in the first case, not 255. 255 works for unsigned char but not for char and makes no sense for integer. – Robinson Jun 17 '15 at 08:59