use macros for float conversions

[sidred]

Define three macros

macro_rules! flt {
    ($x: expr) => {
        T::from($x).unwrap()
    };
}

macro_rules! f_0 {
    () => (T::zero());
}

macro_rules! f_1 {
    () => (T::one());
}

The the equation become easier to read

x: T::from(X_N).unwrap() * f_inv((T::one() / T::from(116.0).unwrap()) *
    (lab.l * T::from(100.0).unwrap() + T::from(16.0).unwrap()) +
    (T::one() / T::from(500.0).unwrap()) * lab.a * T::from(128.0).unwrap()),
y: T::from(Y_N).unwrap() * f_inv((T::one() / T::from(116.0).unwrap()) *
    (lab.l * T::from(100.0).unwrap() + T::from(16.0).unwrap())),
z: T::from(Z_N).unwrap() * f_inv((T::one() / T::from(116.0).unwrap()) *
    (lab.l * T::from(100.0).unwrap() + T::from(16.0).unwrap()) -
    (T::one() / T::from(200.0).unwrap()) * lab.b * T::from(128.0).unwrap()),

x: rgb.red * T::from(0.4124).unwrap() + rgb.green * T::from(0.3576).unwrap() + rgb.blue * T::from(0.1805).unwrap(),
y: rgb.red * T::from(0.2126).unwrap() + rgb.green * T::from(0.7152).unwrap() + rgb.blue * T::from(0.0722).unwrap(),
z: rgb.red * T::from(0.0193).unwrap() + rgb.green * T::from(0.1192).unwrap() + rgb.blue * T::from(0.9505).unwrap(),

becomes

x: flt!(X_N) * f_inv((f_1!() / flt!(116.0)) *
    (lab.l * flt!(100.0) + flt!(16.0)) +
    (f_1!() / flt!(500.0)) * lab.a * flt!(128.0)),
y: flt!(Y_N) * f_inv((f_1!() / flt!(116.0)) *
    (lab.l * flt!(100.0) + flt!(16.0))),
z: flt!(Z_N) * f_inv((f_1!() / flt!(116.0)) *
    (lab.l * flt!(100.0) + flt!(16.0)) -
    (f_1!() / flt!(200.0)) * lab.b * flt!(128.0)),

x: rgb.red * flt!(0.4124) + rgb.green * flt!(0.3576) + rgb.blue * flt!(0.1805),
y: rgb.red * flt!(0.2126) + rgb.green * flt!(0.7152) + rgb.blue * flt!(0.0722),
z: rgb.red * flt!(0.0193) + rgb.green * flt!(0.1192) + rgb.blue * flt!(0.9505),

[Ogeon]

flt!(..) can be very useful. Have you tried implementing it as a function? Is the compiler smart enough to infer what T is in fn flt<T: NumCast, P: FromPrimitive>(prim: P) -> T?

The other two will just shave off a few characters, so I think we can live with T::one() and T::zero(). It's more self explanatory than f_1!() and f_0!().

[sidred]

What is the advantage of using a function instead of a macro?

[Ogeon]

It's more generic than an assumed type T and they are often easier to reason about, since they can't just do anything. Tools like Racer doesn't work with macros for that reason, but they may do it in the future.

[sidred]

This works

pub fn flt<T: num::Float, P: num::ToPrimitive>(prim: P) -> T {
    num::NumCast::from(prim).unwrap()
}

// use as flt(2.90)

• should we just use f instead of flt ?
• should T::one() and T::zero() be changed to flt(0.0) and flt(1.0)? Since the compiler will optimize this away, performance will not be impacted.

[Ogeon]

Cool! flt is a better clue to what it does. I would also prefer to keep T::one() and T::zero(), since they have an other level of guarantee and we don't have to make it more complicated for the compiler than necessary. 😄 They may also help type inference a bit more.

[sidred]

I'll make this change once we merge the yxy color. Otherwise there will be too many conflicts.

[Ogeon]

Sounds good. One thing at the time 😄

[sidred]

Using a function requires type annotations in some cases while the macro does not:

    pub fn new_u8(red: u8, green: u8, blue: u8) -> Rgb<T> {
        Rgb {
            red: flt<T,_>(red) / flt(255.0),
            green: flt<T,_>(green) / flt(255.0),
            blue: flt<T,_>(blue) / flt(255.0),
        }
    }

vs

    pub fn new_u8(red: u8, green: u8, blue: u8) -> Rgb<T> {
        Rgb {
            red: flt!(red) / flt!(255.0),
            green: flt!(green) / flt!(255.0),
            blue: flt!(blue) / flt!(255.0),
        }
    }

Do you have a preference for either?

[Ogeon]

That's not too bad, IMO. It's significantly less noise, compared to the unwraps. I wonder if this would work:

let c255: T = flt(255.0);
Rgb {
    red: flt(red) / c255,
    green: flt(green) / c255,
    blue: flt(blue) / c255,
}

...or if it still needs to know the left hand side fist. Float implies Copy, so I have moved some repeated constants to variables like c255 above, to both clean things up a little and to lower the amount of repeated conversions of the same number.

In any case, I'm still biased towards a function, since it's possible in a reasonable way. I prefer to only use macros where nothing else helps, such as repetitive parts and/or in cases like try!.

[sidred]

No, the first parameter of a formula (or start of open brace) needs to be of known type T.

I have both methods working, if you want to check the difference
Function: https://github.com/sidred/palette/tree/use_float_fn
Macro: https://github.com/sidred/palette/tree/use_float_macro

[Ogeon]

No, the first parameter of a formula (or start of open brace) needs to be of known type T.

I suspected as much, but I wanted to believe.

I have both methods working, if you want to check the difference

Nice! It looks like the function works fine in the critical parts, where the rest of the code is complicated enough as it is. My opinion is that it's completely acceptable. It may not be pretty, but it's at least isolated to the easy parts, as far as I can see, and the intent is clearer than with a macro.