code review 1903049: image: introduce Grayscale and Grayscale16 types.

image: introduce Gray and Gray16 types, and remove the named colors
except for Black and White.

[nigeltao, 2010-08-01 17:53:00]

Hello rsc (cc: golang-dev@googlegroups.com, mpl, r),

I'd like you to review this change.

[rsc1, 2010-08-01 18:08:32]

Looks good but I would consider renaming the color,
which I think is far less useful than the type and so 
deserves the shorter name.  The color could be Gray50
(as in 50%).  See what Rob thinks.

[nigeltao_gnome, 2010-08-01 18:18:51]

On 2 August 2010 04:08,  <rsc@google.com> wrote:
> Looks good but I would consider renaming the color,
> which I think is far less useful than the type and so
> deserves the shorter name.  The color could be Gray50
> (as in 50%).  See what Rob thinks.

OTOH the names in names.go aren't arbitrary -- they're from
http://www.w3.org/TR/REC-html40/types.html#h-6.5

[rsc, 2010-08-01 21:25:38]

On Sun, Aug 1, 2010 at 11:18, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> On 2 August 2010 04:08,  <rsc@google.com> wrote:
>> Looks good but I would consider renaming the color,
>> which I think is far less useful than the type and so
>> deserves the shorter name.  The color could be Gray50
>> (as in 50%).  See what Rob thinks.
>
> OTOH the names in names.go aren't arbitrary -- they're from
> http://www.w3.org/TR/REC-html40/types.html#h-6.5

I know, but tough.  I think the color names take
second priority to the type names.  When was the
last time you used Gray (or Aqua) in a program?

Russ

[nigeltao_gnome, 2010-08-01 23:28:48]

On 02/08/2010, Rob 'Commander' Pike <r@golang.org> wrote:
> i agree the names are long, and i don't see the word 'grayscale'
> anywhere on that page.

That's the point. That page names colors, not image color models. For
them, "gray" means a color. Grayscale images are irrelevant.


On 02/08/2010, Russ Cox <rsc@golang.org> wrote:
> I know, but tough.  I think the color names take
> second priority to the type names.  When was the
> last time you used Gray (or Aqua) in a program?

On the contrary, I would expect to use the color more often than the
image type. I don't see myself using grayscale images, but I can see
myself drawing some gray text.

I think that, given image.Red and image.White are colors, then
image.Gray should also be one.

An alternative is to spin out a separate color package, so that
there's color.Gray and image.Gray. But my preference is to do what's
in this changelist.

[nigeltao_gnome, 2010-08-02 01:13:56]

On 2 August 2010 09:28, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> I think that, given image.Red and image.White are colors, then
> image.Gray should also be one.

Another alternative is just to drop the named colors like image.Red
and image.White, and if users want red they should just use
image.RGBAColor{255, 0, 0, 255}.

[rsc, 2010-08-02 22:46:52]

On Sun, Aug 1, 2010 at 18:13, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> On 2 August 2010 09:28, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
>> I think that, given image.Red and image.White are colors, then
>> image.Gray should also be one.
>
> Another alternative is just to drop the named colors like image.Red
> and image.White, and if users want red they should just use
> image.RGBAColor{255, 0, 0, 255}.

Black, White, Opaque, and Transparent
come up as colors relatively often.
The others not as much.  I like the idea
of having a separate image/color package.
color.White is a nice name.

Russ

[nigeltao_gnome, 2010-08-02 23:33:08]

On 3 August 2010 08:46, Russ Cox <rsc@golang.org> wrote:
> color.White is a nice name.

Yeah, color.White is a nice name.

Even if we spin up a separate color package, though, we still have a
naming problem. I would expect the things that are currently
image.FooColor (e.g. image.RGBAColor, image.AlphaColor) would move to
be color.Foo, and that would include what this CL currently calls
image.GrayscaleColor. The "color versus colormodel" question would now
rest on color.Gray instead of image.Gray.

I agree that Black, White, Opaque and Transparent are the important
colors. I'm happy to drop the others like Red, Blue and Gray.

How about I s/Grayscale/Gray/ as well as drop the non-important colors
in this CL, and the next CL will spin up a separate color package?

[rsc, 2010-08-02 23:37:28]

> Even if we spin up a separate color package, though, we still have a
> naming problem. I would expect the things that are currently
> image.FooColor (e.g. image.RGBAColor, image.AlphaColor) would move to
> be color.Foo, and that would include what this CL currently calls
> image.GrayscaleColor. The "color versus colormodel" question would now
> rest on color.Gray instead of image.Gray.

No, you can't do that because image types depend on these types.
That would be a circular dependency.  I think the image/color package
would be *just* for color names, a helper package to import when
you need an image of a specific color and nothing more.

Russ

[rsc, 2010-08-02 23:37:45]

> How about I s/Grayscale/Gray/ as well as drop the non-important colors
> in this CL, and the next CL will spin up a separate color package?

Sure

[nigeltao_gnome, 2010-08-02 23:43:53]

On 3 August 2010 09:37, Russ Cox <rsc@golang.org> wrote:
> No, you can't do that because image types depend on these types.
> That would be a circular dependency.  I think the image/color package
> would be *just* for color names, a helper package to import when
> you need an image of a specific color and nothing more.

I think I can move the named colors and colormodel types out of the
image package without causing a circular dependency. Package image
would import package color, but not vice versa. The color.ColorImage
type would implement the image.Image interface but you can implement
an interface without importing its package.

My aim is:

image.Color --> color.Color
image.ColorModel --> color.Model
image.Image --> image.Image
image.White --> color.White
image.RGBAColor --> color.RGBA
image.RGBAColorModel --> color.RGBAModel
image.RGBA --> image.RGBA
image.ColorImage --> color.ColorImage

WDYT?

[rsc, 2010-08-02 23:56:33]

> image.Color --> color.Color
> image.ColorModel --> color.Model
> image.Image --> image.Image
> image.White --> color.White
> image.RGBAColor --> color.RGBA
> image.RGBAColorModel --> color.RGBAModel
> image.RGBA --> image.RGBA
> image.ColorImage --> color.ColorImage

image.RGBA and image.RGBAColor are part
of the same implementation and should be in
the same package.  Splitting them like this
just means that the common case is that I have
to import two packages now where I currently
import just one.

On the other hand, if we moved just the
named colors out, then most clients (all?)
wouldn't change.

Russ

[nigeltao_gnome, 2010-08-02 23:56:56]

PTAL.

[nigeltao_gnome, 2010-08-03 00:05:06]

On 3 August 2010 09:56, Russ Cox <rsc@golang.org> wrote:
> On the other hand, if we moved just the
> named colors out, then most clients (all?)
> wouldn't change.

OK, I'll leave image.RGBAColor as image.RGBAColor.

But if the color package would only hold a handful of variables
(Black, White, Opaque, Transparent) and no functions or types, it
hardly seems worth it to me. I think I'd rather just leave it as
image.Black, etc.

[rsc, 2010-08-03 00:10:41]

> OK, I'll leave image.RGBAColor as image.RGBAColor.
>
> But if the color package would only hold a handful of variables
> (Black, White, Opaque, Transparent) and no functions or types, it
> hardly seems worth it to me. I think I'd rather just leave it as
> image.Black, etc.

It would mean that if you're just getting started there's
less to learn about the image package.  The named colors
are so unimportant compared to the rest.

It's fine either way.

Russ

[nigeltao, 2010-08-03 00:58:19]

*** Submitted as http://code.google.com/p/go/source/detail?r=98a867302cb4 ***

image: introduce Gray and Gray16 types, and remove the named colors
except for Black and White.

R=rsc
CC=golang-dev, mpl, r
http://codereview.appspot.com/1903049

[rog, 2010-08-03 10:02:14]

On 3 August 2010 01:05, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> On 3 August 2010 09:56, Russ Cox <rsc@golang.org> wrote:
>> On the other hand, if we moved just the
>> named colors out, then most clients (all?)
>> wouldn't change.
>
> OK, I'll leave image.RGBAColor as image.RGBAColor.
>
> But if the color package would only hold a handful of variables
> (Black, White, Opaque, Transparent) and no functions or types, it
> hardly seems worth it to me. I think I'd rather just leave it as
> image.Black, etc.

the color package could also implement support
for conversion of colour names (as strings) to colour
values. it could also implement conversions from other
colour spaces, such as HSV and CMYK, to RGB.

this seems enough meat for a package to me.

i'd be sad to lose the colour names.