move color structs to colors.h?

[grass-svn2git]

Reported by hamish on 1 Mar 2010 00:07 UTC 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

GRASS GIS version and provenance

svn-trunk

Migrated-From: https://trac.osgeo.org/grass/ticket/969

[grass-svn2git]

Comment by @landam on 13 Feb 2013 22:14 UTC include/colors.h sounds as reasonable please for RGBA_Color structure. Please go ahead.

[grass-svn2git]

Comment by @neteler on 25 Aug 2014 20:21 UTC Would this count as an API change? Then make it a blocker or change to GRASS GIS 8 target.

[grass-svn2git]

Comment by @wenzeslaus on 4 Sep 2014 02:47 UTC Replying to [comment:2 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.

[grass-svn2git]

Comment by @landam on 14 Sep 2014 14:46 UTC Replying to [comment:3 wenzeslaus]:

Replying to [comment:2 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?

[grass-svn2git]

Comment by @metzm on 16 Sep 2014 20:13 UTC Replying to [comment:3 wenzeslaus]:

Replying to [comment:2 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 https://trac.osgeo.org/grass/changeset/62002,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.

[grass-svn2git]

Comment by @neteler on 24 Oct 2014 07:21 UTC Is anything missing here?

[grass-svn2git]

Modified by @neteler on 8 Nov 2014 21:21 UTC

[grass-svn2git]

Comment by @landam on 24 Nov 2014 17:29 UTC what is status of this ticket?

[grass-svn2git]

Comment by @landam on 24 Nov 2014 23:48 UTC Replying to [comment:5 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)...

[grass-svn2git]

Comment by @landam on 8 Dec 2014 18:03 UTC Replying to [comment:9 martinl]:

Replying to [comment:5 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...