Bug 695081 - grayscale images sometimes show in color
Summary: grayscale images sometimes show in color
Status: UNCONFIRMED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: X Display Driver (show other bugs)
Version: 9.10
Hardware: PC Linux
: P4 normal
Assignee: Chris Liddell (chrisl)
URL:
Keywords:
Depends on:
Blocks: 691432
  Show dependency tree
 
Reported: 2014-03-01 23:47 UTC by William Bader
Modified: 2023-05-23 15:59 UTC (History)
3 users (show)

See Also:
Customer:
Word Size: ---


Attachments
File to reproduce the problem (383 bytes, application/postscript)
2014-03-01 23:47 UTC, William Bader
Details
screen capture showing the problem (14.00 KB, image/png)
2014-03-01 23:49 UTC, William Bader
Details
fix x gray palette problem (1.72 KB, patch)
2015-05-13 08:45 UTC, Hin-Tak Leung
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description William Bader 2014-03-01 23:47:15 UTC
Created attachment 10732 [details]
File to reproduce the problem

It was reported on http://savannah.gnu.org/bugs/?41737 that "gv -grayscale" sometimes shows grayscale images in color. Some grayscale values become red.
gv simply passes the grayscale flag to gs, see SetProperty() in gv-3.7.4/src/Ghostview.c
The real work is done by gdev_x_setup_colors() in gs9.10/devices/gdevxcmp.c
I put debug code in gdevxcmp.c, and setup_cube() seems to be working correctly.
Is the problem related to the grayscale comment in x_strip_tile_rectangle() in devices/gdevx.c?
I am attaching a simple ps file that reproduces the problem and a screen capture of the results with gs -grayscale.
I don't need this to work.  I just got curious after seeing the posting on the gv mailing list.
William
Comment 1 William Bader 2014-03-01 23:49:05 UTC
Created attachment 10733 [details]
screen capture showing the problem

screen capture from running
  gv -grayscale grayscale.ps
Comment 2 Hin-Tak Leung 2014-03-02 19:37:15 UTC
This looks like a gv bug, really, or an xtoolkit bug, really... gv seems to do grayscale switch via setting palette to XtPaletteGrayscale.
If you run the gs command line directly (from just doing ps), gs works correctly.
Comment 3 William Bader 2014-03-02 22:41:59 UTC
I think that the fact that the file works directly with gs is evidence that the problem is in the way that gs handles the grayscale flag.  When the file works with gs directly, it means that the display has enough colors to build the grayscale ramp.  When the file works with gv with no options, it means that gv is building a good display window.  Given that gs and gv with no options work, and given that gv -grayscale does not work, and given that gv does nothing different with the grayscale option except to pass it to gs, I think that the problem is with the way that gs handles the grayscale option.

>gv seems to do grayscale switch via setting palette to XtPaletteGrayscale.

Yes, but even though it has an "Xt" prefix, it is just a private enum in Ghostview.h. It is not a palette or a color mapping table.

The person who reported the problem in the gv list had a magenta-like color, while I have a reddish color.  I suspect that gdev_x_setup_colors() in gs/devices/gdevxcmp.c is not setting up enough colors.  For normal color, I think that it ends up with the default visual, which is 24 bit color on my laptop, but for grayscale, it requests a grayscale color map with XA_RGB_GRAY_MAP. Is it possible that it is ending up with an indexed color map that can't represent all of the gray scales? http://tronche.com/gui/x/xlib/window/visual-types.html
Comment 4 Hin-Tak Leung 2014-03-05 14:57:08 UTC
Possibly duplicate/related to bug 691432 (more detailed in bug 690428) which also played with X resources in a similar manner, setting "Ghostscript*palette: GrayScale" in ~/.Xdefaults .
Comment 5 William Bader 2014-03-05 16:11:54 UTC
Thanks, I think that you are right.
I created an .Xdefaults (actually, .Xdefaults-example.com, I ran strace to see where it looked) with one line "Ghostscript*palette: GrayScale", and I can reproduce the problem with gs9.10 (both the version that I compiled from source and the version distributed with Fedora 20).
I get the same results setting a GrayScale palette in Xdefaults as using -grayscale in gv.
William
Comment 6 James Cloos 2014-03-06 12:24:39 UTC
This (tested using Ghostscript*palette: GrayScale") shows up with the x11, x11alpha and x11rg32x DEVICEs.

It does not affect the x11cmyk*, x11gray*, x11mono or x11rg16x DEVICEs.

Nor does it affect the (gtk) display DEVICE, with any DisplayFormat.
Comment 7 Hin-Tak Leung 2014-07-24 18:04:16 UTC
I think this is the correct fix:

--- a/gs/devices/gdevxcmp.c
+++ b/gs/devices/gdevxcmp.c
@@ -431,6 +431,7 @@ grayscale:
         }
 
         /* Allocate the dynamic color table. */
+        if (!xdev->ghostview)
         alloc_dynamic_colors(xdev, xdev->cman.num_rgb -
                              xdev->color_info.dither_grays);
         break;


The reason why some shades shows as color is because they don't match the color cube, and so are dynamically allocated - through the underlying visual, which do have a full range of colors.

The problem with setting xresource manually is a similar problem - the color cube is set according to the worst of visual + resources, but dynamic color is allocated off the visual's.

I am seeing red for release build and magenta for debug build. So the full fix for both this and bug 691432 is:


diff --git a/gs/devices/gdevxcmp.c b/gs/devices/gdevxcmp.c
index d91e78d..67e4d47 100644
--- a/gs/devices/gdevxcmp.c
+++ b/gs/devices/gdevxcmp.c
@@ -252,6 +252,7 @@ gdev_x_setup_colors(gx_device_X * xdev)
          (xdev->vinfo->class != GrayScale) ? 'C' :     /* Color */
          (xdev->vinfo->colormap_size > 2) ? 'G' :              /* GrayScale */
          'M');         /* MonoChrome */
+    bool palette_mismatch = false;
 
     if (xdev->ghostview) {
         Atom gv_colors = XInternAtom(xdev->dpy, "GHOSTVIEW_COLORS", False);
@@ -281,6 +282,7 @@ gdev_x_setup_colors(gx_device_X * xdev)
             xdev->palette[0] = 'G';
         else if (xdev->palette[0] == 'm')
             xdev->palette[0] = 'M';
+        if (palette != xdev->palette[0]) palette_mismatch = true;
         palette = max(palette, xdev->palette[0]);
     }
 
@@ -431,6 +433,7 @@ grayscale:
         }
 
         /* Allocate the dynamic color table. */
+        if (!xdev->ghostview && !palette_mismatch)
         alloc_dynamic_colors(xdev, xdev->cman.num_rgb -
                              xdev->color_info.dither_grays);
         break;
Comment 8 William Bader 2014-07-25 09:50:03 UTC
Thanks! The patch works for me with gs 9.14 and 64-bit Fedora 20. William
Comment 9 Hin-Tak Leung 2014-11-24 18:53:39 UTC
The palette is correct but there seems to be another bug. It appears that only the red channel is being used for the gray values. See

http://bugs.ghostscript.com/show_bug.cgi?id=691432#c6
Comment 10 Hin-Tak Leung 2015-05-01 12:18:55 UTC
Just duplicating part of:
http://bugs.ghostscript.com/show_bug.cgi?id=691432#c7

---
I am reading through the xlib programming manual slowly, and indeed it mentions in section 7.8.5 that in a gray colormap, only the red channel is used. The code just has never worked properly even on a gray display with color input (i.e. gs's X11 device on a gray-only X server has always displayed only the red channel).

I'll prepare a better patch for this 2nd bug.
---
Comment 11 Hin-Tak Leung 2015-05-13 08:45:31 UTC
Created attachment 11679 [details]
fix x gray palette problem

There are really two problems with the code - it initialises part of it in gray
(num_component = 1) and starts with a gray ramp, but some of the graphic methods are still in color; and the dynamic color allocation receives data in color
and allocates as such.

So this fix initializes the graphic core properly and makes everything gray according to the graphic core, and repeats the gray index in dynamic color matching and allocation in the X server color allocation.

FWIW, I think the code never worked correctly.
Comment 12 Hin-Tak Leung 2015-05-13 08:50:34 UTC
The previous change was incorrect, because disallowing dynamic color only fixes some of the problem; color matching was still happening in color partly in color, and as some part thinks it is in color while some thinks it is in gray, uses only the red component to index into the gray ramp.
Comment 13 Hin-Tak Leung 2015-05-14 23:10:11 UTC
patch cluster tested. wasn't expecting any difference, and did not get any. So it is just about who wants to review it, and when to commit.
Comment 14 Marcos H. Woehrmann 2015-11-30 11:35:17 UTC
Henry is the author of  gdevcmp.c so I'm assigning this to him to review the patch.