Hamish wrote:
> > > Is a new D_number_to_RGB(int color, unsigned char &r,&g,&b) fn
> > > needed in lib/display/tran_colr.c?
Glynn:
> > Yes. E.g.:
[...]
Hamish:
> Any reason you use int for the r,g,b type? color.h defines
> {}color_rgb as unsigned char, as does gis.h's {}RGBA_Color. I'd
> like to avoid new casting issues, and for GRASS 7 would like to
> settle on one consistent type rather than the mix of two we have
> now.
Glynn:
It's unusual to have variables smaller than an "int". "short" and
"char" are normally reserved for array indices and structure fields,
where you may want to conserve space, either because you have an
array which may contain thousands of elements or a structure which
you want to fit into a 32-bit word.
If you want the values stored in variables, the variables are
typically going to be integers, so you may as well pass pointers to
the variables themselves. Note that G_str_to_color() uses ints.
If you want the values stored in an RGBA_Color, it would make more
sense to pass a pointer to the structure rather than to individual
fields.
ok, that makes sense: either use int or struct but not unsigned char
for a public function.
AFAICT, there's no fundamental reason why tran_color.c can't be
changed to use RGBA_Color instead of the color_rgb structure.
If tran_color.c is to be dumped in GRASS 7, for risk-vs-benefit reasons
I'd prefer to leave it the old way as a historical artifact. (see
below)
Hamish:
> If I pass the colors as a single RGBA_Color struct, I would like to
> have a parsing lib fn to replace D_parse_color(). It would call
> G_str_to_color() then fill the RGBA_Color struct, like
> set_last_color() in d.graph. e.g.:
> G_str_to_RGBA(char* str, RGBA_Color color);
[written before I remembered I'd already done that for d.paint.labels,
see previous email]
Ha:
> but for GRASS 7 does that belong in libgis? for now RGBA_Color is
> defined in gis.h and does not depend on external libs so I'd call
> it G_, but as libgis is already bloated I hesitate...
Gl:
I would be inclined to simply replace color_rgb with RGBA_Color.
see 6.4 vs 7 comments above and below.
Also, I'm not sure why we have lib/gis/named_colr.c with its own
table of named colours, but using floats for the R/G/B components.
I would have guessed for use in ps.map (or old GRASS 5 p.* modules),
but that uses its own table in ps/ps.map/ps_colors.c.
(see old RT bug about "the color purple" and Cedric's changes)
For now I only see G_color_*() used in d.rast, r.colors, and libgis:
$ grep -rI G_color_ * | grep -v '\.svn\|swig\|dist.i686'
All of this should really be harmonised;
Yes please.
I'm just wondering whether it's too late to do it for 6.4.
For 6.x I would suggest it is wasted effort to do so, and against the
goal of pre-release stabilization.
<2 cents>
My hope is that we release 6.3.0 soon so we can focus on new business.
ie soon after that is away cut a new releasebranch_6_4 and declare
svn-trunk to be for GRASS 7 --- why wait? AFAIK no one has plans for a
6.6 release, and if one day they do, that can be hewn from
releasebranch_6_4.
I think it is a big mistake to try and do two development branches at
the same time then try and merge changes between them.
wxGUI is the anomaly, but luckily it is a somewhat independent beast. I
would suggest continuing to work on it in releasebranch_6_4 until such
time as we tag 6.4.0pre1 and move into bugfix-only mode. At that point
we would port the GUI to GRASS 7 and move all new GUI development
there.
This presupposes that in the early days of GRASS 7 (the radical change
period) we only need a disposable barebones GUI.
Continuing development on native MS-Windows support for 6.4 is more
entwined with the C code, but I hope we are past the bulk of those
changes already and can limit the multi-branch merges to those (and
bugs of course).
It is unclear to me where new documentation and translation efforts
should be focused during that period. 6.4 branch until the GUI freezes?
</2 cents>
Hamish
____________________________________________________________________________________
Be a better friend, newshound, and
know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ
