Is there a reason `AlphaColorValue` does not inherit `ColorValue`?
tomasaschan opened this issue · 5 comments
As the title states: is there a rationale behind the fact that the AlphaColorValue type doesn't inherit ColorValue?
If it did (and any required methods were implemented, of course) it would be trivial to e.g. get some opacity functionality in e.g. Gadfly just by using e.g. RGBA values instead of RGB.
If it's a non-trivial change, what more than just making AlphaColorValue inherit ColorValue would need to change? I know next to nothing about how these things are implemented, but if it makes sense to change this, and it's a relatively simple change, I'll gladly help out.
It would be easy to do, but I'd feel slightly uncomfortable about it. First, with the currently implemented color spaces you can convert any type T <: ColorValue to any other type U <: ColorValue. To keep that property, we'd have to define a convert function to convert AlphaColorValue{RGB} to RGB, which I guess would just drop the alpha value, which seems a little weird. It would also suddenly make AlphaColorValue{AlphaColorValue{RGB}} a valid type, despite not making any sense.
I wonder if it would make more sense to define some new supertype of ColorValue and AlphaColorvalue, or maybe just rename ColorValue BasicColorValue and have ColorValue be an abstract supertype of AlphaColorValue and BasicColorValue.
Yeah - an abstract supertype of both AlphaColorValue and ColorValue (in its current meaning) probably makes more sense.
If this change is made, will users of ColorValue need to change to take the AlphaColorValue possibility into account? For example, I imagine the code in Compose (and perhaps packages like Cairo?) that draws stuff, will it be able to correctly handle the alpha stuff just like that? If not, maybe this change needs some more coordinated effort? What can I do to help out?
I think it depends on whether we change the meaning of ColorValue or keep it the same and name the supertype something new. If we change ColorValue, probably a bunch of stuff will break, so we'd need to warn people using Color and be very careful about it. If we just introduce a supertype with a new name, it should be pretty painless.
So, we should try to first try to think of a good alternate name for the supertype, so we can avoid changing the meaning of ColorValue if possible.
Wouldn't it make more sense to have the inheritance the other way around? i.e. ColorValue <: AbstractAlphaColorValue? A ColorValue is effectively just the special case of an alpha colorvalue with alpha==1. Then you would have an accessor method alpha{T}(c::ColorValue{T}) = one(T), and so on.
This way, we could write generic alpha-aware code that worked on AbstractAlphaColorValue and have it work automatically with ColorValue too. (But the converse, of having alpha-unaware code work with AlphaColorValues, makes less sense to me.)
+1 for this suggestion.
If one is contemplating changing things, also relevant are: #56 and color work in the up-and-coming "new Images" which I'd be happy to move partly or completely over here.