groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ralph Corderoy
Subject: Re: [Groff] Patch for two grohtml bug fixes..
Date: Tue, 09 Oct 2001 11:35:53 +0100

Hi Gaius,

> > > Also modified is the constant 65535 to MAX_COLOR_VAL, for what it
> > > is worth.. :-)
> > > 
> > > ...
> > > 
> > > -  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?

           scale by   scale by
    f         65535      65536
    1        0xffff    0x10000
    0.5      0x7fff     0x8000
    0.125    0x1fff     0x2000

> Perhaps with hindsight the literals make more sense anyway in this
> context as 'f' is not always associated with 'color',

`f' isn't.  But this routine is just interested in colour.

Sorry if I've the wrong end of the stick.  It's early here  ;-)


Ralph.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]