[GRASS5] Color prompt partially implemented, color confusion.

[Cedric_Shock]

Other grass developers,

I have partially implemented a color prompt. It works for the colors
recognized by G_str_to_color. The gisprompt for it is "color,grass,color".
(It also works for tcltk colors of the #RRGGBB variety with the prompt
"color,tcltk,color"). In command line interactive mode the prompt is ignored
and entry works as before.

There is much disparity in the code relating to colors in the display modules,
enough to generate compiler warnings. Display module option colors come in at
least three flavors:

* red green and blue specified via G_str_to_color
These programs use G_str_to_color to parse colors. They then either look for
the color in the color table or possibly add it. I added gisprompts of the
form "color,grass,color" for all of these.
d.graph
d.mapgraph
d.path
d.vect
d.vect.chart

These generate the warnings:
warning: passing argument 1 of ‘R_RGB_color’ with different width due to
prototype (d.graph, d.vect, d.vect.chart)
warning: passing argument 1 of ‘R_reset_color’ with different width due to
prototype (d.mapgraph, d.path, d.vect)

* An already allocated, named color via D_translate_color.
These are probably best served by the options list.

* either red:green:blue or the name of an already allocated, named color.
There programs usually allocate a color for each option, implement their own
parsers for red:green:blue colors and define that color to be the result of
parsing, or if a named color is used replace the reference to their allocated
color with the number of the named color (gotten via D_translate_color).

These might be well served by another color prompt, say "color,display,color".
They might also be better off looking for an existing matching color and
using it if it exists, otherwise adding a new color, and if that fails
picking the closest existing color. This could (and might already be) all
bound up in a single procedure that takes a string for a color and returns an
index into the color table to the most appropriate color.

These also tend to use R_reset_color and generate warnings.

The main issue boils down to the following: Is it better to have consistent
color names between D_translate_color and G_str_to_color, or to have the
color names currently used have the same meanings they did in the past. If
consistency is more important then the color tables should be made to match
and a single prompt should be used for both. If backwards compatibility is
more important then we should have multiple prompts.

Differences in named color definitions:

G_str_to_color ignores case.
D_translate_color is case sensitive

name D_translate_color G_str_to_color
red 255 0 0 same
orange 255 128 0 255 127 0
yellow 255 255 0 same
green 0 255 0 same
blue 0 0 255 same
indigo 0 128 255 0 127 255
violet 255 0 255 same (this is a strange violet)
black 0 0 0 same
white 255 255 255 same
gray 175 175 175 127 127 127
brown 180 77 25 same
magenta 255 0 128 255 0 255 (D_ has a very strange magenta)
aqua 100 128 255 100 127 255
grey lookup to gray 127 127 127
cyan doesn't exist 0 255 255

How should we straighten out these colors?

--Cedric

[Helena_Mitasova]

The following are my suggestions - I vote for consistency - the changes needed are
relatively small so they should not dramatically change expected color tables:

Differences in named color definitions:

G_str_to_color ignores case.
D_translate_color is case sensitive

name D_translate_color G_str_to_color
red 255 0 0 same

                                                change 127 -> 128

orange 255 128 0 255 127 0

yellow 255 255 0 same
green 0 255 0 same
blue 0 0 255 same

                                                 change 127->128

indigo 0 128 255 0 127 255

this is really magenta - violet should be 128 0 255

violet 255 0 255 same (this is a strange violet)
black 0 0 0 same
white 255 255 255 same
gray 175 175 175 127 127 127
brown 180 77 25 same

this should be 255 0 255

magenta 255 0 128 255 0 255 (D_ has a very strange magenta)
aqua 100 128 255 100 127 255
grey lookup to gray 127 127 127
cyan doesn't exist 0 255 255

there are some issues with grey and gray that I don't remember ,
maybe gray as 175 and grey 127 would work?

How should we straighten out these colors?

others may have different suggestions,

Helena

--Cedric

_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

[Cedric_Shock]

Hey,

Does this (untested) snippet look useful for turning strings of colors into
colors in the color table? We could get slightly more information about
failure to find colors / use of none if it was safe to use a negative return
value for a failure (there are no negative colors) or if the integer
suggested color number is a pointer and is modified.

Thanks for the comments,

--Cedric

/*!
* \brief color name and suggested number to actual number
*
* Takes a color <b>name</b> or <b>red:green:blue</b> code in ascii
* and a <b>suggested color index</b>.
* If the color is a standard preallocated color it returns the color number
for that color.
* Otherwise (if the color is not standard) it sets the color of the supplied
index to
* the specified color (see R_reset_color).
* Returns 0 if color is not known or if the color is none.
*
* \param name_or_code
* \param suggested_color_index
* \return int
*/

int D_translate_or_add_color (const char * str, int index)
{
  int redi, greeni, bluei;
  int preallocated, ret;

  char lowerstr[MAX_COLOR_LEN];

  /* Make the color string lowercase for display colors */
  G_strcpy (lowerstr, str );
  G_chop (lowerstr);
  G_tolcase (lowerstr);

  preallocated = D_translate_color (lowerstr);
  
  if (preallocated != 0) {
    return preallocated;
  }

  ret = G_str_to_color (str, &redi, &greeni, &bluei);

  if (ret == 2) {
    /* None color */
    return 0;
  } else if (ret == 1) {
    /* Add the specified color to the suggested index */
    R_reset_color ( (unsigned char) redi, (unsigned char) greeni, (unsigned
char) bluei, index);
    return index;
  }

  return 0;
}

[Glynn_Clements1]

Cedric Shock wrote:

I have partially implemented a color prompt. It works for the colors
recognized by G_str_to_color. The gisprompt for it is "color,grass,color".
(It also works for tcltk colors of the #RRGGBB variety with the prompt
"color,tcltk,color").

There should be a single syntax for colours. Both libgis and Tcl/Tk
should accept either form. The only situation which may need special
handling is if anything is restricted to the standard colours (those
listed in colors.h, which are defined automatically by the display
drivers).

In command line interactive mode the prompt is ignored
and entry works as before.

There is much disparity in the code relating to colors in the display modules,
enough to generate compiler warnings. Display module option colors come in at
least three flavors:

* red green and blue specified via G_str_to_color
These programs use G_str_to_color to parse colors. They then either look for
the color in the color table or possibly add it. I added gisprompts of the
form "color,grass,color" for all of these.
d.graph
d.mapgraph
d.path
d.vect
d.vect.chart

These generate the warnings:
warning: passing argument 1 of e-F¡R_RGB_color¢ with different width due to e-A
prototype (d.graph, d.vect, d.vect.chart)
warning: passing argument 1 of e-F¡R_reset_color¢ with different width due to e-A
prototype (d.mapgraph, d.path, d.vect)

This is a non-issue, although adding explicit type casts should make
the warnings go away.

* An already allocated, named color via D_translate_color.
These are probably best served by the options list.

* either red:green:blue or the name of an already allocated, named color.
There programs usually allocate a color for each option, implement their own
parsers for red:green:blue colors and define that color to be the result of
parsing, or if a named color is used replace the reference to their allocated
color with the number of the named color (gotten via D_translate_color).

These might be well served by another color prompt, say "color,display,color".
They might also be better off looking for an existing matching color and
using it if it exists, otherwise adding a new color, and if that fails
picking the closest existing color. This could (and might already be) all
bound up in a single procedure that takes a string for a color and returns an
index into the color table to the most appropriate color.

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).

These also tend to use R_reset_color and generate warnings.

The main issue boils down to the following: Is it better to have consistent
color names between D_translate_color and G_str_to_color, or to have the
color names currently used have the same meanings they did in the past. If
consistency is more important then the color tables should be made to match
and a single prompt should be used for both. If backwards compatibility is
more important then we should have multiple prompts.

Consistency. I'm fairly sure that the differences between the two
tables are a mistake, probably due to someone changing the definition
in only one place.

In 5.3, XDRIVER uses 127 for grey while the PNG driver uses 175. Both
have the wrong value for magenta.

Differences in named color definitions:

G_str_to_color ignores case.
D_translate_color is case sensitive

name D_translate_color G_str_to_color
red 255 0 0 same
orange 255 128 0 255 127 0
yellow 255 255 0 same
green 0 255 0 same
blue 0 0 255 same
indigo 0 128 255 0 127 255
violet 255 0 255 same (this is a strange violet)
black 0 0 0 same
white 255 255 255 same
gray 175 175 175 127 127 127
brown 180 77 25 same
magenta 255 0 128 255 0 255 (D_ has a very strange magenta)
aqua 100 128 255 100 127 255
grey lookup to gray 127 127 127
cyan doesn't exist 0 255 255

How should we straighten out these colors?

Colours which refer to the corners of the colour cube should have the
correct values, so magenta should be 255,0,255. Arguably, gray/grey
(do we need both spellings; the rest of GRASS uses US spellings
consistently) should be 127/128 (the centre of the colour cube).

It doesn't really matter about orange, indigo, violet, brown or aqua,
as those don't have any obvious definition, although we should decide
whether 50% is 127 or 128 and use one value consistently.

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.

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.

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

[Maciej_Sieczka3]

Cedrick,

There is an issue with colors in ps.map which I thought might be
somewhat related to this thread. Thought you might like to know.

http://intevation.de/rt/webrt?serial_num=3049

Best,
Maciek

--------------------
W polskim Internecie s? setki milion?w stron. My przekazujemy Tobie tylko najlepsze z nich!
http://katalog.panoramainternetu.pl/

[Helena_Mitasova]

G_str_to_color ignores case.
D_translate_color is case sensitive

name D_translate_color G_str_to_color
red 255 0 0 same
orange 255 128 0 255 127 0
yellow 255 255 0 same
green 0 255 0 same
blue 0 0 255 same
indigo 0 128 255 0 127 255
violet 255 0 255 same (this is a strange violet)
black 0 0 0 same
white 255 255 255 same
gray 175 175 175 127 127 127
brown 180 77 25 same
magenta 255 0 128 255 0 255 (D_ has a very strange magenta)
aqua 100 128 255 100 127 255
grey lookup to gray 127 127 127
cyan doesn't exist 0 255 255

How should we straighten out these colors?

Colours which refer to the corners of the colour cube should have the
correct values, so magenta should be 255,0,255. Arguably, gray/grey
(do we need both spellings; the rest of GRASS uses US spellings
consistently) should be 127/128 (the centre of the colour cube).

It doesn't really matter about orange, indigo, violet, brown or aqua,
as those don't have any obvious definition, although we should decide
whether 50% is 127 or 128 and use one value consistently.

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.

I would vote for adding cyan if possible, Helena

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.

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

_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

[Cedric_Shock]

Fellow developers,

On Monday 17 April 2006 01:16, Glynn Clements wrote:

Cedric Shock wrote:
> I have partially implemented a color prompt. It works for the colors
> recognized by G_str_to_color. The gisprompt for it is
> "color,grass,color". (It also works for tcltk colors of the #RRGGBB
> variety with the prompt "color,tcltk,color").

There should be a single syntax for colours. Both libgis and Tcl/Tk
should accept either form. The only situation which may need special
handling is if anything is restricted to the standard colours (those
listed in colors.h, which are defined automatically by the display
drivers).

That's right-on from the user's standpoint. I musn't have been thinking
clearly. I'll remove the tcltk prompt code.

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.

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().

Consistency. I'm fairly sure that the differences between the two
tables are a mistake, probably due to someone changing the definition
in only one place.

Ok, change they shall.

Colours which refer to the corners of the colour cube should have the
correct values, so magenta should be 255,0,255. Arguably, gray/grey
(do we need both spellings; the rest of GRASS uses US spellings
consistently) should be 127/128 (the centre of the colour cube).

I use both grey and gray consistently. It doesn't cost us much of anything
since they are using the same color table entry.

It doesn't really matter about orange, indigo, violet, brown or aqua,
as those don't have any obvious definition, although we should decide
whether 50% is 127 or 128 and use one value consistently.

128. There it's decided. (Actually I went looking on the internet for any
compelling reason to pick one over the other and found none.)

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.

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

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

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}
...

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.

--Cedric

[Glynn_Clements1]

Helena Mitasova wrote:

>> G_str_to_color ignores case.
>> D_translate_color is case sensitive
>>
>> name D_translate_color G_str_to_color
>> red 255 0 0 same
>> orange 255 128 0 255 127 0
>> yellow 255 255 0 same
>> green 0 255 0 same
>> blue 0 0 255 same
>> indigo 0 128 255 0 127 255
>> violet 255 0 255 same (this is a strange violet)
>> black 0 0 0 same
>> white 255 255 255 same
>> gray 175 175 175 127 127 127
>> brown 180 77 25 same
>> magenta 255 0 128 255 0 255 (D_ has a very strange
>> magenta)
>> aqua 100 128 255 100 127 255
>> grey lookup to gray 127 127 127
>> cyan doesn't exist 0 255 255
>>
>> How should we straighten out these colors?
>
> Colours which refer to the corners of the colour cube should have the
> correct values, so magenta should be 255,0,255. Arguably, gray/grey
> (do we need both spellings; the rest of GRASS uses US spellings
> consistently) should be 127/128 (the centre of the colour cube).
>
> It doesn't really matter about orange, indigo, violet, brown or aqua,
> as those don't have any obvious definition, although we should decide
> whether 50% is 127 or 128 and use one value consistently.
>
> 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.

I would vote for adding cyan if possible, Helena

Can we discard "aqua" in favour of cyan? Or make "aqua" a synonym for
cyan? If we don't have to change the number of standard colours, it's
less likely that something important will break.

I wouldn't completely rule out the possibility that some random module
has the number 13 hardcoded into it.

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

[Helena_Mitasova]

Helena Mitasova
Dept. of Marine, Earth and Atm. Sciences
1125 Jordan Hall, NCSU Box 8208,
Raleigh NC 27695
http://skagit.meas.ncsu.edu/~helena/

On Apr 17, 2006, at 1:02 PM, Glynn Clements wrote:

Helena Mitasova wrote:

G_str_to_color ignores case.
D_translate_color is case sensitive

name D_translate_color G_str_to_color
red 255 0 0 same
orange 255 128 0 255 127 0
yellow 255 255 0 same
green 0 255 0 same
blue 0 0 255 same
indigo 0 128 255 0 127 255
violet 255 0 255 same (this is a strange violet)
black 0 0 0 same
white 255 255 255 same
gray 175 175 175 127 127 127
brown 180 77 25 same
magenta 255 0 128 255 0 255 (D_ has a very strange
magenta)
aqua 100 128 255 100 127 255
grey lookup to gray 127 127 127
cyan doesn't exist 0 255 255

How should we straighten out these colors?

Colours which refer to the corners of the colour cube should have the
correct values, so magenta should be 255,0,255. Arguably, gray/grey
(do we need both spellings; the rest of GRASS uses US spellings
consistently) should be 127/128 (the centre of the colour cube).

It doesn't really matter about orange, indigo, violet, brown or aqua,
as those don't have any obvious definition, although we should decide
whether 50% is 127 or 128 and use one value consistently.

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.

I would vote for adding cyan if possible, Helena

Can we discard "aqua" in favour of cyan? Or make "aqua" a synonym for
cyan? If we don't have to change the number of standard colours, it's
less likely that something important will break.

I use aqua a lot in addition to cyan in the same map, so making aqua synonym for cyan would be
confusing (at least for me). If necessary, it would be less confusing to discard aqua and I would just define it as RGB or use indigo
instead.

Helena

I wouldn't completely rule out the possibility that some random module
has the number 13 hardcoded into it.

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

[Glynn_Clements1]

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@gclements.plus.com>

[H_B]

although we should decide whether 50% is 127 or 128 and use one value
consistently.

0,127,255
1,128,256

These series seem logical. If 255 is max, use 127.

aqua: FWIW I like the given color and use it often. I don't care if it
is renamed. If there is a pressing reason I could easily live without it.

I wouldn't completely rule out the possibility that some random module
has the number 13 hardcoded into it.

FWIW, I think I fixed that to use MAXCOLORS+1 wherever I at first
hardcoded it and a quick grep doesn't turn up anything.

Hamish

[H_B]

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.

If implementing the second case you might as well just have it parse the
700 odd standard X color names from /etc/X11/rgb.txt.

> 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

(any reason?)

I'd prefer to match the ANSI color order (first one).
Does the the old DOS background color order match any of the above?
If there's a standard or some logical order we can latch on to we might
as well use that.

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

what's wrong with
G_str_to_color("red", &r, &g, &b);
and
R_standard_color(D_translate_color("red"));
?
(or this is for the sub-library layer?)

Hamish

[H_B]

> > 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).

???

In the past I have been able to get rid of the unsigned char <-> int
conversion warnings by doing proper casts. I never tried to fix them
as I prefered a proper answer to randomly picking one or the other, and
warnings are better than a fix with no policy backing it up.

Hamish

[Glynn_Clements1]

Hamish wrote:

I'd prefer to match the ANSI color order (first one).
Does the the old DOS background color order match any of the above?
If there's a standard or some logical order we can latch on to we might
as well use that.

Actually, CGA (and the default EGA/VGA palettes) is the other way
around, i.e. 4r+2b+g, with the top bit being the "brightness" flag.

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

[Glynn_Clements1]

Cedric Shock wrote:

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]));
}

One note about source code: tab stops are every 8 columns. If your
editor is set to use something else, you need to change it when
editing GRASS source code, otherwise the formatting will get screwed
up.

The indentation width is a different matter. There isn't (yet) an
agreed standard; for existing files, you should follow whichever
convention(s) the source file is already using.

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

[Glynn_Clements1]

Glynn Clements wrote:

> 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.

... and that one place should be a source (.c) file, not a header
file.

Declarations go in header files, definitions go in source files.

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

[H_B]

One note about source code: tab stops are every 8 columns. If your
editor is set to use something else, you need to change it when
editing GRASS source code, otherwise the formatting will get screwed
up.

The indentation width is a different matter. There isn't (yet) an
agreed standard; for existing files, you should follow whichever
convention(s) the source file is already using.

This has been added to the SUBMITTING file not long ago:

14. To promote a consistent coding style, please use the "indent" program
    on all new C modules using the following switches:
    
     $ indent -nbad -bap -bbb -nbbo -nbc -br -bli1 -bls -cbi0 -ncdb -nce \
        -ci4 -cli0 -ncs -d0 -di0 -fc1 -nfca -hnl -i4 -ip4 -l80 -lc80 -lp \
  -npcs -pi4 -nprs -npsl -sbi0 -sc -nsob -ss -ts8 main.c

    Existing code should not be re-indented except in extreme cases, as this
    will make "diff" comparisons with older versions impossible. If indent is
    needed, do not check in any changes other than the indentation in the same
    commit! Do add the indent switches and any indent warning messages to the
    CVS log. Any change or fix mixed in with an indent is very hard to track
    making it hard for others to follow the change or fix any new bugs.

http://freegis.org/cgi-bin/viewcvs.cgi/grass6/SUBMITTING?rev=HEAD&content-type=text/vnd.viewcvs-markup

Suggested indent level is 4 spaces, 8 spaces become a tab.

The idea is not to be strict about it but for the code to be readable
and (somewhat) consistent in style.

Hamish

[Glynn_Clements1]

Hamish wrote:

This has been added to the SUBMITTING file not long ago:

14. To promote a consistent coding style, please use the "indent" program
    on all new C modules using the following switches:
    
     $ indent -nbad -bap -bbb -nbbo -nbc -br -bli1 -bls -cbi0 -ncdb -nce \
        -ci4 -cli0 -ncs -d0 -di0 -fc1 -nfca -hnl -i4 -ip4 -l80 -lc80 -lp \
  -npcs -pi4 -nprs -npsl -sbi0 -sc -nsob -ss -ts8 main.c

Any objections to changing:

  -bli1 -> -bli0
  -br -> -bl
?

Does anyone else have specific preferences?

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

[H_B]

> This has been added to the SUBMITTING file not long ago:
>
> 14. To promote a consistent coding style, please use the "indent" program
> on all new C modules using the following switches:
>
> $ indent -nbad -bap -bbb -nbbo -nbc -br -bli1 -bls -cbi0 -ncdb -nce \
> -ci4 -cli0 -ncs -d0 -di0 -fc1 -nfca -hnl -i4 -ip4 -l80 -lc80 -lp \
> -npcs -pi4 -nprs -npsl -sbi0 -sc -nsob -ss -ts8 main.c

Any objections to changing:

  -bli1 -> -bli0
  -br -> -bl

translation:
  '{' comes on next line after if,for etc instead of "if {"
  Structs, functions already do this (-bls)

?

Does anyone else have specific preferences?

We all will have our own, slighltly differing.
I am not too worried about it.

Hamish

[Glynn_Clements1]

Hamish wrote:

> > This has been added to the SUBMITTING file not long ago:
> >
> > 14. To promote a consistent coding style, please use the "indent" program
> > on all new C modules using the following switches:
> >
> > $ indent -nbad -bap -bbb -nbbo -nbc -br -bli1 -bls -cbi0 -ncdb -nce \
> > -ci4 -cli0 -ncs -d0 -di0 -fc1 -nfca -hnl -i4 -ip4 -l80 -lc80 -lp \
> > -npcs -pi4 -nprs -npsl -sbi0 -sc -nsob -ss -ts8 main.c
>
> Any objections to changing:
>
> -bli1 -> -bli0
> -br -> -bl

translation:
  '{' comes on next line after if,for etc instead of "if {"
  Structs, functions already do this (-bls)

That's the -br -> -bl change.

Changing -bli1 -> -bli0 means that braces are aligned with the command
rather than being an extra space in, i.e.

        if (...)
        {
                ...
        }

rather than:

        if (...)
         {
                ...
         }

Having braces aligned makes it easier to do manual alignment, as
everything is then aligned to a multiple of 4 columns. It's also
easier in terms of configuring Emacs' auto-formatting, which expects
everything to be specified as multiples of the indentation step.

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