[GRASS-dev] [GRASS GIS] #969: move color structs to colors.h?

GDAL · March 1, 2010, 12:07am

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@lists.osgeo.org
Type: task | Status: new
Priority: normal | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------
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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/969>
GRASS GIS <http://grass.osgeo.org>

GDAL · February 13, 2013, 10:14pm

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: normal | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------

Comment(by martinl):

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:1>
GRASS GIS <http://grass.osgeo.org>

GDAL · August 25, 2014, 8:21pm

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: normal | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------

Comment(by neteler):

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:2>
GRASS GIS <http://grass.osgeo.org>

GDAL · September 4, 2014, 2:47am

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------
Changes (by wenzeslaus):

* priority: normal => blocker

Comment:

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.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:3>
GRASS GIS <http://grass.osgeo.org>

GDAL · September 14, 2014, 2:46pm

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------

Comment(by martinl):

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?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:4>
GRASS GIS <http://grass.osgeo.org>

GDAL · September 16, 2014, 8:13pm

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------

Comment(by mmetz):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/969#comment:5>
GRASS GIS <http://grass.osgeo.org>

GDAL · October 24, 2014, 7:21am

#969: move color structs to colors.h?
------------------------------------------+---------------------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
------------------------------------------+---------------------------------

Comment(by neteler):

Is anything missing here?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:6>
GRASS GIS <http://grass.osgeo.org>

GDAL · November 8, 2014, 9:21pm

#969: move color structs to colors.h?
----------------------------------------------------+-----------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: needinfo, RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
----------------------------------------------------+-----------------------
Changes (by neteler):

* keywords: RGBA_Color, G_str_to_color() => needinfo, RGBA_Color,
G_str_to_color()

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:7>
GRASS GIS <http://grass.osgeo.org>

GDAL · November 24, 2014, 5:29pm

#969: move color structs to colors.h?
----------------------------------------------------+-----------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: needinfo, RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
----------------------------------------------------+-----------------------

Comment(by martinl):

what is status of this ticket?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:8>
GRASS GIS <http://grass.osgeo.org>

GDAL · November 24, 2014, 11:48pm

#969: move color structs to colors.h?
----------------------------------------------------+-----------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Display | Version: svn-trunk
Keywords: needinfo, RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
----------------------------------------------------+-----------------------

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:9>
GRASS GIS <http://grass.osgeo.org>

GDAL · December 8, 2014, 6:03pm

#969: move color structs to colors.h?
----------------------------------------------------+-----------------------
Reporter: hamish | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 8.0.0
Component: Display | Version: svn-trunk
Keywords: needinfo, RGBA_Color, G_str_to_color() | Platform: All
Cpu: All |
----------------------------------------------------+-----------------------
Changes (by martinl):

* milestone: 7.0.0 => 8.0.0

Comment:

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/969#comment:10>
GRASS GIS <http://grass.osgeo.org>