Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Distinguishing between AbstractGray{T} and Color{T, 1} #230

Closed
kimikage opened this issue Mar 4, 2021 · 2 comments · Fixed by #252
Closed

[RFC] Distinguishing between AbstractGray{T} and Color{T, 1} #230

kimikage opened this issue Mar 4, 2021 · 2 comments · Fixed by #252
Milestone

Comments

@kimikage
Copy link
Collaborator

kimikage commented Mar 4, 2021

Currently, AbstractGray{T} is the alias for Color{T, 1}, but this is a bit weird. (cf. #145 (comment))

Here is an extreme example.

struct Lightness{T} <: Color{T, 1}
     v::T
end

struct Darkness{T} <: Color{T, 1}
     v::T
end

ColorTypes.gray(c::Lightness) = c.v
ColorTypes.gray(c::Darkness) = oneunit(c.v) - c.v
ColorTypes.comp1(c::Darkness) = c.v
julia> Darkness(0.2) == Lightness(0.8) # OK
true

julia> Darkness(0.2) > Lightness(0.3) # Uh, yes, that's correct.
true

julia> Darkness(0.2) == 0.8 # What? Oh, hmm.
true 

Colored tones such as sepia and cyanotype can be prevented from strange reinterpretations by not implementing gray(). However, when you implement gray() for a "gray" scale, strange things happen.

That is, color types which have difference between comp1 and gray are inherently incompatible with AbstractGray.

IMO, AbstractGray should be an abstract type as well as AbsractRGB.

@kimikage
Copy link
Collaborator Author

kimikage commented Mar 11, 2021

This is an experimental type (it does not conform to any specific standard).

struct Cyanotype{T <: Real} <: Color{T,1}
   value::T
end
function Base.convert(::Type{Cout}, c::C) where {Cout <: AbstractRGB, T, C <: Cyanotype{T}}
    r = max(1 - 1.5 * c.value, 0)
    g = 0.98 - 0.7 * c.value
    b = 0.88 - 0.5 * c.value^2
    Cout(T(r), T(g), T(b))
end

It would be a step forward if ColorTypes.jl and its downstream packages did not misidentify this as a "gray" type. And that's technically easy to do.

cyano

@kimikage
Copy link
Collaborator Author

The following are obviously not equal, as they belong to different color spaces.

Cyanotype{Float32}(0.5) == Gray{Float32}(0.5) # -> false

We treat gray as a real number.

0.5 == Gray{Float32}(0.5) # -> true

However, it is debatable whether the following are equal. The following equality relation itself is reasonable, but by accepting it, the transitivity is no longer satisfied.

Cyanotype{Float32}(0.5) == 0.5 # -> false?

If we don't allow comparisons with real numbers, then defining real is also debatable.
However, since real is called explicitly, the conversion to real numbers seems to be fine.

real(Cyanotype{Float32}(0.5)) # -> 0.5f0?

More problematic is the convert. If we support conversion to real numbers, something strange should happen with the default constructor for color types.

convert(Real, Cyanotype{Float32}(0.5)) # -> MethodError?

IMO, only AbstractGray should support "inter"-conversion with real numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant