Make RGB(::Integer,::Integer,::Integer) make sense.

[lucasb-eyer]

Is there anything I'm missing, or does

RGB(r::Integer, g::Integer, b::Integer) = RGB{Float64}(r, g, b)

not make any sense because, according to the comments in RGB, the r, g and b values are within [0,1] and thus this line only permits to construct fully saturated colors?

This PR at least makes RGB(0xa4,0x39,0x1f) do what one'd expect, though the result of RGB(164,57,31) would still be unexpected. We could just always divide by 255 to make the latter one work, but that'd make handling 48 or 96 bit colors wrong. I'm in favor of 255 for the "least surprise."

[timholy]

The intention is that, for RGB, 1 always means saturated. For your example of RGB(164,57,31), dividing by 255 would be rather problematic if the values actually came from a 16-bit camera.

You should use Ufixed8 numbers, not Uint8. For example, RGB(0xa4uf8, 0x39uf8, 0x1fuf8).

[timholy]

(You need to say using FixedPointNumbers for that example to work.)

[timholy]

Note also that RGB(164uf8,57uf8,31uf8) works just fine:

julia> 255uf8
Ufixed8(1.0)

If you need to convert a whole array of Uint8s, use reinterpret.

[lucasb-eyer]

For your example of RGB(164,57,31), dividing by 255 would be rather problematic if the values actually came from a 16-bit camera.

Right, that's why I divided by typemax in the PR since r, g and b should be within [0,1], shouldn't they? Otherwise, RGB constructors with any "dumb" integer type are essentially useless, except for building saturated colors or the neat Ufixed trick. But if that's really what you want, it should be prominently documented since it goes against all of (my) prior experience. And now it dawns upon me, I've just been bitten by what I predicted :smile:

The trickyness gets worse because

julia> RGB(255uf8, 100uf8 + 155, 255uf8)
RGB{Float64}(1.0,155.3921568627451,1.0)

Imagine a loop-index being used to generate colors.

Note that I'm not advocating for my PR anymore, but against too generic/useless RGB constructors.

[timholy]

It will be documented, I just haven't had time to do it yet. Between documenting it here and in Images, and updating Images to work with the new color, it's at least half a day's work (maybe more). Got other fires burning brighter, unfortunately.

The idea is to get away from thinking about pixels as having integer-valued intensities. Pretty soon most Images will be Ufixed types and I predict you'll quickly get used to writing RGB(c.r, c.g+0.6, c.b). I don't see why thinking about an intensity as 155 is any more natural than thinking about it as 0.6 (the latter actually seems clearer to me).

[lucasb-eyer]

Ok, I partly forgot that we're in the middle of a big change, sorry for the noise!

[timholy]

Not a problem, I fully understand it's pretty awkward. Consider checking out a v0.2 version of Color until I have a chance to update Images.