Re: [Groff] Patch for two grohtml bug fixes..

[Werner LEMBERG]

> > > > -  if (val > 65536) {
> > > > +  if (val > color::MAX_COLOR_VAL) {
> > > >      warning(WARN_RANGE, "%1 cannot be greater than 1", color);
> > > > -    return 65535;      // this is 0xffff
> > > > +    return color::MAX_COLOR_VAL;
> > > 
> > > The two constants replaced by color::MAX_COLOR_VAL are different.
> > > This looks OK because the old code looked `out by one' but I
> > > thought it worth raising to make sure as I haven't looked at the
> > > code at all, just the diff.
> > 
> > aaahhgg, yes a bug sorry Werner. Well spotted Ralph, this should
> > either be color::MAX_COLOR_VAL+1 or just use the literal constants as
> > Werner had previously.
> 
> Is that right?  The `f' scale modifier scales by 65536, so `1f' is
> 65536, e.g. 0x10000, not 0xffff.  MAX_COLOR_VAL is 0xffff.  The code
> above is from the get_color_element routine so we're only interested in
> colours, not other uses of `f'.
> 
> So it seems reasonable for the new code to say, as Gaius wrote,
> 
>     if (val > color::MAX_COLOR_VAL) {
>         warning(WARN_RANGE, "%1 cannot be greater than 1", color);
>         return color::MAX_COLOR_VAL;
> 
> except that the warning text is wrong.  val isn't allowed to be greater
> than *or equal to* 1 since 1 is 0x10000.
> 
> So the code looks OK but the scale of `f' seems wrong since I'd want
> `full on' for colour to be a simple `1f', not `0.9999847412109375f'.
> 
> Shouldn't `f' scale by 65535?

No.  1f=65536u.  Anything else would be extremely painful and
non-intuitive IMHO.  The question should rather be: How shall we map
the range [0.0,1.0] onto [0x0000,0xFFFF]?  My original code above
handles 0xFFFF and 0x10000 equal, i.e, the color value `1.0' is
handled as an exception for convenience.  What does e.g. Adobe do?

Scaling by 65535 might be an alternative, but I doubt it is worth the
effort.  The `loss' by converting 1.0 to 0xFFFF instead of 0x10000 is
really neglegible.


    Werner