move color structs to colors.h?

[hamish]

Hi,

in trunk the RGBA_Color struct has been moved into include/raster.h. As its use is more general than just raster maps (e.g. d.graph and D_symbol() use it) IMO there's no reason for it to be there and it should be moved into include/colors.h or back into gis.h.

Also, any objections to changing G_str_to_color() in trunk to use unsigned char instead of int for R,G,B? It would save some casting.

Hamish

[martinl]

include/colors.h sounds as reasonable please for RGBA_Color structure. Please go ahead.

[neteler]

Would this count as an API change? Then make it a blocker or change to GRASS GIS 8 target.

[wenzeslaus]

Replying to neteler:

Would this count as an API change? Then make it a blocker or change to GRASS GIS 8 target.

It is an API change.

Should we also change G_str_to_color()? This would be a blocker too.

[martinl]

Replying to wenzeslaus:

Replying to neteler:

Would this count as an API change? Then make it a blocker or change to GRASS GIS 8 target.

It is an API change.

Should we also change G_str_to_color()? This would be a blocker too.

does anyone planning to work on this issue?

[mmetz]

Replying to wenzeslaus:

Replying to neteler:

Would this count as an API change? Then make it a blocker or change to GRASS GIS 8 target.

It is an API change.

RGBA_Color moved to include/display.h in r62002,3 (it is only used in display functions).

Should we also change G_str_to_color()? This would be a blocker too.

IMHO no, because there is no bug reported for G_str_to_color(), using unsigned char instead of int means that a range check can no longer be done, and changing G_str_to_color() implies changing

display/d.barscale/draw_scale.c
display/d.extract/main.c
display/d.graph/do_graph.c
display/d.graph/main.c
display/d.grid/fiducial.c
display/d.labels/color.c
display/d.northarrow/draw_n_arrow.c
display/d.path/main.c
display/d.rast/display.c
display/d.thematic.area/main.c
display/d.thematic.area/plot1.c
display/d.vect.chart/main.c
display/d.vect/opt.c
display/d.vect/shape.c
general/g.cairocomp/main.c
general/g.pnmcomp/main.c
lib/cairodriver/Graph.c
lib/display/tran_colr.c
lib/imagery/iclass_signatures.c
lib/imagery/iclass_statistics.c
lib/nviz/nviz.c
lib/ogsf/Gp3.c
lib/ogsf/Gv3.c
lib/pngdriver/Graph_set.c
lib/raster/color_hist.c
misc/m.nviz.image/main.c
ps/ps.map/do_labels.c
ps/ps.map/getgrid.c
ps/ps.map/ps_outline.c
ps/ps.map/ps_vareas.c
ps/ps.map/ps_vlines.c
ps/ps.map/ps_vpoints.c
ps/ps.map/r_border.c
ps/ps.map/r_colortable.c
ps/ps.map/r_header.c
ps/ps.map/r_info.c
ps/ps.map/r_instructions.c
ps/ps.map/r_plt.c
ps/ps.map/r_text.c
ps/ps.map/r_vareas.c
ps/ps.map/r_vlegend.c
ps/ps.map/r_vlines.c
ps/ps.map/r_vpoints.c
ps/ps.map/r_wind.c
raster/r.out.png/main.c
raster/r.out.tiff/main.c
vector/v.colors/read_rgb.c
vector/v.to.rast/support.c

Thus the benefit of changing a harmless type cast comes at a cost.

[neteler]

Is anything missing here?

[martinl]

what is status of this ticket?

[martinl]

Replying to mmetz:

Should we also change G_str_to_color()? This would be a blocker too.

IMHO no, because there is no bug reported for G_str_to_color(), using unsigned char instead of int means that a range check can no longer be done, and changing G_str_to_color() implies changing

it sounds like something for G8 and not G7 (we are in beta stage)...

[martinl]

Replying to martinl:

Replying to mmetz:

Should we also change G_str_to_color()? This would be a blocker too.

IMHO no, because there is no bug reported for G_str_to_color(), using unsigned char instead of int means that a range check can no longer be done, and changing G_str_to_color() implies changing

it sounds like something for G8 and not G7 (we are in beta stage)...

I took liberty to change milestone of this ticket...