[GRASS5] Color code cleanup

GRASS GIS GRASS GIS developer
1

Hi,

I've committed quite a large piece of color code cleanup. Here's a summary:

Definitions of the rgb values for preallocated ("standard") colors are now in
colors.h. The major color related code (D_translate, G_str_to_color, and the
definitions of color tables in XDRIVER and PNG) have been updated to use
this. I haven't updated the named_colr.c code because I don't have an easy
way to test it. I haven't touched the postscript code, as it is quite
dissimilar.

gui.tcl color code is changed to match the colors in colors.h. My illconceived
tcltk color prompt type has been removed.

Color definitions have changed slightly. MAGENTA and VIOLET make more sense
now for the XDRIVER and PNG colors. GREY and GRAY are now 128 128 128, and
other midrange values are 128s. The midrange standard changes the meaning of
G_str_to_color named colors slightly.

I added a new preallocated color for cyan. This makes MAXCOLORS 14.

PURPLE and "purple" are aliases for VIOLET and "violet", which might help out
the ps.map issue Hamish pointed out.

If you are getting incorrect colors after make-ing or if "brown" fails to be a
color the easiest way to correct this will be with a make clean and a make.
touch-ing or modifying colors.h doesn't cause XDRIVER, PNG, or lib/display
to be recompiled.

These are somewhat independent changes:

It is possible to extend named colors modifying just G_str_to_color, though I
am uncertain of the wisdom of doing so. It is ill-advised at least until the
color display modules consistently use G_str_to_color to parse colors.

I added some more D_ commands to make working with colors easier and eliminate
the need for including colors.h and working with MAXCOLORS in display
modules. I've updated a couple modules to use this simplified code. These
modules also now have gisprompts for color.

I hope to get through almost all of the display modules, simplifying code,
adding custom color support to those that don't already have it, and adding
gisprompts. d.colortable isn't going to get custom border colors.

--Cedric

2

Cedric Shock wrote:

Definitions of the rgb values for preallocated ("standard") colors are now in
colors.h. The major color related code (D_translate, G_str_to_color, and the
definitions of color tables in XDRIVER and PNG) have been updated to use
this. I haven't updated the named_colr.c code because I don't have an easy
way to test it. I haven't touched the postscript code, as it is quite
dissimilar.

If you've touched the colour-handling code in the display driver or
libraries, please test the changes on a colour-mapped display (e.g.
"startx -- :1 -depth 8 -cc 3").

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

3

(As I'm sure you've found)
When I added RGB support to a bunch the display modules using
MAXCOLORS+1 type tricks I considered it a bit of an ugly hack and it was
done a little differently for each module. This was partially to get
around the fact that gis.h doesn't have a RGBCOLOR struct in it.
(e.g. SYMBCOLOR in symbol.h)

I'd have such a creature as so:

(in gis.h)

typedef struct {
    int set; /* 0 unset, 1 set, -1 "none" */
    unsigned char r, g, b;
} RGBCOLOR;

(RGB_COLOR is already in graphics.h)

FWIW, SYMBCOLOR includes floating point RGB values, but I'm not sure
if they'd ever be used.

Hamish

4

I haven't updated the named_colr.c code because I don't have an easy
way to test it. I haven't touched the postscript code, as it is quite
dissimilar.

note
ps/ps.map/ps_colors.c
and
lib/gis/named_colr.c

define the same struct. (that answers some questions for me)

G_color_values() and G_color_name() as defined in named_colr.c are used
by d.rast, r.colors(, and the SWIG interface?).

PostScript will use 0.00->1.00 instead of 0->255

MAGENTA

is the default MASK color still shocking?

I added some more D_ commands to make working with colors easier and
eliminate the need for including colors.h and working with MAXCOLORS
in display modules. I've updated a couple modules to use this
simplified code.
These modules also now have gisprompts for color.

Please post summaries of such changes to grass5 as it's hard to keep up
with, understand, and vet all changes in CVS. I'm going to try and
finish the symbol plotting function(s) in the next few days, no point in
doing things twice or "the old way".

thanks,
Hamish

5

Hamish wrote:

PostScript will use 0.00->1.00 instead of 0->255

This should be achieved by dividing by 255.0 in ps.map; it should
still use the same tables.

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

6

Hamish,

> I added some more D_ commands to make working with colors easier and
> eliminate the need for including colors.h and working with MAXCOLORS
> in display modules. I've updated a couple modules to use this
> simplified code.
> These modules also now have gisprompts for color.

Please post summaries of such changes to grass5 as it's hard to keep up
with, understand, and vet all changes in CVS. I'm going to try and
finish the symbol plotting function(s) in the next few days, no point in
doing things twice or "the old way".

The "new way" isn't very much different from the "old way". Colors are still
being handled as integers, not as RGB structures, but the programmer can
ignore the gritty details of this.

1. On options for colors do not use, for example:
optfgcolor->options = D_list_colors() ; /* Don't use this anymore */
Instead use a color gisprompt:
optfgcolor->gisprompt = GISPROMPT_COLOR ;
It's a good idea to specify the default too:
optfgcolor->answer = DEFAULT_FG_COLOR ;

2. Use D_parse_color to interpret these options. The second argument is 0 if
none is not an acceptable color, 1 if it is.

int fgcolor, bgcolor;
bgcolor = D_parse_color (optbgcolor->answer, 1);
fgcolor = D_parse_color (optfgcolor->answer, 0);

This command will G_fatal_error if the color is no good.

3. Use D_raster_use_color to select colors for drawing. It returns false if
the color can't be drawn with, such as "none". (This replaces use of R_color
and R_standard_color).

if (D_raster_use_color (bgcolor) ) {
    R_box_abs(...) ;
}

D_raster_use_color (fgcolor) ;

R_polyline_rel (...

4. Remove colors.h from the included files. This will help make sure the code
is cleaner.

--Cedric

7

On Tuesday 18 April 2006 02:12, you wrote:

(As I'm sure you've found)
When I added RGB support to a bunch the display modules using
MAXCOLORS+1 type tricks I considered it a bit of an ugly hack and it was
done a little differently for each module. This was partially to get
around the fact that gis.h doesn't have a RGBCOLOR struct in it.
(e.g. SYMBCOLOR in symbol.h)

I'd have such a creature as so:

(in gis.h)

typedef struct {
    int set; /* 0 unset, 1 set, -1 "none" */
    unsigned char r, g, b;
} RGBCOLOR;

Why do we need to keep track of set information? Since structs are sometimes
but not consistently initialized to zeros by compilers this seems like an
easy way to make bugs that are very difficult to track down.

This is everything I can think of that could go in a color:

typedef struct {
    int standard; /* Positive integer if this is a standard color, 0
otherwise*/
    unsigned char r, g, b, a; /* red, green, blue, and alpha */
    float fr, fg, fb, fa; /* This is twice as big as everything else combined
*/
} RGBA_COLOR;

None can simply be alpha==0. An easy way to check this is just:

if (color.a) {
  do something
}

I think the floats should probably go. We might want some shared structure for
them, but it'd be separate anyway.

Display code might want to tack on another indexed color for the non-standard
color table. This does avoid lookup of RGB value in a table on an indexed
display, right? Setting colors seems to be being done fairly infrequently so
this probably isn't a big performance concern.

Is there any R_ protocol command for none. Something that just turns off
drawing until another color gets set?

--Cedric

8

Cedric Shock wrote:

Display code might want to tack on another indexed color for the non-standard
color table. This does avoid lookup of RGB value in a table on an indexed
display, right? Setting colors seems to be being done fairly infrequently so
this probably isn't a big performance concern.

Is there any R_ protocol command for none. Something that just turns off
drawing until another color gets set?

No.

The notion of "none" is only applicable to the raster operations,
where index zero can optionally indicate transparent (used by e.g.
"d.rast -o ...").

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

9

> gis.h doesn't have a RGBCOLOR struct in it.
> (e.g. SYMBCOLOR in symbol.h)
>
> I'd have such a creature as so:
>
> (in gis.h)
>
> typedef struct {
> int set; /* 0 unset, 1 set, -1 "none" */
> unsigned char r, g, b;
> } RGBCOLOR;
>

Why do we need to keep track of set information?

It's good accounting practice.
We need a meta-place holder for "none" anyway.

Since structs are sometimes but not consistently initialized to zeros
by compilers this seems like an easy way to make bugs that are very
difficult to track down.

I think that bug would be pretty easy to track down. If the compiler
initialized it to 0 (CFLAGS="-g"?), the default is "unset", which tells
you pretty quick that you forgot something when you go to use it.
If everything comes up as 0:0:0 black you've got a good lead too.

This is everything I can think of that could go in a color:

typedef struct {
    int standard; /* Positive integer if this is a standard color, 0
otherwise*/
    unsigned char r, g, b, a; /* red, green, blue, and alpha */
    float fr, fg, fb, fa; /* This is twice as big as everything else
    combined
*/
} RGBA_COLOR;

I do like adding the alpha channel for GUI future-proofness.
I have no idea if it will ever be useful*, but it only costs a byte.
  *(guidance from the GUI experienced please?)
For now it does mean setting it to 1 every time you set a color, which
is extra work for little gain.

What's the point of knowing if there's an equivalent standard color?
If dealing with standard colors, wouldn't you just use
D_raster_use_color() or R_standard_color() in the first place and not
bother with RGB? Are the standard colors that much faster to use that
knowing you had one would make a difference in the 1 in 10^6 chance
that you hit one?

None can simply be alpha==0. An easy way to check this is just:

if (color.a) {
  do something
}

mmmhh. It would work, but I'm not crazy about mixing concepts.
In a practical sense I see how 99.9% of the time they are the same
thing, and I'm having a hard time trying to think of a case where
"color=none" doesn't mean "transparent", but I do consider "none" more
of an absence case than a alpha=0 case.

Logically, "if(color.set != TRUE) continue;" is a lot better and
readable than "if(!color.a) continue;".

If used uninitialized (& compiler sets to 0) it could be frustrating to
figure out why objects randomly don't display. If used uninitialized
and the compiler hasn't initialized it anything, you've got a very good
chance of bizarre random shading.

..... I'm still preferring to have the set flag in there.

I think the floats should probably go. We might want some shared
structure for them, but it'd be separate anyway.

I agree. Who knows how far off analog displays are? (retro!)
So far I haven't heard a case for when fp color would be used. Anyone?
(besides PostScript...)

If there are no objections, I'll add this to gis.h in the coming days:

typedef struct {
    int set; /* 0 unset, 1 set, -1 "none" */
    unsigned char r, g, b, a; /* red, green, blue, and alpha */
} RGBA_COLOR;

I would like to get this right the first time.

Hamish