Bug 692235 - Rendering PDF to 8bit colors shows bad colors in display device
Summary: Rendering PDF to 8bit colors shows bad colors in display device
Status: RESOLVED INVALID
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: MS Windows Display Driver (show other bugs)
Version: master
Hardware: PC Windows 7
: P4 enhancement
Assignee: Michael Vrhel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-25 21:03 UTC by Martin Osieka
Modified: 2019-01-25 18:48 UTC (History)
3 users (show)

See Also:
Customer:
Word Size: ---


Attachments
PDF rendered to 8bit color (79.42 KB, image/png)
2011-05-26 16:28 UTC, Martin Osieka
Details
png256.png (39.40 KB, image/png)
2011-05-26 17:28 UTC, Ray Johnston
Details
01Fassaden.pdf (439.51 KB, application/pdf)
2011-05-31 11:08 UTC, Robin Watts
Details
colorcir.ps rendered using RGBK 64rgb+32g (362.71 KB, image/png)
2011-06-08 14:14 UTC, Martin Osieka
Details
colorcir.ps rendered after patch has been applied (484.92 KB, image/png)
2011-06-08 14:20 UTC, Martin Osieka
Details
0001-Fix-bug-692235.patch (12.67 KB, patch)
2011-07-21 19:52 UTC, Robin Watts
Details | Diff
GS 8.70 -dDisplayFormat=16#30801 (54.23 KB, image/png)
2011-07-24 04:49 UTC, Russell Lang
Details
GS 9.03 before patch of 23 July (148.68 KB, image/png)
2011-07-24 04:51 UTC, Russell Lang
Details
GS 9.04 pre-release after the 23 July 2011 patch (47.64 KB, image/png)
2011-07-24 04:53 UTC, Russell Lang
Details
GS 9.04 pre-release after the 23 July 2011 patch, via GSview (26.64 KB, image/png)
2011-07-24 04:54 UTC, Russell Lang
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Osieka 2011-05-25 21:03:25 UTC
Tested with the "01 Fassaden.pdf" I provided earlier.

Green looks mostly yellow (see attachement).

From looking to the source I assume you are just using 64 of 256 colors.
But even if my assumption is true, I would expect a better result of the color quantization.

Martin
Comment 1 Martin Osieka 2011-05-26 16:28:17 UTC
Created attachment 7536 [details]
PDF rendered to 8bit color
Comment 2 Ray Johnston 2011-05-26 17:19:58 UTC
To help others in the future, please put a bug number not just some bogus
name -- I had to search through your bugs to find the one with the file.

For reference, it is http://bugs.ghostscript.com/show_bug.cgi?id=692227

Please provide the complete command line used to produce the attached png.

When I run to png16m (8 bit per component) it looks fine, matching Adobe.

Please reopen the bug after adding the information. Closing as WORKSFORME
for now (in case we don't get a response)

Better bug submissions will help us not waste time.
Comment 3 Ray Johnston 2011-05-26 17:28:18 UTC
Created attachment 7537 [details]
png256.png

BTW, in case you meant 8-bit TOTAL color created with the png256 device, this
attachment is my result. Still WORKSFORME. The command line I used was:

gswin32c -r40 -sDEVICE=png256 -o png256.png c:/temp/Bug692227.pdf
Comment 4 Martin Osieka 2011-05-26 18:42:08 UTC
(In reply to comment #3)
> Created an attachment (id=7537) [details]
> png256.png
> 
> BTW, in case you meant 8-bit TOTAL color created with the png256 device, this
> attachment is my result. Still WORKSFORME. The command line I used was:
> 
> gswin32c -r40 -sDEVICE=png256 -o png256.png c:/temp/Bug692227.pdf

Used commandline 

gswin32.exe -dTextAlphaBits=4 -dGraphicsAlphaBits=4 -dDisplayFormat=3344385
-r300x300 "01 Fassaden.PDF"

which shows the colors as in attachement "PDF rendered to 8bit color.png". I copied and resized the result to keep the file small.
Comment 5 Martin Osieka 2011-05-26 19:23:21 UTC
-dDisplayFormat=3344385 is the following combination of format bits

DISPLAY_DEPTH_8 
| DISPLAY_COLORS_NATIVE
| DISPLAY_ROW_ALIGN_4 
| DISPLAY_LITTLEENDIAN 
| DISPLAY_BOTTOMFIRST;

I tested this first using dll and gsapi_set_display_callback() 
and second on the commandline using gswin32.exe.

The bug containing mentioned pdf "This is the real pdf" attachment is 692227.
Comment 6 Robin Watts 2011-05-31 11:08:46 UTC
Created attachment 7551 [details]
01Fassaden.pdf

To make life simpler, a restatement of the bug.

Using the attached file, the following command

  gswin32.exe -r300 01Fassaden.pdf

renders fine. Forcing this to 256 colors using:

  gswin32.exe -r300 -dDisplayFormat=16#330801 -r300 01Fassaden.pdf

results in an unacceptable color cast (from greeny/brown -> yellow) and loss of the background gradient. This is visible at lower resolutions too, but becomes more evident at higher ones.
Comment 7 Robin Watts 2011-05-31 11:28:33 UTC
0x330801  = 
   (1<< 0) = DISPLAY_COLORS_NATIVE |
   (1<<11) = DISPLAY_DEPTH_8       |
   (1<<16) = DISPLAY_LITTLE_ENDIAN |
   (1<<17) = DISPLAY_BOTTOM_FIRST  |
   (3<<20) = DISPLAY_ROW_ALIGN_4

DISPLAY_ROW_ALIGN_4 can be removed without causing any differences (as you'd expect).

Using DISPLAY_COLORS_RGB:

  gswin32.exe -r300 -dDisplayFormat=16#30804 -r300 01Fassaden.pdf

looks perfect, as does using DISPLAY_DEPTH_16:

  gswin32.exe -r300 -dDisplayFormat=16#32001 -r300 01Fassaden.pdf
Comment 8 Robin Watts 2011-05-31 12:20:00 UTC
Digging into the code, it looks like using DISPLAY_DEPTH_8 with DISPLAY_COLORS_RGB actually uses 8 bit per *component*, rather than 8 bit per colour, so it's not surprising it looks good.

Using DISPLAY_DEPTH_8 with DISPLAY_COLORS_NATIVE uses a 96 colour palette; 2 bits each for R, G and B, with 63 grey levels added in. This is listed in the code as "8 bit SVGA palette", but I can find no explanation of where this was derived from.

It seems like a fairly poor choice of palette; using 96 out of a possible 256 entries is never going to give us a decent color rendition.

I'm not sure we really consider 8bit colour output a priority any more, but if we did, we could consider alternative output palettes.

Acorn used an 8 bit system: RRGGBBTT (where TT is 'tint', resulting in the following 4 bit values for red, green and blue, RRTT, GGTT, BBTT).

Alternatively, using rudimentary arithmetic coding we can provide 6 shades of R, 6 shades of G, 6 shades of B, and combine then as R+6*(G+6*B), to give us 216 colours, leaving 40 shades available for greyscales?

Or we can exploit the fact that the human eye is much more sensitive to green, and offer 6 shades of R and B, and 7 of green (using 252 colors, leaving 4 more for greys).

I suspect though that outputting to 8 bit RGB is so rarely used these days that this is a case we really don't care about. Martin, can you tell us why this is important to you please ? Thanks.
Comment 9 Ray Johnston 2011-05-31 16:06:19 UTC
Since the png256 looks OK, just using the color method of png256 would be
a good start (and allow lifting code). The code in base/gdevpccm.c is:

/* ------ SVGA 8-bit color mapping ------ */
/*
 * For 8-bit color, we use a 6x6x6 "cube".  This only provides 216
 * different colors.  The halftoning logic assumes that we have the same
 * number of shades of each color.  Thus asymetric cubes like 8x8x4 or
 * 7x7x5 do not work properly.
 */

gx_color_index
pc_8bit_map_rgb_color(gx_device * dev, const gx_color_value cv[])
{
    gx_color_value r, g, b;
    uint rv, gv;
    r = cv[0]; g = cv[1]; b = cv[2];
    rv = r / (gx_max_color_value / 6 + 1);
    gv = g / (gx_max_color_value / 6 + 1);

    return (gx_color_index)
         (rv * 6 + gv) * 6 + b / (gx_max_color_value / 6 + 1);
}
Comment 10 Martin Osieka 2011-06-01 14:16:40 UTC
(In reply to comment #8)
> Martin, can you tell us why this is important to you please ? Thanks.

I'm not saying it is quite important for me (this is why I marked the bug as enhancement in lack of a better state).

It's just that ghostscript docu tells that it is possible.

I would use it to generate thumbnails.

In addition I'm doing a plugin replacement for 3 different tools (one is Irfanview) which allows to view ps/eps/pdf. In this plugin you are allowed to render to

B/W, 16 grays, 256 grays, 16 colors, 256 colors, 32768 colors, RGB and ARGB.

So all simple color spaces which are defined by ghostscript. There is the general problem for 16 and 256 colors that no palette is defined. You have to dig in the ghostscript code to get the one used in the demo.

To use the 6x6x6 cube as proposed by Ray would be a good solution for my purpose. You should also define this in documentation.

I would not drop the support as long as you are offering something simular for png and other output.
Comment 11 Martin Osieka 2011-06-02 08:22:50 UTC
I was not able to figure out what is the problem with 8-bit (256) colors but for 4-bit (16) colors.

diff --git a/gs/base/gdevdsp.c b/gs/base/gdevdsp.c
index 87b8051..5f85ed7 100644
--- a/gs/base/gdevdsp.c
+++ b/gs/base/gdevdsp.c
@@ -1670,7 +1670,7 @@ display_set_color_format(gx_device_display *ddev, int nFormat)
                     break;
                 case DISPLAY_DEPTH_4:
                     /* 4bit/pixel VGA color */
-                    set_color_info(&dci, DISPLAY_MODEL_RGB, 3, 4, 3, 2);
+                    set_color_info(&dci, DISPLAY_MODEL_RGB, 3, 4, 1, 1);
                     dci.separable_and_linear = GX_CINFO_SEP_LIN_NONE;
                     set_rgb_color_procs(pdev, display_map_rgb_color_device4,
                                                 display_map_color_rgb_device4);

This is because the driver uses a 2x2x2 color cube so the maxcolor parameter of set_color_info() must be 1.

The color table must be

00000 -      -      -       -      -      -      -
-     0000FF 00FF00 00FFFF  FF0000 FF00FF FFFF00 FFFFFF

I'm telling this because dwimg.c/image_color() creates a different one for 4-bit than pc_4bit_map_rgb_color() provides. So you should modify this file too.

I was able to get a good result by using DISPLAY_MODEL_RGB instead of DISPLAY_MODEL_RGBK for 8-bit using a 6x6x6 color cube. But I'm not sure if you like to do this change. And up to now I have no idea why DISPLAY_MODEL_RGBK is not working.
Comment 12 Martin Osieka 2011-06-02 12:39:32 UTC
Here is a complete diff to remove RGBK model from all files. I'm guissing that RGBK has been introduced in 2004 but nobody maintained it later on. So I would say, easiest is to remove RGBK completely and replace the 8-bit code with the code used for 4-bit (but using a 6x6x6 cube).

Martin


diff --git a/gs/base/gdevdsp.c b/gs/base/gdevdsp.c
index 87b8051..0e6ea3f 100644
--- a/gs/base/gdevdsp.c
+++ b/gs/base/gdevdsp.c
@@ -372,66 +372,19 @@ display_map_color_rgb_device4(gx_device * dev, gx_color_index color,
 }

 /* DISPLAY_COLORS_NATIVE, 8bit/pixel */
-/* Map a r-g-b-k color to a color code */
+/* Map a r-g-b color to a color code */
 static gx_color_index
-display_encode_color_device8(gx_device * dev, const gx_color_value cv[])
+display_map_rgb_color_device8(gx_device * dev, const gx_color_value cv[])
 {
-    /* palette of 96 colors */
-    /* 0->63 = 00RRGGBB, 64->95 = 010YYYYY */
-    gx_color_value r = cv[0];
-    gx_color_value g = cv[1];
-    gx_color_value b = cv[2];
-    gx_color_value k = cv[3]; /* 0 = black */
-    if ((r == 0) && (g == 0) && (b == 0)) {
-    if (k > 0) {
-        /* The RGB->RGBK color mapping shouldn't generate this. */
-        r = ((r+k) > gx_max_color_value) ? gx_max_color_value :
-            (gx_color_value)(r+k);
-        g = ((g+k) > gx_max_color_value) ? gx_max_color_value :
-            (gx_color_value)(g+k);
-        b = ((b+k) > gx_max_color_value) ? gx_max_color_value :
-            (gx_color_value)(b+k);
-    }
-    r = ((r >> (gx_color_value_bits - 3)) + 1) >> 1;
-    if (r > 0x3)
-        r = 0x3;
-    g = ((g >> (gx_color_value_bits - 3)) + 1) >> 1;
-    if (g > 0x3)
-        g = 0x3;
-    b = ((b >> (gx_color_value_bits - 3)) + 1) >> 1;
-    if (b > 0x3)
-        b = 0x3;
-    return (r << 4) + (g << 2) + b;
+    return pc_8bit_map_rgb_color(dev, cv);
 }

-/* Map a color code to r-g-b-k. */
+/* Map a color code to r-g-b. */
 static int
-display_decode_color_device8(gx_device * dev, gx_color_index color,
-                 gx_color_value prgb[4])
+display_map_color_rgb_device8(gx_device * dev, gx_color_index color,
+                 gx_color_value prgb[3])
 {
-    gx_color_value one;
-    /* palette of 96 colors */
-    /* 0->63 = 00RRGGBB, 64->95 = 010YYYYY */
-    if (color < 64) {
-        one = (gx_color_value) (gx_max_color_value / 3);
-        prgb[0] = (gx_color_value) (((color >> 4) & 3) * one);
-        prgb[1] = (gx_color_value) (((color >> 2) & 3) * one);
-        prgb[2] = (gx_color_value) (((color) & 3) * one);
-        prgb[3] = 0;
-    }
-    else if (color < 96) {
-        one = (gx_color_value) (gx_max_color_value / 31);
-        prgb[0] = prgb[1] = prgb[2] = 0;
-        prgb[3] = (gx_color_value) ((color & 0x1f) * one);
-    }
-    else {
-        prgb[0] = prgb[1] = prgb[2] = prgb[3] = 0;
-    }
+    pc_8bit_map_color_rgb(dev, color, prgb);
     return 0;
 }

@@ -1459,9 +1412,8 @@ display_set_separations(gx_device_display *dev)
 typedef enum DISPLAY_MODEL_e {
     DISPLAY_MODEL_GRAY=0,
     DISPLAY_MODEL_RGB=1,
-    DISPLAY_MODEL_RGBK=2,
-    DISPLAY_MODEL_CMYK=3,
-    DISPLAY_MODEL_SEP=4
+    DISPLAY_MODEL_CMYK=2,
+    DISPLAY_MODEL_SEP=3
 } DISPLAY_MODEL;

 /*
@@ -1491,11 +1443,6 @@ set_color_info(gx_device_color_info * pdci, DISPLAY_MODEL model,
             pdci->cm_name = "DeviceRGB";
             pdci->gray_index = GX_CINFO_COMP_NO_INDEX;
             break;
-        case DISPLAY_MODEL_RGBK:
-            pdci->polarity = GX_CINFO_POLARITY_ADDITIVE;
-            pdci->cm_name = "DeviceRGBK";
-            pdci->gray_index = 3;
-            break;
         case DISPLAY_MODEL_CMYK:
             pdci->polarity = GX_CINFO_POLARITY_SUBTRACTIVE;
             pdci->cm_name = "DeviceCMYK";
@@ -1562,20 +1509,6 @@ set_rgb_color_procs(gx_device * pdev,

 /*
  * This is an utility routine to set up the color procs for the display
- * device.  This routine is used when the display device is RGBK.
- */
-static void
-set_rgbk_color_procs(gx_device * pdev,
-        dev_t_proc_encode_color((*encode_color), gx_device),
-        dev_t_proc_decode_color((*decode_color), gx_device))
-{
-    set_color_procs(pdev, encode_color, decode_color,
-        gx_default_DevRGBK_get_color_mapping_procs,
-        gx_default_DevRGBK_get_color_comp_index);
-}
-
-/*
- * This is an utility routine to set up the color procs for the display
  * device.  This routine is used when the display device is CMYK.
  */
 static void
@@ -1670,17 +1603,17 @@ display_set_color_format(gx_device_display *ddev, int nFormat)
                     break;
                 case DISPLAY_DEPTH_4:
                     /* 4bit/pixel VGA color */
-                    set_color_info(&dci, DISPLAY_MODEL_RGB, 3, 4, 3, 2);
+                    set_color_info(&dci, DISPLAY_MODEL_RGB, 3, 4, 1, 1);
                     dci.separable_and_linear = GX_CINFO_SEP_LIN_NONE;
                     set_rgb_color_procs(pdev, display_map_rgb_color_device4,
                                                 display_map_color_rgb_device4);
                     break;
                 case DISPLAY_DEPTH_8:
                     /* 8bit/pixel 96 color palette */
-                    set_color_info(&dci, DISPLAY_MODEL_RGBK, 4, 8, 31, 3);
+                    set_color_info(&dci, DISPLAY_MODEL_RGB, 3, 8, 5, 5);
                     dci.separable_and_linear = GX_CINFO_SEP_LIN_NONE;
-                    set_rgbk_color_procs(pdev, display_encode_color_device8,
-                                                display_decode_color_device8);
+                    set_rgb_color_procs(pdev, display_map_rgb_color_device8,
+                                                display_map_color_rgb_device8);
                     break;
                 case DISPLAY_DEPTH_16:
                     /* Windows 16-bit display */
diff --git a/gs/base/gxcmap.c b/gs/base/gxcmap.c
index a904cc9..25d7b09 100644
--- a/gs/base/gxcmap.c
+++ b/gs/base/gxcmap.c
@@ -233,43 +233,6 @@ cmyk_cs_to_rgb_cm(gx_device * dev, frac c, frac m, frac y, frac k, frac out[])
 }

 static void
-gray_cs_to_rgbk_cm(gx_device * dev, frac gray, frac out[])
-{
-    out[0] = out[1] = out[2] = frac_0;
-    out[3] = gray;
-}
-
-static void
-rgb_cs_to_rgbk_cm(gx_device * dev, const gs_imager_state *pis,
-                                  frac r, frac g, frac b, frac out[])
-{
-    if ((r == g) && (g == b)) {
-        out[0] = out[1] = out[2] = frac_0;
-        out[3] = r;
-    }
-    else {
-        out[0] = r;
-        out[1] = g;
-        out[2] = b;
-        out[3] = frac_0;
-    }
-}
-
-static void
-cmyk_cs_to_rgbk_cm(gx_device * dev, frac c, frac m, frac y, frac k, frac out[])
-{
-    frac rgb[3];
-    if ((c == frac_0) && (m == frac_0) && (y == frac_0)) {
-        out[0] = out[1] = out[2] = frac_0;
-        out[3] = frac_1 - k;
-    }
-    else {
-        color_cmyk_to_rgb(c, m, y, k, NULL, rgb, dev->memory);
-        rgb_cs_to_rgbk_cm(dev, NULL, rgb[0], rgb[1], rgb[2], out);
-    }
-}
-
-static void
 gray_cs_to_cmyk_cm(gx_device * dev, frac gray, frac out[])
 {
     out[0] = out[1] = out[2] = frac_0;
@@ -331,10 +294,6 @@ static const gx_cm_color_map_procs DeviceCMYK_procs = {
     gray_cs_to_cmyk_cm, rgb_cs_to_cmyk_cm, cmyk_cs_to_cmyk_cm
 };

-static const gx_cm_color_map_procs DeviceRGBK_procs = {
-    gray_cs_to_rgbk_cm, rgb_cs_to_rgbk_cm, cmyk_cs_to_rgbk_cm
-};
-
 /*
  * These are the default handlers for returning the list of color space
  * to color model conversion routines.
@@ -358,12 +317,6 @@ gx_default_DevCMYK_get_color_mapping_procs(const gx_device * dev)
 }

 const gx_cm_color_map_procs *
-gx_default_DevRGBK_get_color_mapping_procs(const gx_device * dev)
-{
-    return &DeviceRGBK_procs;
-}
-
-const gx_cm_color_map_procs *
 gx_error_get_color_mapping_procs(const gx_device * dev)
 {
     /*
@@ -436,23 +389,6 @@ gx_default_DevCMYK_get_color_comp_index(gx_device * dev, const char * pname,
         return -1;                 /* Indicate that the component name is "unknown" */
 }

-/* Default color component to index for a DeviceRGBK color model */
-int
-gx_default_DevRGBK_get_color_comp_index(gx_device * dev, const char * pname,
-                                            int name_size, int component_type)
-{
-    if (compare_color_names(pname, name_size, "Red"))
-        return 0;
-    if (compare_color_names(pname, name_size, "Green"))
-        return 1;
-    if (compare_color_names(pname, name_size, "Blue"))
-        return 2;
-    if (compare_color_names(pname, name_size, "Black"))
-        return 3;
-    else
-        return -1;                 /* Indicate that the component name is "unknown" */
-}
-
 /* Default color component to index for an unknown color model */
 int
 gx_error_get_color_comp_index(gx_device * dev, const char * pname,
diff --git a/gs/base/gxcmap.h b/gs/base/gxcmap.h
index aeff691..7624a28 100644
--- a/gs/base/gxcmap.h
+++ b/gs/base/gxcmap.h
@@ -236,7 +236,6 @@ dev_proc_get_color_comp_index(gx_error_get_color_comp_index);
 dev_proc_get_color_comp_index(gx_default_DevGray_get_color_comp_index);
 dev_proc_get_color_comp_index(gx_default_DevRGB_get_color_comp_index);
 dev_proc_get_color_comp_index(gx_default_DevCMYK_get_color_comp_index);
-dev_proc_get_color_comp_index(gx_default_DevRGBK_get_color_comp_index);

 /*
  * These are the default routines for getting the color space conversion
@@ -246,7 +245,6 @@ dev_proc_get_color_mapping_procs(gx_error_get_color_mapping_procs);
 dev_proc_get_color_mapping_procs(gx_default_DevGray_get_color_mapping_procs);
 dev_proc_get_color_mapping_procs(gx_default_DevRGB_get_color_mapping_procs);
 dev_proc_get_color_mapping_procs(gx_default_DevCMYK_get_color_mapping_procs);
-dev_proc_get_color_mapping_procs(gx_default_DevRGBK_get_color_mapping_procs);

 /*
  * These are the default routines for converting a colorant value list
diff --git a/gs/cups/libs/cups/ppd.c b/gs/cups/libs/cups/ppd.c
index 8d86f0b..1b2cdd4 100644
--- a/gs/cups/libs/cups/ppd.c
+++ b/gs/cups/libs/cups/ppd.c
@@ -1622,8 +1622,6 @@ ppdOpen2(cups_file_t *fp)         /* I - File to read from */
           ppd->colorspace = PPD_CS_CMYK;
        else if (!strcmp(string, "RGB"))
           ppd->colorspace = PPD_CS_RGB;
-       else if (!strcmp(string, "RGBK"))
-          ppd->colorspace = PPD_CS_RGBK;
        else if (!strcmp(string, "N"))
           ppd->colorspace = PPD_CS_N;
        else
diff --git a/gs/cups/libs/cups/ppd.h b/gs/cups/libs/cups/ppd.h
index aa54ac9..72a759e 100644
--- a/gs/cups/libs/cups/ppd.h
+++ b/gs/cups/libs/cups/ppd.h
@@ -88,7 +88,6 @@ typedef enum ppd_cs_e                 /**** Colorspaces ****/
   PPD_CS_CMY,                          /* CMY colorspace */
   PPD_CS_GRAY = 1,                     /* Grayscale colorspace */
   PPD_CS_RGB = 3,                      /* RGB colorspace */
-  PPD_CS_RGBK,                         /* RGBK (K = gray) colorspace */
   PPD_CS_N                             /* DeviceN colorspace */
 } ppd_cs_t;
Comment 13 Russell Lang 2011-06-08 13:07:12 UTC
I can't remember the exact details, but the code with
/* palette of 96 colors */
/* 0->63 = 00RRGGBB, 64->95 = 010YYYYY */
gave a better result with example/colorcir.ps.  The colour circles would use a crude palette, but the grey circles would be much smoother.
It used a different colour cube when dealing with setgray than with colours.
Comment 14 Martin Osieka 2011-06-08 14:14:22 UTC
Created attachment 7575 [details]
colorcir.ps rendered using RGBK 64rgb+32g
Comment 15 Martin Osieka 2011-06-08 14:19:10 UTC
(In reply to comment #13)
> I can't remember the exact details, but the code with
> /* palette of 96 colors */
> /* 0->63 = 00RRGGBB, 64->95 = 010YYYYY */
> gave a better result with example/colorcir.ps.  The colour circles would use a
> crude palette, but the grey circles would be much smoother.
> It used a different colour cube when dealing with setgray than with colours.

I'm not saying that RGBK would not be better but have a look to the results I am getting for colorcir.ps. Maybe I'm doing something wrong but looking to the ghostscript source I'm guessing something is broken there for RGBK.
Comment 16 Martin Osieka 2011-06-08 14:20:54 UTC
Created attachment 7576 [details]
colorcir.ps rendered after patch has been applied
Comment 17 Robin Watts 2011-07-21 19:52:16 UTC
Created attachment 7698 [details]
0001-Fix-bug-692235.patch

A git format patch to solve the bug. To apply use:

  git am 0001-Fix-bug-692235.patch

This changes the RGBK space to be a 6x6x6 RGB cube, supplemented by 40 greyscales.

The results are significantly better than the existing version.

Unless anyone objects, I'll commit this fairly soon, so please test and speak up!
Comment 18 Robin Watts 2011-07-23 10:01:16 UTC
After consultantion with Russell, my patch has been committed as:

commit 7f5d3d7b298bbabc13a484e38e749d572d817bc8
Author: Robin Watts <Robin.Watts@artifex.com>
Date:   Thu Jul 21 20:42:33 2011 +0100

    Fix bug 692235: Rendering to 8bit colors shows bad colors.

    The display device uses an RGBK space for 8 bit rendering. Currently
    this corresponds to a palette of 96 colors; 2 bits each for R,G,B,
    supplemented by 16 greys.

    This review alters it to use a 6x6x6 RGB cube, supplemented by 40 grey
    levels.

    In fact, this is slightly wasteful as we repeat the black and white
    representations as color and as greyscale, but it's a huge improvement
    on what we had before.

    No cluster differences expected as we don't test the display device.

Thanks.
Comment 19 Russell Lang 2011-07-24 01:20:58 UTC
The palette is not passed between Ghostscript and callers, so any change to the palette will result in incorrect rendering.  This will affect GSview in the infrequently used 8-bit display mode.
Is there another fix to make it work as it previously did, without changing the 96 colour palette?
Comment 20 Russell Lang 2011-07-24 04:49:57 UTC
Created attachment 7706 [details]
GS 8.70 -dDisplayFormat=16#30801

This is how the display device should work in 8-bit native mode, using a fixed palette of 96 colours.
Comment 21 Russell Lang 2011-07-24 04:51:33 UTC
Created attachment 7707 [details]
GS 9.03 before patch of 23 July

This is how GS 9.03 works before the patch of 23 July 2011.
The greys of the colour circule are halftoned, when there should be sufficient levels (32) to prevent half toning.
Comment 22 Russell Lang 2011-07-24 04:53:20 UTC
Created attachment 7708 [details]
GS 9.04 pre-release after the 23 July 2011 patch

Greys are now correct.
However the colours are wrong an appear as greys.
Comment 23 Russell Lang 2011-07-24 04:54:41 UTC
Created attachment 7709 [details]
GS 9.04 pre-release after the 23 July 2011 patch, via GSview

The palette has been changed, so anything existing code that uses -dDisplayFormat=16#30801 will now be wrong, as shown by this output seen from GSview.
Comment 24 Robin Watts 2011-07-24 16:55:52 UTC
After some more discussion with Russell, I've pulled the previous patch, reopened this bug, and updated the title slightly to reflect that it's only the display device that is affected.

The commit reverting the patch is 961a9a9.

Why have I reverted the patch?

Well, firstly, it tests out OK for me, it gave Russell problems (he got just greyscale in colorcir.ps).

Secondly, it will break any code (such as GSview) that expects the standard 8 bit palette.

Thirdly, a vanishingly small number of people will be using 8 bit output in the display device.

Having done the work on the patch however, it has caused me to question the original bug report; the complaint was that gs was unsuitable for producing 8 bit color output images, say for use as thumbnails. This is NOT the case, as you would presumably be using the png256 or similar devices, rather than the display device to achieve such things.

Those devices use the 6x6x6 color cube rather than the 96 color palette that the display device does, and so will produce much nicer results.

A better question might be "why does the display device use a non-standard 8 bit palette", and the answer would be "historical reasons".

Martin, as the original bug reporter, can you comment please. Why do you specifically need the display device to work in 8 bits? (Clearly it would be nice if it worked better than it did, but is it actually essential for you?)
Comment 25 Martin Osieka 2011-07-25 17:43:41 UTC
> Martin, as the original bug reporter, can you comment please. Why do you
> specifically need the display device to work in 8 bits? (Clearly it would be
> nice if it worked better than it did, but is it actually essential for you?)

It's not essential. 

I'm using the display device because this allows me to do the job in memory. For sure there are always workarounds. So it is on you to decide if you like to break the compatibility to offer better results.

Proposal: Add a switch to allow the user of the api to use the new colortable. This would be great for my needs. Then it would really be an enhancement.

But at the end there are much more important issues like that you can not use ghostscript from more than one thread. But I guess this is not on your list...
Comment 26 Michael Vrhel 2019-01-25 18:48:16 UTC
Closing after discussion with Robin.  The display device is behaving as specified.   If a free user wants a device to work "in memory" with a custom palette he/she can go ahead and write such a device.  There is plenty of documentation on how to write devices for ghostscript.