[GRASS5] Color prompt partially implemented, color confusion.

[Glynn Clements]

Cedric Shock wrote:

> > This is a non-issue, although adding explicit type casts should make
> > the warnings go away.
> 
> It doesn't. It's quite strange; Searching I could only fund R_reset_color 
> prototyped and implemented once, both as int R_reset_color (unsigned char, 
> unsigned char, unsigned char, int). Explicit casts are used almost everywhere 
> and the warning is still generated.

In that case, I can only assume that the warning is essentially
telling you that you have an "odd" prototype (i.e. one which doesn't
follow the K&R rules of not using anything narrower than int/double).

> > Options which only accept a standard colour (are there any?) should
> > use "opt->options = D_COLOR_LIST;". Options which accept either RGB or
> > a named colour should preserve the distinction where possible (e.g.
> > using R_standard_color() instead of adding a new colourmap entry).
> 
> Most of these use D_color_list().

Same value; the D_COLOR_LIST macro has the advantage that it can be
used to initialise static variables.

> > Cyan should either be removed from G_str_to_color() or added to the
> > list of standard colours in the display architecture. Updating the
> > display architecture is tricky, as there are quite a few places that
> > would need to be changed. OTOH, it's the only corner which isn't in
> > the list.
> 
> Let's add it, assuming it would get used enough that having it preallocated 
> and thus more likely to be shared will tend to reduce color table size.
> 
> There's no reason we can't have two sets of color names, one that is to 
> preallocated colors and one that is just many,many,many color names that only 
> work as another way of specifying RGB tripples.

It's more of a problem if the two lists are almost, but not quite,
identical.

However, would the longer list be hard-coded or read from a file? If
it's a file, the temptation is there to make local modifications,
which people then forget (or don't realise) are local modifications.

> > If choose to add cyan to the list of standard colours, we may as well
> > renumber them so that the corners use the indices from 1 to 8 (i.e.
> > index = 1+r+2g+4b); zero is reserved for transparent in some contexts.
> 
> This doesn't really matter, but that'd be:
> 
> 1 - Black
> 2 - Red 
> 3 - Green
> 4 - Yellow
> 5 - Blue
> 6 - Magenta
> 7 - Cyan
> 8 - White
> 
> I'm kind of partial to
> 1 - Black
> 2 - Red
> 3 - Green
> 4 - Blue
> 5 - Yellow
> 6 - Magenta
> 7 - Cyan
> 8 - White

r+2g+4b has a rational basis, which is why it's the numbering used for
the ANSI colour escape sequences (curses etc), the VGA palette etc
(actually, every default 8- or 16-colour palette I've encountered uses
it).

> I'm in favor of some more macros in colors.h somewhere along this vein:
> 
> #define BLACK_RGB 0, 0, 0
> #define RED_RGB 255, 0, 0

Macros which expand to multiple syntactic entities are a bad idea.

> These would slightly confuse the calls to color adding things. Even better 
> would be another global like: (pardon the cheating c)
> 
> unsigned char standard_color_rgb[MAX_COLOR_NUM + 1][3] = {
>   {   0,   0,   0}, /* This is a dummy entry to make lookup easier */
>   {   0,   0,   0},
>   {255,   0,   0}
>  ...

That makes more sense. Also, G_str_to_color() should have a
lower-level equivalent which translates a string to an index.

> Which would allow great simplifications to driver code of something like this:
> 
> for (colorindex = 1; colorindex <= MAX_COLOR_NUM; colorindex++) {
>   LIB_assign_standard_color(colorindex, DRV_lookup_color(
>     standard_color_rgb[colorindex][0],
>     standard_color_rgb[colorindex][1],
>     standard_color_rgb[colorindex][2]));	
> }
> 
> If we also have standard_color_names[...] then we could eliminate almost all 
> of the disparate code. I haven't looked at the postscript stuff yet.

Certainly, the standard colour table should be defined in one place
only.

-- 
Glynn Clements <glynn at gclements.plus.com>