Re: Partial fix for Bug 687418 WTS does not work with bitcmyk driver

Igor,

Yes, the log message is clearly wrong.   Trying to implement that as 
code wouldn't have passed the first test.

Russell

> From: "Igor V. Melichev" <[email protected]>
> To: <[email protected]>
> Subject: Re: [gs-code-review] Partial fix for Bug 687418 WTS does not work withbitcmyk driver
> Date: Tue, 11 May 2004 07:00:17 +0400
> 
> I believe that "max_color+1 != comp_bits[i]" isn't a correct condition.
> The right one probably is "max_color+1 != 2 ^ comp_bits[i]".
> I don't review code before the intention is clarified.
> 
> Igor.
> 
> 
> ----- Original Message ----- 
> From: "Russell Lang" <[email protected]>
> To: <[email protected]>
> Sent: Friday, May 07, 2004 4:17 PM
> Subject: [gs-code-review] Partial fix for Bug 687418 WTS does not work withbitcmyk driver
> 
> 
> > Dan,
> > 
> > Note that this patch does not change color_info.gray_index,
> > which is not set by check_device_separable and so could be
> > wrong for some devices.
> > 
> > This patch also includes some test code in zcolor.c which can (or 
> > should) be omitted.  This test code checks whether the encode_color, 
> > decode_color, separable_and_linear routines are consistent.  
> > 
> > 
> > LOG MESSAGE:
> > Prevent check_device_separable from accepting non-consecutive
> > bits as separable and linear.  Modify gx_default_encode_color
> > and gx_default_decode_color to work on devices for which
> > max_color+1 != comp_bits[i].  Partial fix for bug #687418.  
> > 
> > DETAILS:
> > One example device for which max_color+1 != comp_bits[i] is
> > the Windows 16-bit native formats with 5:6:5 bits for RGB.
> > This has max_color set to 63 (not 31) to get the best results 
> > for green, but this did not work with the default encode_color
> > function.
> > 
> > Russell Lang                   [email protected]
> > Ghostgum Software Pty Ltd      http://www.ghostgum.com.au/
> > 
> > 
> > 
> > 
> 
> 
> --------------------------------------------------------------------------------
> 
> 
> > diff -u l:/cvs/gs/src/gdevdflt.c src/gdevdflt.c
> > --- l:/cvs/gs/src/gdevdflt.c Fri May 07 09:18:46 2004
> > +++ src/gdevdflt.c Fri May 07 11:17:58 2004
> > @@ -447,8 +447,11 @@
> >       color_index >>= 1;
> >   comp_shift[i] = j;
> >   /* Determine the bit count for the colorant */
> > - for (j = 0; color_index != 0; j++)
> > + for (j = 0; color_index != 0; j++) {
> > +     if ((color_index & 1) == 0) /* check for non-consecutive bits */
> > + return;
> >       color_index >>= 1;
> > + }
> >   comp_bits[i] = j;
> >   /*
> >   * We could verify that the bit count matches the dither_grays or
> > diff -u l:/cvs/gs/src/gxcmap.c src/gxcmap.c
> > --- l:/cvs/gs/src/gxcmap.c Fri Jan 30 09:36:00 2004
> > +++ src/gxcmap.c Fri May 07 11:24:00 2004
> > @@ -52,6 +52,7 @@
> >      int             ncomps = dev->color_info.num_components;
> >      int             i, i_gray = dev->color_info.gray_index;
> >      const byte *    comp_shift = dev->color_info.comp_shift;
> > +    const byte *    comp_bits = dev->color_info.comp_bits;
> >      gx_color_index  color = 0;
> >  
> >  #ifdef DEBUG
> > @@ -61,11 +62,8 @@
> >      }
> >  #endif
> >      for (i = 0; i < ncomps; i++) {
> > -        ulong   cbits = cv[i] * ((i == i_gray ?
> > -                                 dev->color_info.max_gray :
> > -                                 dev->color_info.max_color ) + 1);
> > -
> > -        color |= (cbits / (gx_max_color_value + 1)) << comp_shift[i];
> > + color |= (gx_color_index)(cv[i] >> (gx_color_value_bits - comp_bits[i]))
> > + << comp_shift[i];
> >      }
> >      return color;
> >  }
> > @@ -76,7 +74,9 @@
> >      int                     ncomps = dev->color_info.num_components;
> >      int                     i;
> >      const byte *            comp_shift = dev->color_info.comp_shift;
> > +    const byte *            comp_bits = dev->color_info.comp_bits;
> >      const gx_color_index *  comp_mask = dev->color_info.comp_mask;
> > +    int bits, shift;
> >  
> >  #ifdef DEBUG
> >      if ( dev->color_info.separable_and_linear != GX_CINFO_SEP_LIN ) {
> > @@ -86,12 +86,13 @@
> >  #endif
> >  
> >      for (i = 0; i < ncomps; i++) {
> > -        gx_color_index  div = ( i == dev->color_info.gray_index ? 
> > -                                dev->color_info.max_gray : 
> > -                                dev->color_info.max_color ) + 1;
> > -
> > -        cv[i] = (gx_color_value)(((color & comp_mask[i]) >> comp_shift[i]) * 
> > -            (gx_max_color_value + 1) / div);
> > + bits = (color & comp_mask[i]) >> comp_shift[i];
> > + shift = gx_color_value_bits - comp_bits[i];
> > + cv[i] = bits << shift;
> > + while ((shift-=comp_bits[i]) >= 0)
> > +     cv[i] |= bits << shift;
> > + if (shift < 0)
> > +     cv[i] |= bits >> -shift;
> >      }
> >      return 0;
> >  }
> > diff -u l:/cvs/gs/src/zcolor.c src/zcolor.c
> > --- l:/cvs/gs/src/zcolor.c Thu May 06 10:58:03 2004
> > +++ src/zcolor.c Fri May 07 11:11:47 2004
> > @@ -475,6 +475,183 @@
> >      return 0;
> >  }
> >  
> > +/* 
> > + * <param1> ... <paramN> .color_test <param1> ... <paramN>
> > + *
> > + * encode and decode color to allow mapping to be tested.
> > + */
> > +int zcolor_test(i_ctx_t *i_ctx_p)
> > +{
> > +    gx_color_value cv[GX_DEVICE_COLOR_MAX_COMPONENTS];
> > +    gx_device *dev = gs_currentdevice(igs);
> > +    int ncomp = dev->color_info.num_components;
> > +    gx_color_index color;
> > +    int i;
> > +    if (ref_stack_count(&o_stack) < ncomp)
> > + return_error(e_stackunderflow);
> > +    for (i=0; i<ncomp; i++) {
> > + if (r_has_type(&osp[-i], t_real))
> > +     cv[i] = (gx_color_value)
> > + (osp[-i].value.realval * gx_max_color_value);
> > + else if (r_has_type(&osp[-i], t_integer))
> > +     cv[i] = (gx_color_value)
> > + (osp[-i].value.intval * gx_max_color_value);
> > + else
> > +     return_error(e_typecheck);
> > +    }
> > +    color = (*dev_proc(dev, encode_color)) (dev, cv);
> > +    (*dev_proc(dev, decode_color)) (dev, color, cv);
> > +    for (i=0; i<ncomp; i++)
> > +        make_real(&osp[-i], (float)cv[i] / (float)gx_max_color_value);
> > +    return 0;
> > +}
> > +
> > +/* 
> > + * <levels> .color_test_all <value0> ... <valueN>
> > + *
> > + * Test encode/decode color procedures for a range of values.
> > + * Return value with the worst error in a single component.
> > + */
> > +int zcolor_test_all(i_ctx_t *i_ctx_p)
> > +{
> > +    os_ptr                  op = osp;
> > +    gx_color_value cv[GX_DEVICE_COLOR_MAX_COMPONENTS];
> > +    gx_color_value cvout[GX_DEVICE_COLOR_MAX_COMPONENTS];
> > +    gx_color_value cvbad[GX_DEVICE_COLOR_MAX_COMPONENTS];
> > +    int counter[GX_DEVICE_COLOR_MAX_COMPONENTS];
> > +    gx_device *dev = gs_currentdevice(igs);
> > +    int ncomp = dev->color_info.num_components;
> > +    int steps;
> > +    int maxerror = 0;
> > +    int err;
> > +    int acceptable_error;
> > +    int linsep = dev->color_info.separable_and_linear == GX_CINFO_SEP_LIN;
> > +    int linsepfailed = 0;
> > +    int lsmaxerror = 0;
> > +    gx_color_index color, lscolor;
> > +    int i, j, k;
> > +    int finished = 0;
> > +
> > +    if (ncomp == 1) 
> > + acceptable_error = gx_max_color_value / dev->color_info.max_gray + 1;
> > +    else
> > + acceptable_error = gx_max_color_value / dev->color_info.max_color + 1;
> > +
> > +    if (ref_stack_count(&o_stack) < 1)
> > + return_error(e_stackunderflow);
> > +    if (!r_has_type(&osp[0], t_integer))
> > +        return_error(e_typecheck);
> > +    steps = osp[0].value.intval;
> > +    for (i=0; i<ncomp; i++) {
> > +        counter[i] = 0; 
> > + cvbad[i] = 0;
> > +    }
> > +
> > +    dprintf1("\nNumber of components = %d\n", ncomp);