[GRASS5] confused by lib fns

GRASS GIS GRASS GIS developer
1

while trying to fix the broken d.rast.num "-f", I am running into two
(possible) gis lib bugs in lib/gis/color_get.c.

1) G_get_raster_color() should just be G_get_color() updated to handle
   FP raster maps, ie you can specify map_type. You should be able to
   feed the fn a value and a color table and it will return R,G,B values.
   But G_get_raster_color() seems confused .. it takes a raster array?!
   This conflicts with the Doxygen comment? or at least is misleading.

   Also flows through to color_look.c's G_lookup_raster_colors() and
   G__lookup_colors(), also conflicting with Doxygen comments?

   lib/display/raster_rgb.c and ps/ps.map/ps_raster.c use it, so I won't
   say that it is completly broken, but it is confusing and I can't get
   it to return real values for R,G,B. see number.c in CVS (&cell[col]).
   Do I need to G_alloc() a cell array first or something?

   To get `d.rast.num -f` to work currently I need to cast to CELL then
   run G_get_color(). :confused:

   Note double Doxygen comments in color_get.c messes up old Prog. Manual.

2) All fn's in color_get.c have R,G,B as int, but lib/raster/RGB_color.c
   wants unsigned char. You can't access & set the display color without
   the compiler doing a cast. gis.h likes unsigned char.
   Should color_get.c be changed to use unsigned char?

thanks,
Hamish

2

Hamish wrote:

while trying to fix the broken d.rast.num "-f", I am running into two
(possible) gis lib bugs in lib/gis/color_get.c.

1) G_get_raster_color() should just be G_get_color() updated to handle
   FP raster maps, ie you can specify map_type. You should be able to
   feed the fn a value and a color table and it will return R,G,B values.

Can you clarify?

   But G_get_raster_color() seems confused .. it takes a raster array?!
   This conflicts with the Doxygen comment? or at least is misleading.

No, it takes a pointer to a single value. The value can be either
CELL, FCELL or DCELL, depending upon the map_type argument, so you
can't pass it directly.

   Also flows through to color_look.c's G_lookup_raster_colors() and
   G__lookup_colors(), also conflicting with Doxygen comments?

G_get_c_raster_color()'s comment is inaccurate. The function has the
same effect as the stated G_get_color() call, but doesn't actually
call it.

   lib/display/raster_rgb.c and ps/ps.map/ps_raster.c use it, so I won't
   say that it is completly broken, but it is confusing and I can't get
   it to return real values for R,G,B. see number.c in CVS (&cell[col]).
   Do I need to G_alloc() a cell array first or something?

No.

   To get `d.rast.num -f` to work currently I need to cast to CELL then
   run G_get_color(). :confused:

  G_get_raster_color(cell[col], &R, &G, &B, &colors, inmap_type);

should be:

  G_get_raster_color(&cell[col], &R, &G, &B, &colors, map_type);

The first argument needs to be a pointer to the value, and the last
argument should reflect the actual type of the data (i.e. it should be
map_type rather than inmap_type).

2) All fn's in color_get.c have R,G,B as int,

Pointer to int.

but lib/raster/RGB_color.c
   wants unsigned char. You can't access & set the display color without
   the compiler doing a cast. gis.h likes unsigned char.
   Should color_get.c be changed to use unsigned char?

RGB_color.c isn't the only user of that function; most of the callers
use ints.

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

3

> while trying to fix the broken d.rast.num "-f", I am running into
> two (possible) gis lib bugs in lib/gis/color_get.c.

..

> To get `d.rast.num -f` to work currently I need to cast to CELL
> then run G_get_color(). :confused:

  G_get_raster_color(cell[col], &R, &G, &B, &colors, inmap_type);
should be:
  G_get_raster_color(&cell[col], &R, &G, &B, &colors, map_type);

The first argument needs to be a pointer to the value, and the last
argument should reflect the actual type of the data (i.e. it should be
map_type rather than inmap_type).

Thanks. I knew about the pointer; it was the inmap_type vs. map_type
mistake which was causing me grief. Now fixed in CVS.

> 2) All fn's in color_get.c have R,G,B as int,

Pointer to int.

> but lib/raster/RGB_color.c
> wants unsigned char. You can't access & set the display color
> without the compiler doing a cast. gis.h likes unsigned char.
> Should color_get.c be changed to use unsigned char?

RGB_color.c isn't the only user of that function; most of the callers
use ints.

Can we at least standardize the library functions to something we agree
is "correct" (gis.h should be canonical) and worry about what the
rabble of modules call later?

Hamish

4

Hamish wrote:

> > 2) All fn's in color_get.c have R,G,B as int,
>
> Pointer to int.
>
> > but lib/raster/RGB_color.c
> > wants unsigned char. You can't access & set the display color
> > without the compiler doing a cast. gis.h likes unsigned char.
> > Should color_get.c be changed to use unsigned char?
>
> RGB_color.c isn't the only user of that function; most of the callers
> use ints.

Can we at least standardize the library functions to something we agree
is "correct" (gis.h should be canonical) and worry about what the
rabble of modules call later?

Code which deals with single colours will usually want to use int;
code which uses arrays of colours will want to use unsigned char.

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