bugGNUstep - Bugs: bug #28590, Cairo backend displays wrong...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #28590: Cairo backend displays wrong colors on X server with a different endianness

Submitted by:  Wolfgang Lux <wlux>
Submitted on:  Wed 13 Jan 2010 01:55:48 PM UTC  
 
Category:  Backend Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed

( Jump to the original submission)

Fri 22 Jan 2010 08:19:11 AM UTC, comment #11:

I've committed the patch.

Wolfgang Lux <wlux>
Thu 21 Jan 2010 08:50:45 PM UTC, comment #10:

Thank you for the explanation, this new patch of yours sounds like the right way to solve the issue. Feel free to go ahead.

And you are also right about xlib not using this class at all.

Fred Kiefer <FredKiefer>
Project Member
Thu 21 Jan 2010 06:35:55 PM UTC, comment #9:

Sorry for being that dense. I was just so concerned with getting the XImage settings correct that I managed to completely forget the @defs hack.

Anyway, the point is that ImageByteOrder(wi->display) doesn't work for cairo (in fact this call would be redundant since XCreateImage will set up the byte_order field with exactly that value). So, for cairo we must set up wi->ximage->byte_order according to GS_WORDS_BIGENDIAN. Yet, this doesn't work for the art backend, which attempts to determine the pixel format used by server and adjust to it. This means that we need to add an endianness field to the display info structure. I have a working patch for this and can contribute that later. The xlib backend is not affected by these changes; it does not call +windowBufferForWindow:depthInfo: at all (does this mean that the xlib backend treat a windows as NSBackingStoreNonRetained?)

Wolfgang Lux <wlux>
Wed 20 Jan 2010 02:47:38 PM UTC, comment #8:

Looks like I didn't express myself clear enough. Sorry for that.

What I tried to say was that we should put any patch into the file XWindowBuffer.m. And we only need to change any code in the case where we don't use shared memory (this is for some reason broken on modern X Servers and I don't have a clue).
This means we need to set the properties of the XImage we create in line 390 of that file. And a line like

wi->ximage->byte_order = ImageByteOrder(wi->display);

Should do the trick for cairo. This is very similar to your patch, just at a different place. If your change works this one should work just as well.

Now we have to test how this affects the xlib and the art backend and I am not able to test this. If this change breaks any of these backends we need to rework it, by passing in the byte order via the DI structure. Lets discuss this case, when it is actually needed.

Does this explanation make any more sense to you than the last one?

Fred Kiefer <FredKiefer>
Project Member
Tue 19 Jan 2010 10:12:45 AM UTC, comment #7:

Unfortunately I lack a machine where X is using shared memory at present. However, I thought that shared memory is used only when the X server and client are running on the same machine, in which case the patch should not break anything since the endianness should be the same. (Of course, this does change the hackishness of my patch :-)

I do not understand your suggestion to use the endianness of the server. The problem is that we pass the cairo image surface the raw buffer and not an XImage, so cairo can only use the endianness of the host. I've also found no way to tell cairo to use another endianness (which is not very surprising, since the image surface is a pure memory only representation). If we want generic code that works for all backends, we can only use the host's native endianness for the XImage buffer.

Wolfgang Lux <wlux>
Tue 19 Jan 2010 08:22:46 AM UTC, comment #6:

Thank you for your analysis. This really seems to be the cause of the problem.

You patch is a complete hack and cannot be applied :-)
It would break all cases where we use shared memory in XWindowBuffer. But it clearly points in the right direction. Why not put a similar change into XWindowBuffer.m, close to line 390 after we create the XImage? For the endianness, we should use that of the XServer, ImageByteOrder(dpy) should do the trick.

I am not sure, what this will result in for xlib and art when used remotely. If this change breaks anything, we need to move the byte order out of this method and pass it in as part of DI.
Could you please test this, as I don't have computers with different endianness.

Fred Kiefer <FredKiefer>
Project Member
Thu 14 Jan 2010 04:54:33 PM UTC, comment #5:

I finally managed to track down this bug. There is an endianness mismatch for the XImage used as backing store. In XWindowBuffer +windowBufferForWindow:depthInfo: the image is initialized with the byte order of the X11 display. However, the memory based cairo image surface allocated in XGCairoXImageSurface -initWithDevice: uses the host's native byte order. I have attached a rather hackish patch to set the correct byte order for the backing store; probably, there is a better way to do this.

(file #19492)

Wolfgang Lux <wlux>
Thu 14 Jan 2010 02:44:47 PM UTC, comment #4:

I wrote:

> To make things more weird, windows displayed during drag and drop show the right colors.


Turns out that things are not weird at all: The dragging window uses NSBackingStoreNonretained whereas the application windows use NSBackingStoreBuffered (or maybe NSBackingStoreRetained). If I change the backing store of the application windows to NSBackingStoreNonretained they also display correctly, so I guess an unintended byte swap happens either when writing to the backing store or when sending the backing store image to the X-server.

Wolfgang Lux <wlux>
Thu 14 Jan 2010 12:26:46 PM UTC, comment #3:

I did some more experiments and it looks like paths are displayed with wrong colors, too.
To make things more weird, windows displayed during drag and drop show the right colors. E.g., if I drag an item out of one of Gorm's palettes, the cursor turns green (as it should) and the dragged item is displayed with the correct colors in a window with a gray background. Of course, once dragging is complete, the dragged item shows up with the wrong colors again.

Wolfgang Lux <wlux>
Wed 13 Jan 2010 10:46:42 PM UTC, comment #2:

I'm not sure about a test case for paths. Any suggestion?

Anyway, it doesn't look like a cairo issue. I've checked a few cairo demos and there is no difference between local and remote display.

Wolfgang Lux <wlux>
Wed 13 Jan 2010 10:14:34 PM UTC, comment #1:

Does this happen only for images or for paths as well? The difference is that for images we do a conversion ourselves and this could of course be wrong. If it happens for paths as well, it is more likely a cairo issue. Or just a different problem we are causing :-)
Do you get similar problem with other cairo based applications when running remotely?

Fred Kiefer <FredKiefer>
Project Member
Wed 13 Jan 2010 01:55:48 PM UTC, original submission:

When running GNUstep applications on a x86 machine and displaying them on an X-server running on a PowerPC machine or vice versa, all pixels either lack a byte swap or or are swapped once too often. See the attached screen shots of Ink's color panel. Apparently ARGB get displayed as BRGA pixels. Neither the art nor the xlib backend suffer this problem. The X-server has depth 24.

Out of curiosity I also tried to run the server in 16 bit mode, but the cairo backend does not work in this configuration (neither locally nor remote), reporting the error:
Cairo status 'invalid value for stride' in DPSinitgraphics

Wolfgang Lux <wlux>

 

Attached Files
file #19492:  hack#28590.patch added by wlux (1KiB - application/octet-stream)
file #19483:  color-panel-black.tiff added by wlux (21KiB - image/tiff)
file #19484:  color-panel-red.tiff added by wlux (22KiB - image/tiff)
file #19485:  color-panel-green.tiff added by wlux (20KiB - image/tiff)
file #19486:  color-panel-blue.tiff added by wlux (21KiB - image/tiff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by FredKiefer (Posted a comment)
  • -email is unavailable- added by wlux (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-01-22 FredKiefer StatusNone => Fixed
        Open/ClosedOpen => Closed
    2010-01-14 wlux Attached File- => Added hack#28590.patch, #19492
    2010-01-13 wlux Attached File- => Added color-panel-black.tiff, #19483
        Attached File- => Added color-panel-red.tiff, #19484
        Attached File- => Added color-panel-green.tiff, #19485
        Attached File- => Added color-panel-blue.tiff, #19486

    Back to the top


    Powered by Savane 3.4