Fix colors on big endian (#4135)

* client/x11: Fix colors on big endian

The bitmaps are recieved in little endian byte order (LSBFirst in terms
of X11). This is a problem on systems, where bitmaps are expected in big
endian byte order (MSBFirst). X11 client tries to handle this situation
by different color formats (e.g. RGBX vs. BGRX), but this doesn't work.
BGRX is not MSBFirst variant of RGBX, it should be XRGB. But this would
work only for 32/24 depths, where color channels matches bytes. Let's
say to the XServer that all the bitmaps are in LSBFirst encoding instead
of changing the color formats on big endian systems.

https://github.com/FreeRDP/FreeRDP/issues/2520

* client/x11: Fix cursor color on big endian

The cursor color is wrong on big endian systems. It is not probably
possible to force bitmap_order for XcursorImage as it is possible for
XImage. Fortunately, cursors are always 32 bit so we can use ABGR
instead of BGRA to deal with.

https://github.com/FreeRDP/FreeRDP/issues/2520

* client/x11: Fix comment indentation

The comment has wrong indentation for some reason, let's fix it.

* client/x11: Fix BGR vs. RGB detection

The BGR vs. RGB detection code is leftover from history and I am conviced
that it is wrong with the current color handling, where invert flag is TRUE
by default. However, I suppose that the detection still makes sense and
XServer may use the inverted formats in some cases. Maybe we can force XServer
to use our masks somehow, but let's just fix the value to FALSE now.

* client/x11: Remove unused color shifts

The color shifts are lefover from history and are not used in current
code. Let's remove them.

diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c
index 6391ac8..5be609c 100644 (file)
--- a/client/X11/xf_client.c
+++ b/client/X11/xf_client.c
@@ -381,6 +381,9 @@ static BOOL xf_sw_desktop_resize(rdpContext* context)
                goto out;
        }
 
+       xfc->image->byte_order = LSBFirst;
+       xfc->image->bitmap_bit_order = LSBFirst;
+
        ret = xf_desktop_resize(context);
 out:
        xf_unlock_x11(xfc, TRUE);
@@ -628,6 +631,8 @@ BOOL xf_create_window(xfContext* xfc)
                                          ZPixmap, 0, (char*) gdi->primary_buffer,
                                          settings->DesktopWidth, settings->DesktopHeight,
                                          xfc->scanline_pad, gdi->stride);
+               xfc->image->byte_order = LSBFirst;
+               xfc->image->bitmap_bit_order = LSBFirst;
        }
 
        return TRUE;
@@ -772,15 +777,6 @@ void xf_unlock_x11(xfContext* xfc, BOOL display)
        }
 }
 
-static void xf_calculate_color_shifts(UINT32 mask, UINT8* rsh, UINT8* lsh)
-{
-       for (*lsh = 0; !(mask & 1); mask >>= 1)
-               (*lsh)++;
-
-       for (*rsh = 8; mask; mask >>= 1)
-               (*rsh)--;
-}
-
 static BOOL xf_get_pixmap_info(xfContext* xfc)
 {
        int i;
@@ -849,20 +845,13 @@ static BOOL xf_get_pixmap_info(xfContext* xfc)
        if (xfc->visual)
        {
                /*
-                        * Detect if the server visual has an inverted colormap
-                        * (BGR vs RGB, or red being the least significant byte)
-                        */
+                * Detect if the server visual has an inverted colormap
+                * (BGR vs RGB, or red being the least significant byte)
+                */
                if (vi->red_mask & 0xFF)
                {
-                       xfc->invert = TRUE;
+                       xfc->invert = FALSE;
                }
-
-               /* calculate color shifts required for rdp order color conversion */
-               xf_calculate_color_shifts(vi->red_mask, &xfc->red_shift_r, &xfc->red_shift_l);
-               xf_calculate_color_shifts(vi->green_mask, &xfc->green_shift_r,
-                                         &xfc->green_shift_l);
-               xf_calculate_color_shifts(vi->blue_mask, &xfc->blue_shift_r,
-                                         &xfc->blue_shift_l);
        }
 
        XFree(vis);
@@ -1813,7 +1802,7 @@ static BOOL xfreerdp_client_new(freerdp* instance, rdpContext* context)
        xfc->screen = ScreenOfDisplay(xfc->display, xfc->screen_number);
        xfc->depth = DefaultDepthOfScreen(xfc->screen);
        xfc->big_endian = (ImageByteOrder(xfc->display) == MSBFirst);
-       xfc->invert = (ImageByteOrder(xfc->display) == MSBFirst) ? FALSE : TRUE;
+       xfc->invert = TRUE;
        xfc->complex_regions = TRUE;
        xfc->x11event = CreateFileDescriptorEvent(NULL, FALSE, FALSE, xfc->xfds,
                        WINPR_FD_READ);

diff --git a/client/X11/xf_gdi.c b/client/X11/xf_gdi.c
index 57a3e53..ef06b35 100644 (file)
--- a/client/X11/xf_gdi.c
+++ b/client/X11/xf_gdi.c
@@ -241,6 +241,8 @@ static Pixmap xf_brush_new(xfContext* xfc, UINT32 width, UINT32 height,
                                   &xfc->context.gdi->palette, FREERDP_FLIP_NONE);
                image = XCreateImage(xfc->display, xfc->visual, xfc->depth,
                                     ZPixmap, 0, (char*) cdata, width, height, xfc->scanline_pad, 0);
+               image->byte_order = LSBFirst;
+               image->bitmap_bit_order = LSBFirst;
                gc = XCreateGC(xfc->display, xfc->drawable, 0, NULL);
                XPutImage(xfc->display, bitmap, gc, image, 0, 0, 0, 0, width, height);
                XFree(image);
@@ -264,6 +266,8 @@ static Pixmap xf_mono_bitmap_new(xfContext* xfc, int width, int height,
        bitmap = XCreatePixmap(xfc->display, xfc->drawable, width, height, 1);
        image = XCreateImage(xfc->display, xfc->visual, 1,
                             ZPixmap, 0, (char*) data, width, height, 8, scanline);
+       image->byte_order = LSBFirst;
+       image->bitmap_bit_order = LSBFirst;
        XPutImage(xfc->display, bitmap, xfc->gc_mono, image, 0, 0, 0, 0, width, height);
        XFree(image);
        return bitmap;
@@ -998,6 +1002,9 @@ static BOOL xf_gdi_update_screen(xfContext* xfc, const BYTE* pSrcData,
                if (!image)
                        break;
 
+               image->byte_order = LSBFirst;
+               image->bitmap_bit_order = LSBFirst;
+
                XPutImage(xfc->display, xfc->primary, xfc->gc, image, 0, 0, left, top, width, height);
                XFree(image);
                ret = xf_gdi_surface_update_frame(xfc, left, top, width, height);

diff --git a/client/X11/xf_gfx.c b/client/X11/xf_gfx.c
index 1a66d74..dd73d1e 100644 (file)
--- a/client/X11/xf_gfx.c
+++ b/client/X11/xf_gfx.c
@@ -293,6 +293,9 @@ static UINT xf_CreateSurface(RdpgfxClientContext* context,
                goto error_surface_image;
        }
 
+       surface->image->byte_order = LSBFirst;
+       surface->image->bitmap_bit_order = LSBFirst;
+
        surface->gdi.outputMapped = FALSE;
        region16_init(&surface->gdi.invalidRegion);
        if (context->SetSurfaceData(context, surface->gdi.surfaceId, (void*) surface) != CHANNEL_RC_OK)

diff --git a/client/X11/xf_graphics.c b/client/X11/xf_graphics.c
index 67c8dec..c2ef117 100644 (file)
--- a/client/X11/xf_graphics.c
+++ b/client/X11/xf_graphics.c
@@ -144,10 +144,12 @@ static BOOL xf_Bitmap_New(rdpContext* context, rdpBitmap* bitmap)
                xbitmap->image = XCreateImage(xfc->display, xfc->visual, xfc->depth,
                                              ZPixmap, 0, (char*) bitmap->data, bitmap->width, bitmap->height,
                                              xfc->scanline_pad, 0);
-
                if (!xbitmap->image)
                        goto unlock;
 
+               xbitmap->image->byte_order = LSBFirst;
+               xbitmap->image->bitmap_bit_order = LSBFirst;
+
                XPutImage(xfc->display, xbitmap->pixmap, xfc->gc, xbitmap->image, 0, 0, 0, 0, bitmap->width,
                          bitmap->height);
        }
@@ -231,9 +233,9 @@ static BOOL xf_Pointer_New(rdpContext* context, rdpPointer* pointer)
                return FALSE;
 
        if (!xfc->invert)
-               CursorFormat = PIXEL_FORMAT_RGBA32;
+               CursorFormat = (!xfc->big_endian) ? PIXEL_FORMAT_RGBA32 : PIXEL_FORMAT_ABGR32;
        else
-               CursorFormat = PIXEL_FORMAT_BGRA32;
+               CursorFormat = (!xfc->big_endian) ? PIXEL_FORMAT_BGRA32 : PIXEL_FORMAT_ARGB32;
 
        xf_lock_x11(xfc, FALSE);
        ZeroMemory(&ci, sizeof(ci));

diff --git a/client/X11/xfreerdp.h b/client/X11/xfreerdp.h
index 8cfcb24..ba86c4d 100644 (file)
--- a/client/X11/xfreerdp.h
+++ b/client/X11/xfreerdp.h
@@ -140,13 +140,6 @@ struct xf_context
        UINT16 frame_x2;
        UINT16 frame_y2;
 
-       UINT8 red_shift_l;
-       UINT8 red_shift_r;
-       UINT8 green_shift_l;
-       UINT8 green_shift_r;
-       UINT8 blue_shift_l;
-       UINT8 blue_shift_r;
-
        int XInputOpcode;
 
        int savedWidth;