search

JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
78 stars 35 forks source link

Promotion rules for same base color pairs #129

Closed kimikage closed 4 years ago

kimikage commented 4 years ago

As https://github.com/JuliaGraphics/Colors.jl/issues/273 shows, there are the problems with promotion rules for same base color pairs. Here is another simple example:

julia> promote_type(HSV{Float64}, HSV{Float32})
HSV

julia> promote(HSV{Float64}(0,0,0), HSV{Float32}(0,0,0))
ERROR: promotion of types HSV{Float64} and HSV{Float32} failed to change any arguments
Stacktrace:
 [1] error(::String, ::String, ::String) at .\error.jl:42
 [2] sametype_error(::Tuple{HSV{Float64},HSV{Float32}}) at .\promotion.jl:308
 [3] not_sametype(::Tuple{HSV{Float64},HSV{Float32}}, ::Tuple{HSV{Float64},HSV{Float32}}) at .\promotion.jl:302
 [4] promote(::HSV{Float64}, ::HSV{Float32}) at .\promotion.jl:285
 [5] top-level scope at REPL[2]:1

A simple (and tactless) solution is adding the rules here.

    @eval Base.promote_rule(::Type{$Csym{T1}}, ::Type{$Csym{T2}}) where {T1, T2} = $Csym{promote_type(T1, T2)}
    @eval Base.promote_rule(::Type{$acol{T1}}, ::Type{$acol{T2}}) where {T1, T2} = $acol{promote_type(T1, T2)}
    @eval Base.promote_rule(::Type{$cola{T1}}, ::Type{$cola{T2}}) where {T1, T2} = $cola{promote_type(T1, T2)}

Of course,

RGB1 and RGB4 require special handling because of the alphadummy field

However, I don't know what should be returned by promote_type(HSV, HSV{Float16}).(HSV{Float16}? or HSV{promote_type(eltype_default(HSV),Float16)}?)

kimikage commented 4 years ago

Recently, I am thinking that I should leave it to the compiler optimization and write if statements without @generated.

kimikage commented 4 years ago

The promotion rules introduced in PR #147 will not see the light of day.:sweat_smile:

Edit: the codes are stale. ```julia function promote_eltype(::Type{C1}, ::Type{C2}) where {C1<:Colorant, C2<:Colorant} T1, T2 = eltype(C1), eltype(C2) isconcretetype(T1) && isconcretetype(T2) && return promote_type(T1, T2) if isconcretetype(T1) return issupported(C2, T1) ? T1 : promote_type(eltype_default(C2), T1) elseif isconcretetype(T2) return issupported(C1, T2) ? T2 : promote_type(eltype_default(C1), T2) end Any end _with_et(C::UnionAll, et) = et === Any ? C : C{et} function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:Colorant, C2<:Colorant} et = promote_eltype(C1, C2) Cbase1, Cbase2 = base_colorant_type(C1), base_colorant_type(C2) Cbase1 === Cbase2 && return _with_et(Cbase1, et) C1 === RGB24 && C2 === Gray24 && return RGB24 C2 === RGB24 && C1 === Gray24 && return RGB24 C1 === ARGB32 && C2 === AGray32 && return ARGB32 C2 === ARGB32 && C1 === AGray32 && return ARGB32 C1 === ARGB32 && C2 <: AbstractARGB && return ARGB{et} C2 === ARGB32 && C1 <: AbstractARGB && return ARGB{et} C1 === AGray32 && C2 <: AbstractAGray && return AGray{et} C2 === AGray32 && C1 <: AbstractAGray && return AGray{et} C1 <: AbstractGray && return C2 === RGB24 ? RGB{et} : _with_et(Cbase2, et) C2 <: AbstractGray && return C1 === RGB24 ? RGB{et} : _with_et(Cbase1, et) C1 <: TransparentGray && return C2 === ARGB32 ? ARGB{et} : _with_et(Cbase2, et) C2 <: TransparentGray && return C1 === ARGB32 ? ARGB{et} : _with_et(Cbase1, et) ccolor(C1, C2) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:TransparentColor, C2<:Color} et = promote_eltype(C1, C2) Cbase1, Cbase2 = base_color_type(C1), base_color_type(C2) Cbase1 === Cbase2 && return _with_et(base_colorant_type(C1), et) # != Cbase1 C1 === ARGB32 && C2 <: AbstractGray && return C2 === Gray24 ? ARGB32 : ARGB{et} C2 === ARGB32 && C1 <: AbstractGray && return C1 === Gray24 ? ARGB32 : ARGB{et} C1 === AGray32 && C2 <: AbstractRGB && return C2 === RGB24 ? ARGB32 : ARGB{et} C2 === AGray32 && C1 <: AbstractRGB && return C1 === RGB24 ? ARGB32 : ARGB{et} C1 <: AbstractAGray && return C2 === RGB24 ? ARGB{et} : _with_et(alphacolor(Cbase2), et) C1 <: AbstractGrayA && return C2 === RGB24 ? RGBA{et} : _with_et(coloralpha(Cbase2), et) C2 <: AbstractGray && return _with_et(base_colorant_type(C1), et) # != Cbase1 ccolor(C1, C2) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:AbstractRGB, C2<:AbstractRGB} et = promote_eltype(C1, C2) Cbase1, Cbase2 = base_color_type(C1), base_color_type(C2) Cbase1 === Cbase2 && return _with_et(Cbase1, et) C1 <: RGBX && !(C2 <: XRGB) && return _with_et(RGBX, et) C1 <: XRGB && !(C2 <: RGBX) && return _with_et(XRGB, et) C2 <: RGBX && !(C1 <: XRGB) && return _with_et(RGBX, et) C2 <: XRGB && !(C1 <: RGBX) && return _with_et(XRGB, et) _with_et(RGB, et) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:AbstractGray, C2<:AbstractGray} et = promote_eltype(C1, C2) Cbase1, Cbase2 = base_color_type(C1), base_color_type(C2) Cbase1 === Cbase2 && return _with_et(Cbase1, et) _with_et(Gray, et) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:TransparentRGB, C2<:AbstractRGB} et = promote_eltype(C1, C2) C1 === ARGB32 && C2 === RGB24 && return ARGB32 C1 === ARGB32 && return _with_et(ARGB, et) _with_et(base_colorant_type(C1), et) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:TransparentGray, C2<:AbstractGray} et = promote_eltype(C1, C2) C1 === AGray32 && C2 === Gray24 && return AGray32 C1 === AGray32 && return _with_et(AGray, et) _with_et(base_colorant_type(C1), et) end function Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:Color, C2<:TransparentColor} promote_rule(C2, C1) end ```

We need additional ~rules and~ tests for non-concrete types. ~The promotion rules above depend on the strange Any(#151), so I will change the code to avoid the Any.~ Done

kimikage commented 4 years ago

Oops! :scream: https://github.com/JuliaGraphics/ColorTypes.jl/blob/5b2e50d62b8fd47a860c2e0543446144d8627c87/test/conversions.jl#L102-L107

kimikage commented 4 years ago

However, my concern is that even though promote_rule and ccolor have almost the same functionality, the codes are different now. The difference between the two is that ccolor has the order of priority between its arguments.

timholy commented 4 years ago

promote_rule and ccolor really are different from one another: promotion is about "type-widening," i.e., common up with a type that can store both values. ccolor is closer to "type-narrowing," e.g., coming up with a more specific type from general guidance.

If you had to pick one thing that promote is for, it's deciding the eltype of containers, e.g., in concatenation.

kimikage commented 4 years ago

That's right. However, promotion rules don't have to match the type hierarchy. In the ColorTypes hierarchy, we can too easily get Any. That's reasonable, but not really useful. If we want to get a purely common type, we should use typeintersect.(Edit: Sorry, as pointed out below, it actually mean typejoin.) However, at the moment, only limited promotions are working, so the useless rules may be fine.

timholy commented 4 years ago

typejoin is closer to what we want than typeintersect, but you're right that what I wrote earlier is more appropriate for typejoin than promote_type. (And indeed that's what it already gives. We should not specialize typejoin since that's a generic reflection tool.)

If there's a type that includes the gamut of most/all other types (XYZ?), we could promote to that. A little loss in precision is acceptable, just as with promote_type(Int, Float64) === Float64.

kimikage commented 4 years ago

I think it's more convenient to allow the user to specify (i.e. override) the rules for concrete types such as RGB{T} vs HSV{T}.

I will change the unclear rules to the rules purely based on the type hierarchy.

timholy commented 4 years ago

Yes, I do think typejoin-like behavior is a reasonable fallback.

kimikage commented 4 years ago

To be honest, I do not have a clear answer on what the promotion rules for non-concrete types should be. promote_type(AbstractRGB{Float32}, RGB24) returns RGB{Float32} for now, but is that OK? (cf. https://github.com/JuliaGraphics/ColorTypes.jl/pull/161#discussion_r378930542) I believe that RGB{Float32} is more pragmatic than AbstractRGB{Float32}. However I doubt that this is the exceptional rule allowed only for AbstractRGB and AbstractGray. In particular, AbstractGray is a typealias, and its substance is Color.

timholy commented 4 years ago

To be honest, I do not have a clear answer on what the promotion rules for non-concrete types should be.

Yes, this is difficult terrain. I think the best guidance is to think about allocating a container of concrete eltype that could store objects of all the specified types reasonably losslessly (whatever that means :man_shrugging: ). If that requires conversion, that's OK.

promote_type(AbstractRGB{Float32}, RGB24) returns RGB{Float32}

Yes, that seems clearly correct.

I think I'd be pretty happy with the following rules (but I doubt they are sufficient), which I do think you've basically implemented:

Colors:

Transparency:

One thing I've probably never mentioned: part of the design of this package was to support custom color spaces (it's why a lot of the constructor-generators are macros). To give a concrete example of where that's useful, a lot of biological imaging is done with two color channels, for example Green Fluorescent Protein (see spectrum) and tdTomato. If you've collected data from both color channels over the same spatial region, ideally the resulting array would have eltype GFPtdTomato{Float32} or something, where

struct GFPtdTomato{T} <: Color{T,2}
    gfp::T
    tdTomato::T
end

That, of course, introduces a whole new class of headaches wrt promotion rules. You don't have to handle that, but that's part of why I said "3 or fewer color channels."

kimikage commented 4 years ago

Oh, I changed the rules to the typejoin-like before you posted the comment above.

julia> using ColorTypes

julia> promote_type(RGB, RGB{Float16})
RGB

julia> promote_type(HSV, HSV{Float16})
HSV

julia> promote_type(AbstractRGB, RGB{Float16})
RGB

julia> promote_type(AbstractRGB{Float32}, RGB{Float16})
RGB{Float32}

if one is <:AbstractGray and the other <:AbstractRGB promote to whatever RGB-type the one has (if it's concrete); if it's just plain AbstractRGB then pick RGB.

In addition, at present (i.e. Colors.jl v0.11 + ColorTypes.jl v0.9), it does not matter what the gray opponent is.

julia> promote_type(HSV{Float32},Gray{Float64})
HSV{Float64}

Of course, there is a difference that RGB-type has AbstractRGB and HSV-type does not have AbstractHSV.

julia> promote_type(Color3{Float32},Gray{Float64}) # Colors v0.11 + ColorTypes.jl v0.9
Color{Float64,N} where N
julia> promote_type(Color3{Float32},Gray{Float64}) # PR #164
Color{Float64,3} where 3
kimikage commented 4 years ago

otherwise, anything with 3 or fewer color channels promotes to XYZ{T}

This is a rule I (we) have not implemented. I agree with this rule if we want to force promotion to concrete types, but if not, I don't really like the rule.

Although ColorTypes.jl is not involved in the color space conversions, Colors.jl actually converts the color spaces as follows. (cf. https://github.com/JuliaGraphics/Colors.jl/issues/354#issuecomment-534547652) 65509304-22078d80-df0d-11e9-8b98-66cbaca3c740 Except for the direct conversion systems using lookup tables, the most conversion flow draws such a graph (with some little differences). The important point is that in addition to XYZ, there are three hubs: RGB, Lab, and Luv.

For example, it is good for consistency that the promoted type of Lab and LCHab is XYZ, but it doesn't seem really practical. However, the semantic relationship between Lab and LCHab is clear, but the semantic relationship between HSV and RGB is slightly complicated (https://en.wikipedia.org/wiki/HSL_and_HSV#/media/File:HSL-HSV_hue_and_chroma.svg). I don't think it is really good to go into such a detail of color spaces.

Another concern for me is that XYZ is an absolute color space, but other color spaces may be relative. The sRGB is an absolute color space, and there is no ambiguity in conversion to XYZ. Although README of ColorTypes.jl describes the RGB means the sRGB, the rules are not strictly followed in actual image processing. I also think that sRGB is "too narrow" for some applications.

On the other hand, even though Lab (CIELAB) is an absolute color space, it depends on the light source or "white point", so it is somewhat relative.

Of course, we can use the default context (e.g. gamut or white point), but the users cannot control the context within Colors.jl.

kimikage commented 4 years ago

Based on the issues above, my strategy was to hide the rules except for trivial rules for RGB24 and RGB32 and so on.

Base.promote_rule(::Type{C1}, ::Type{C2}) where {C1<:Colorant, C2<:Colorant} = _promote_rule(C1, C2)

This makes it difficult for users to the directly intervene in the rules, but this allows to easily override the rules. (Of course, being able to write rules easily is quite different from that the rules are easy, though. :innocent:)

kimikage commented 4 years ago

prefer AlphaColor otherwise

I did not agree to this rule because AlphaColor has no legitimacy to take precedence over ColorAlpha. In fact, CSS uses RGBA instead of ARGB. Therefore, the rules implemented in PR #164 promote AlphaColor and ColorAlpha to TransparentColor.

However, these generalizations (i.e. uplifting in the type hierarchy) make the rules too complicated, mainly due to the irregular rules related to AbstractRGB.

TBH, until two days ago, I believed that the rules could be implemented with ad-hoc and ugly codes. However, when I simulated adding the premultiplied alpha types for issue #167, I realized that the ad-hoc codes have no scalability. To be precise, I knew they were not scalable, but I thought that the only possible change in the type hierarchy was the addition of concrete color types (e.g. HWB).

Thus, I realized the practical need to bias either AlphaColor or ColorAlpha. I'm writing the rules which prioritizes AlphaColor. However, as mentioned above, I'm going not to include the promotions to XYZ in the rules. The promotions to Color/ColorAlpha/AlphaColor do not complicate the rules so much, since it concerns the lowest layer of the hierarchy.