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

Dan,

> Modified Files:
> 	gdevdflt.c 
> Log Message:
> Disable the fix for 687418.  This is causing many errors in the 
regression
> tests.  These need to be investigated.

I tried testing my patches on other devices.
The patch I sent to gs-code-review needs some fixes - you can make 
the gx_default_decode_color go into an infinite loop if comp_bits is
set incorrectly.  Updated patch attached.

Another issue I found is that check_device_separable returns 
immediately if separable_and_linear is already known.  
Testing with png16m I got GX_CINFO_SEP_LIN.  check_device_separable 
returned immediately, set_linear_color_bits_mask_shift was no 
longer called, so the bits/mask/shift were never set.  
Guaranteed to cause a few regressions.

check_device_separable will mark the epswrite device as separable.  
Is this correct for a vector device?

When I did enable check_device_separable, I got a lot of crashes.
Attached is my test code
  gs -dNOPAUSE -dBATCH ctest3.ps
This iterates through nearly all devices, and writes to stderr some 
details from color_info and tests encode_color, decode_color and if 
linear and separable also gx_default_encode_color and 
gx_default_decode_color.

This needs my testing patch to src/zcolor.c

Without check-device_separable enabled, some interesting ones are:
  bjc800: 4 overlapping components, max_color=63, dither_colors=5
  pjxl: max_color=255, dither_colors=5
  devicen: max_color=255 dither_colors=1
  mswindll: marked as separable and linear but bits/mask/shift 
     not set.      This device is obsolete.

Russell



On 7 May 2004 at 22:17, Russell Lang wrote:

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


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	Mon May 10 10:52:12 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	Mon May 10 10:54:13 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,10 @@
 #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);
+	if (comp_bits[i] == 0)
+	    return gs_error_rangecheck;
+	bits = (color & comp_mask[i]) >> comp_shift[i];
+	cv[i] = bits * ( gx_max_color_value / ((1 << comp_bits[i])-1));
     }
     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 23:06:16 2004
@@ -475,6 +475,187 @@
     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(" Number of components = %d\n", ncomp);
+    dprintf1(" Depth = %d\n", dev->color_info.depth);
+    dprintf2(" max_gray = %d   dither_grays = %d\n", 
+	dev->color_info.max_gray, dev->color_info.dither_grays);
+    dprintf2(" max_color = %d   dither_colors = %d\n", 
+ 	dev->color_info.max_color, dev->color_info.dither_colors);
+    dprintf1(" polarity = %s\n", 
+      dev->color_info.polarity == GX_CINFO_POLARITY_ADDITIVE ? "Additive" :
+      dev->color_info.polarity == GX_CINFO_POLARITY_SUBTRACTIVE ?"Subtractive":
+      "Unknown");
+    dprintf1(" separable_and_linear = %s\n", 
+      linsep == GX_CINFO_SEP_LIN_NONE ? "No" :
+      linsep == GX_CINFO_SEP_LIN ? "Yes" :
+      "Unknown");
+    if (linsep) {
+	if (dev->color_info.gray_index == GX_CINFO_COMP_INDEX_UNKNOWN)
+            dprintf(" gray_index is Unknown\n");
+	else if (dev->color_info.gray_index == GX_CINFO_COMP_NO_INDEX)
+            dprintf(" gray_index is None\n");
+	else
+            dprintf1(" gray_index=%d\n", dev->color_info.gray_index);
+        dprintf("  Shift     Mask  Bits\n");
+        for (i=0; i<ncomp; i++) {
+            dprintf3("  %5d %8x  %4d\n",
+		(int)(dev->color_info.comp_shift[i]),
+		(int)(dev->color_info.comp_mask[i]),
+		(int)(dev->color_info.comp_bits[i]));
+        }
+    }
+
+    while (!finished) {
+	for (j=0; j<=steps; j++) {
+	    for (i=0; i<ncomp; i++)
+		cv[i] = counter[i] * gx_max_color_value / steps;
+	    color = (*dev_proc(dev, encode_color)) (dev, cv);
+	    if (linsep) {
+		/* Derive it the other way */
+		lscolor = gx_default_encode_color(dev, cv);
+		if ((color != lscolor) && (linsepfailed < 5)) {
+		    linsepfailed++;
+		    dprintf("Failed separable_and_linear for");
+		    for (i=0; i<ncomp; i++)
+			dprintf1(" %d", cv[i]);
+		    dprintf("\n");
+		    dprintf2("encode_color=%x  gx_default_encode_color=%x\n",
+			(int)color, (int)lscolor);
+		}
+	    }
+	    (*dev_proc(dev, decode_color)) (dev, color, cvout);
+	    for (i=0; i<ncomp; i++) {
+		err = (int)cvout[i] - (int)cv[i];
+		if (err < 0)
+		    err = -err;
+		if (err > maxerror) {
+		    maxerror = err;
+		    for (k=0; k<ncomp; k++)
+			cvbad[k] = cv[k];
+		}
+	    }
+	    if (linsep) {
+	        if (gx_default_decode_color(dev, color, cvout) < 0)
+		    lsmaxerror = gx_max_color_value;
+		for (i=0; i<ncomp; i++) {
+		    err = (int)cvout[i] - (int)cv[i];
+		    if (err < 0)
+			err = -err;
+		    if (err > lsmaxerror) {
+			lsmaxerror = err;
+		    }
+		}
+	    }
+	    counter[0] += 1;
+	}
+	counter[0] = 0;
+	i = 1;
+	while (i < ncomp) {
+	    counter[i] += 1;
+	    if (counter[i] > steps) {
+		counter[i] = 0;
+		i++;
+	    }
+	    else
+		break;
+	}
+	if (i >= ncomp)
+	    finished = 1;
+    }
+
+    dprintf2(" Maximum error %g %s\n", 
+	(float)maxerror / (float)gx_max_color_value,
+	maxerror <= acceptable_error ? "is Ok" :
+	maxerror <= 3*acceptable_error/2 ? "is POOR" : "FAILED");
+
+    if (linsep)
+      dprintf2(" Maximum linear_and_separable error %g %s\n", 
+	(float)lsmaxerror / (float)gx_max_color_value,
+	lsmaxerror <= acceptable_error ? "is Ok" :
+	lsmaxerror <= 3*acceptable_error/2 ? "is POOR" : "FAILED");
+    dprintf("\n");
+
+    /* push worst value */
+    push(ncomp-1);
+    op -= ncomp - 1;
+    for (i=0; i<ncomp; i++)
+        make_real(op+i, (float)cvbad[i] / (float)gx_max_color_value);
+
+    return 0;
+}
+
 /* ------ Initialization procedure ------ */
 
 const op_def    zcolor_op_defs[] = 
@@ -496,6 +677,8 @@
     { "1%zcolor_remap_one_signed_finish", zcolor_remap_one_signed_finish },
     { "0%zcolor_reset_transfer", zcolor_reset_transfer },
     { "0%zcolor_remap_color", zcolor_remap_color },
+    { "0.color_test", zcolor_test },
+    { "1.color_test_all", zcolor_test_all },
 
     /* high level device support */
     { "0.includecolorspace", zincludecolorspace },

_______________________________________________
gs-code-review mailing list
[email protected]
http://www.ghostscript.com/mailman/listinfo/gs-code-review