[Spice-devel] [PATCH v3 spice-gtk 2/2] channel-cursor: mono cursors edge highlighting

Alon Levy alevy at redhat.com
Sat Oct 19 18:16:46 CEST 2013


Fix 998529, mono (invert) cursors not visible on a black background, by doing
simple edge detection on the cursor (this is done once when the cursor
is changed and then cached, cursors are 32x32 generally) and thus having
a cursor with contrast on both dark and light backgrounds.

When (if) GDK gets invert cursor support (wayland?) then we can just use
the cursor as is. Until then X doesn't provide any way I see of solving
this otherwise. The end result was tested with the I beam cursor that
the original bug was referring to (run putty on a windows 7 vm) and
looks ok to me.

Moving the core function to spice-util for testing.
---
 gtk/channel-cursor.c  |  64 +++------------------------
 gtk/spice-util-priv.h |   2 +
 gtk/spice-util.c      | 112 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/util.c          | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+), 58 deletions(-)

diff --git a/gtk/channel-cursor.c b/gtk/channel-cursor.c
index 66e91df..e056b30 100644
--- a/gtk/channel-cursor.c
+++ b/gtk/channel-cursor.c
@@ -277,70 +277,18 @@ static void print_cursor(display_cursor *cursor, const guint8 *data)
 
 static void mono_cursor(display_cursor *cursor, const guint8 *data)
 {
+    int bpl = (cursor->hdr.width + 7) / 8;
     const guint8 *xor, *and;
     guint8 *dest;
-    int bpl, x, y, bit;
+    dest = (uint8_t *)cursor->data;
 
-    bpl = (cursor->hdr.width + 7) / 8;
-    and = data;
-    xor = and + bpl * cursor->hdr.height;
-    dest  = (uint8_t *)cursor->data;
 #ifdef DEBUG_CURSOR
     print_cursor(cursor, data);
 #endif
-    for (y = 0; y < cursor->hdr.height; y++) {
-        bit = 0x80;
-        for (x = 0; x < cursor->hdr.width; x++, dest += 4) {
-            if (and[x/8] & bit) {
-                if (xor[x/8] & bit) {
-                    /*
-                     * flip -> unsupported by x11, since XCreatePixmapCursor has
-                     * no invert functionality, only a mask, shape, background and
-                     * foreground colors. Use this checkerboard hack to get some
-                     * contrast for cursors in the guest that relied on invert for
-                     * the same contrast.
-                     */
-                    if ((x ^ y) & 1) {
-                        dest[0] = 0xff;
-                        dest[1] = 0xff;
-                        dest[2] = 0xff;
-                        dest[3] = 0xff;
-                    } else {
-                        dest[0] = 0x00;
-                        dest[1] = 0x00;
-                        dest[2] = 0x00;
-                        dest[3] = 0x00;
-                    }
-                } else {
-                    /* unchanged -> transparent */
-                    dest[0] = 0x00;
-                    dest[1] = 0x00;
-                    dest[2] = 0x00;
-                    dest[3] = 0x00;
-                }
-            } else {
-                if (xor[x/8] & bit) {
-                    /* set -> white */
-                    dest[0] = 0xff;
-                    dest[1] = 0xff;
-                    dest[2] = 0xff;
-                    dest[3] = 0xff;
-                } else {
-                    /* clear -> black */
-                    dest[0] = 0x00;
-                    dest[1] = 0x00;
-                    dest[2] = 0x00;
-                    dest[3] = 0xff;
-                }
-            }
-            bit >>= 1;
-            if (bit == 0) {
-                bit = 0x80;
-            }
-        }
-        and += bpl;
-        xor += bpl;
-    }
+    and = data;
+    xor = and + bpl * cursor->hdr.height;
+    spice_mono_edge_highlight(cursor->hdr.width, cursor->hdr.height,
+                              and, xor, dest);
 }
 
 static guint8 get_pix_mask(const guint8 *data, gint offset, gint pix_index)
diff --git a/gtk/spice-util-priv.h b/gtk/spice-util-priv.h
index cc559dc..7f0df3c 100644
--- a/gtk/spice-util-priv.h
+++ b/gtk/spice-util-priv.h
@@ -31,6 +31,8 @@ const gchar* spice_yes_no(gboolean value);
 guint16 spice_make_scancode(guint scancode, gboolean release);
 gchar* spice_unix2dos(const gchar *str, gssize len, GError **error);
 gchar* spice_dos2unix(const gchar *str, gssize len, GError **error);
+void spice_mono_edge_highlight(unsigned width, unsigned hight,
+                               const guint8 *and, const guint8 *xor, guint8 *dest);
 
 #if GLIB_CHECK_VERSION(2,32,0)
 #define STATIC_MUTEX            GMutex
diff --git a/gtk/spice-util.c b/gtk/spice-util.c
index 21dfced..8810a19 100644
--- a/gtk/spice-util.c
+++ b/gtk/spice-util.c
@@ -20,6 +20,7 @@
 # include "config.h"
 #endif
 
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <glib.h>
@@ -374,3 +375,114 @@ gchar* spice_unix2dos(const gchar *str, gssize len, GError **error)
                                   NEWLINE_TYPE_CR_LF,
                                   error);
 }
+
+static bool buf_is_ones(unsigned size, const guint8 *data)
+{
+    int i;
+
+    for (i = 0 ; i < size; ++i) {
+        if (data[i] != 0xff) {
+            return false;
+        }
+    }
+    return true;
+}
+
+static bool is_edge_helper(const guint8 *xor, int bpl, int x, int y)
+{
+    return (xor[bpl * y + (x / 8)] & (0x80 >> (x % 8))) > 0;
+}
+
+static bool is_edge(unsigned width, unsigned height, const guint8 *xor, int bpl, int x, int y)
+{
+    if (x == 0 || x == width -1 || y == 0 || y == height - 1) {
+        return 0;
+    }
+#define P(x, y) is_edge_helper(xor, bpl, x, y)
+    return !P(x, y) && (P(x - 1, y + 1) || P(x, y + 1) || P(x + 1, y + 1) ||
+                        P(x - 1, y)     ||                P(x + 1, y)     ||
+                        P(x - 1, y - 1) || P(x, y - 1) || P(x + 1, y - 1));
+#undef P
+}
+
+/* Mono cursors have two places, "and" and "xor". If a bit is 1 in both, it
+ * means invertion of the corresponding pixel in the display. Since X11 (and
+ * gdk) doesn't do invertion, instead we do edge detection and turn the
+ * sorrounding edge pixels black, and the invert-me pixels white. To
+ * illustrate:
+ *
+ *  and   xor      dest RGB (1=0xffffff, 0=0x000000)
+ *
+ *                        dest alpha (1=0xff, 0=0x00)
+ *
+ * 11111 00000     00000  00000
+ * 11111 00000     00000  01110
+ * 11111 00100 =>  00100  01110
+ * 11111 00100     00100  01110
+ * 11111 00000     00000  01110
+ * 11111 00000     00000  00000
+ *
+ * See tests/util.c for more tests
+ *
+ * Notes:xi
+ *  Assumes width >= 8 (i.e. bytes per line is at least 1)
+ *  Assumes edges are not on the boundary (first/last line/column) for simplicity
+ *
+ */
+G_GNUC_INTERNAL
+void spice_mono_edge_highlight(unsigned width, unsigned height,
+                               const guint8 *and, const guint8 *xor, guint8 *dest)
+{
+    int bpl = (width + 7) / 8;
+    bool and_ones = buf_is_ones(height * bpl, and);
+    int x, y, bit;
+    const guint8 *xor_base = xor;
+
+    for (y = 0; y < height; y++) {
+        bit = 0x80;
+        for (x = 0; x < width; x++, dest += 4) {
+            if (is_edge(width, height, xor_base, bpl, x, y) && and_ones) {
+                dest[0] = 0x00;
+                dest[1] = 0x00;
+                dest[2] = 0x00;
+                dest[3] = 0xff;
+                goto next_bit;
+            }
+            if (and[x/8] & bit) {
+                if (xor[x/8] & bit) {
+                    dest[0] = 0xff;
+                    dest[1] = 0xff;
+                    dest[2] = 0xff;
+                    dest[3] = 0xff;
+                } else {
+                    /* unchanged -> transparent */
+                    dest[0] = 0x00;
+                    dest[1] = 0x00;
+                    dest[2] = 0x00;
+                    dest[3] = 0x00;
+                }
+            } else {
+                if (xor[x/8] & bit) {
+                    /* set -> white */
+                    dest[0] = 0xff;
+                    dest[1] = 0xff;
+                    dest[2] = 0xff;
+                    dest[3] = 0xff;
+                } else {
+                    /* clear -> black */
+                    dest[0] = 0x00;
+                    dest[1] = 0x00;
+                    dest[2] = 0x00;
+                    dest[3] = 0xff;
+                }
+            }
+        next_bit:
+            bit >>= 1;
+            if (bit == 0) {
+                bit = 0x80;
+            }
+        }
+        and += bpl;
+        xor += bpl;
+    }
+}
diff --git a/tests/util.c b/tests/util.c
index 86109aa..922818a 100644
--- a/tests/util.c
+++ b/tests/util.c
@@ -78,12 +78,129 @@ static void test_unix2dos(void)
     }
 }
 
+static const struct {
+    unsigned width;
+    unsigned height;
+    guint8 *and;
+    guint8 *xor;
+    guint8 *dest;
+} mono[] = {
+    {
+        8, 6,
+        "11111111"
+        "11111111"
+        "11111111"
+        "11111111"
+        "11111111"
+        "11111111"
+        ,
+        "00000000"
+        "00000000"
+        "00000100"
+        "00000100"
+        "00000000"
+        "00000000"
+        ,
+        "0000" "0000" "0000" "0000" "0000" "0000" "0000" "0000"
+        "0000" "0000" "0000" "0000" "0001" "0001" "0001" "0000"
+        "0000" "0000" "0000" "0000" "0001" "1111" "0001" "0000"
+        "0000" "0000" "0000" "0000" "0001" "1111" "0001" "0000"
+        "0000" "0000" "0000" "0000" "0001" "0001" "0001" "0000"
+        "0000" "0000" "0000" "0000" "0000" "0000" "0000" "0000"
+    }
+};
+
+static void set_bit(guint8 *d, unsigned bit, unsigned value)
+{
+    if (value) {
+        *d |= (0x80 >> bit);
+    } else {
+        *d &= ~(0x80 >> bit);
+    }
+}
+
+static void test_set_bit(void)
+{
+    struct {
+        unsigned len;
+        guint8 *src;
+        guint8 *dest;
+    } tests[] = {
+        {
+            4,
+            "1111",
+            "\xf0",
+        },
+        {
+            16,
+            "1111011100110001",
+            "\xf7\x31",
+        }
+    };
+    int i, j, bit;
+    guint8 *dest;
+    int bytes;
+    
+    for (i = 0 ; i < G_N_ELEMENTS(tests); ++i) {
+        bytes = (tests[i].len + 7) / 8;
+        dest = g_malloc0(bytes);
+        for (j = 0 ; j < tests[i].len;) {
+            for (bit = 0 ; bit < 8 && j < tests[i].len; ++bit, ++j) {
+                set_bit(&dest[j / 8], bit, tests[i].src[j] == '0' ? 0 : 1);
+            }
+        }
+        for (j = 0 ; j < bytes; ++j) {
+            g_assert(dest[j] == tests[i].dest[j]);
+        }
+        g_free(dest);
+    }
+}
+
+static void test_mono_edge_highlight(void)
+{
+    int i, j, bit;
+    guint8 *and;
+    guint8 *xor;
+    guint8 *dest;
+    guint8 *dest_correct;
+    int size, pixels;
+
+    test_set_bit();
+
+    for (i = 0 ; i < G_N_ELEMENTS(mono); ++i) {
+        pixels = mono[i].width * mono[i].height;
+        size = (pixels + 7) / 8;
+        and = g_malloc0(size);
+        xor = g_malloc0(size);
+        dest = g_malloc0(pixels * 4);
+        dest_correct = g_malloc(pixels * 4);
+        for (j = 0 ; j < pixels;) {
+            for (bit = 0; bit < 8 && j < pixels; ++bit, ++j) {
+                set_bit(&and[j / 8], bit, mono[i].and[j] == '0' ? 0 : 1);
+                set_bit(&xor[j / 8], bit, mono[i].xor[j] == '0' ? 0 : 1);
+            }
+        }
+        for (j = 0 ; j < pixels * 4 ; ++j) {
+            dest_correct[j] = mono[i].dest[j] == '0' ? 0x00 : 0xff;
+        }
+        spice_mono_edge_highlight(mono[i].width, mono[i].height, and, xor, dest);
+        for (j = 0; j < pixels; ++j) {
+            g_assert(dest[j] == dest_correct[j]);
+        }
+        g_free(and);
+        g_free(xor);
+        g_free(dest);
+        g_free(dest_correct);
+    }
+}
+
 int main(int argc, char* argv[])
 {
   g_test_init(&argc, &argv, NULL);
 
   g_test_add_func("/util/dos2unix", test_dos2unix);
   g_test_add_func("/util/unix2dos", test_unix2dos);
+  g_test_add_func("/util/mono_edge_highlight", test_mono_edge_highlight);
 
   return g_test_run ();
 }
-- 
1.8.3.1



More information about the Spice-devel mailing list