Fix combining character handling in Pango.

The top-level loop in gtkwin.c which draws text was expecting that the
right way to draw a printing character plus combining characters was
to overprint them one by one on top of each other. This is an OK
assumption for X bitmap fonts, but in Pango, it works very badly -
most obviously because asking Pango to display a combining char on its
own causes it to print a dotted circle representing the base char, but
also because surely there will be character combinations where Pango
wants to do something more sophisticated than just printing them each
at a standard offset, and it would be a shame not to let it.

So I've moved the previous overprinting loop down into the x11font
subclass of the unifont mechanism. The top-level gtkwin.c drawing code
now calls a new method unifont_draw_combining, which in the X11 case
does the same loop as before, but in the Pango case, just passes a
whole base+combinings string to Pango in one go and lets it do the
best job it can.

diff --git a/unix/gtkfont.c b/unix/gtkfont.c
index 260c960237e821d47537470272e0525a461b110b..38a46a62b1bd60177bd9dde7f9db41c54461a954 100644 (file)
--- a/unix/gtkfont.c
+++ b/unix/gtkfont.c
@@ -83,6 +83,9 @@ struct unifont_vtable {
     void (*draw_text)(unifont_drawctx *ctx, unifont *font,
                       int x, int y, const wchar_t *string, int len,
                       int wide, int bold, int cellwidth);
+    void (*draw_combining)(unifont_drawctx *ctx, unifont *font,
+                           int x, int y, const wchar_t *string, int len,
+                           int wide, int bold, int cellwidth);
     void (*enum_fonts)(GtkWidget *widget,
                       fontsel_add_entry callback, void *callback_ctx);
     char *(*canonify_fontname)(GtkWidget *widget, const char *name, int *size,
@@ -107,6 +110,9 @@ static int x11font_has_glyph(unifont *font, wchar_t glyph);
 static void x11font_draw_text(unifont_drawctx *ctx, unifont *font,
                               int x, int y, const wchar_t *string, int len,
                               int wide, int bold, int cellwidth);
+static void x11font_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                   int x, int y, const wchar_t *string,
+                                   int len, int wide, int bold, int cellwidth);
 static unifont *x11font_create(GtkWidget *widget, const char *name,
                               int wide, int bold,
                               int shadowoffset, int shadowalways);
@@ -206,6 +212,7 @@ static const struct unifont_vtable x11font_vtable = {
     x11font_destroy,
     x11font_has_glyph,
     x11font_draw_text,
+    x11font_draw_combining,
     x11font_enum_fonts,
     x11font_canonify_fontname,
     x11font_scale_fontname,
@@ -868,6 +875,20 @@ static void x11font_draw_text(unifont_drawctx *ctx, unifont *font,
     }
 }
 
+static void x11font_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                   int x, int y, const wchar_t *string,
+                                   int len, int wide, int bold, int cellwidth)
+{
+    /*
+     * For server-side fonts, there's no sophisticated system for
+     * combining characters intelligently, so the best we can do is to
+     * overprint them on each other in the obvious way.
+     */
+    int i;
+    for (i = 0; i < len; i++)
+        x11font_draw_text(ctx, font, x, y, string+i, 1, wide, bold, cellwidth);
+}
+
 static void x11font_enum_fonts(GtkWidget *widget,
                               fontsel_add_entry callback, void *callback_ctx)
 {
@@ -1112,6 +1133,10 @@ static int pangofont_has_glyph(unifont *font, wchar_t glyph);
 static void pangofont_draw_text(unifont_drawctx *ctx, unifont *font,
                                 int x, int y, const wchar_t *string, int len,
                                 int wide, int bold, int cellwidth);
+static void pangofont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                     int x, int y, const wchar_t *string,
+                                     int len, int wide, int bold,
+                                     int cellwidth);
 static unifont *pangofont_create(GtkWidget *widget, const char *name,
                                 int wide, int bold,
                                 int shadowoffset, int shadowalways);
@@ -1157,6 +1182,7 @@ static const struct unifont_vtable pangofont_vtable = {
     pangofont_destroy,
     pangofont_has_glyph,
     pangofont_draw_text,
+    pangofont_draw_combining,
     pangofont_enum_fonts,
     pangofont_canonify_fontname,
     pangofont_scale_fontname,
@@ -1384,9 +1410,10 @@ static void pango_cairo_draw_layout(unifont_drawctx *ctx,
 }
 #endif
 
-static void pangofont_draw_text(unifont_drawctx *ctx, unifont *font,
-                                int x, int y, const wchar_t *string, int len,
-                                int wide, int bold, int cellwidth)
+static void pangofont_draw_internal(unifont_drawctx *ctx, unifont *font,
+                                    int x, int y, const wchar_t *string,
+                                    int len, int wide, int bold, int cellwidth,
+                                    int combining)
 {
     struct pangofont *pfont = (struct pangofont *)font;
     PangoLayout *layout;
@@ -1463,45 +1490,55 @@ static void pangofont_draw_text(unifont_drawctx *ctx, unifont *font,
         * them to do that.
         */
 
-       /*
-        * Start by extracting a single UTF-8 character from the
-        * string.
-        */
-       clen = 1;
-       while (clen < utflen &&
-              (unsigned char)utfptr[clen] >= 0x80 &&
-              (unsigned char)utfptr[clen] < 0xC0)
-           clen++;
-       n = 1;
-
-        if (is_rtl(string[0]) ||
-            pangofont_char_width(layout, pfont, string[n-1],
-                                 utfptr, clen) != desired) {
+        if (combining) {
             /*
-             * If this character is a right-to-left one, or has an
-             * unusual width, then we must display it on its own.
+             * For a character with combining stuff, we just dump the
+             * whole lot in one go, and expect it to take up just one
+             * character cell.
              */
+            clen = utflen;
+            n = 1;
         } else {
             /*
-             * Try to amalgamate a contiguous string of characters
-             * with the expected sensible width, for the common case
-             * in which we're using a monospaced font and everything
-             * works as expected.
+             * Start by extracting a single UTF-8 character from the
+             * string.
              */
-            while (clen < utflen) {
-                int oldclen = clen;
-                clen++;                       /* skip UTF-8 introducer byte */
-                while (clen < utflen &&
-                       (unsigned char)utfptr[clen] >= 0x80 &&
-                       (unsigned char)utfptr[clen] < 0xC0)
-                    clen++;
-                n++;
-                if (pangofont_char_width(layout, pfont,
-                                         string[n-1], utfptr + oldclen,
-                                         clen - oldclen) != desired) {
-                    clen = oldclen;
-                    n--;
-                    break;
+            clen = 1;
+            while (clen < utflen &&
+                   (unsigned char)utfptr[clen] >= 0x80 &&
+                   (unsigned char)utfptr[clen] < 0xC0)
+                clen++;
+            n = 1;
+
+            if (is_rtl(string[0]) ||
+                pangofont_char_width(layout, pfont, string[n-1],
+                                     utfptr, clen) != desired) {
+                /*
+                 * If this character is a right-to-left one, or has an
+                 * unusual width, then we must display it on its own.
+                 */
+            } else {
+                /*
+                 * Try to amalgamate a contiguous string of characters
+                 * with the expected sensible width, for the common case
+                 * in which we're using a monospaced font and everything
+                 * works as expected.
+                 */
+                while (clen < utflen) {
+                    int oldclen = clen;
+                    clen++;                   /* skip UTF-8 introducer byte */
+                    while (clen < utflen &&
+                           (unsigned char)utfptr[clen] >= 0x80 &&
+                           (unsigned char)utfptr[clen] < 0xC0)
+                        clen++;
+                    n++;
+                    if (pangofont_char_width(layout, pfont,
+                                             string[n-1], utfptr + oldclen,
+                                             clen - oldclen) != desired) {
+                        clen = oldclen;
+                        n--;
+                        break;
+                    }
                 }
             }
         }
@@ -1528,6 +1565,37 @@ static void pangofont_draw_text(unifont_drawctx *ctx, unifont *font,
     g_object_unref(layout);
 }
 
+static void pangofont_draw_text(unifont_drawctx *ctx, unifont *font,
+                                int x, int y, const wchar_t *string, int len,
+                                int wide, int bold, int cellwidth)
+{
+    pangofont_draw_internal(ctx, font, x, y, string, len, wide, bold,
+                            cellwidth, FALSE);
+}
+
+static void pangofont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                     int x, int y, const wchar_t *string,
+                                     int len, int wide, int bold,
+                                     int cellwidth)
+{
+    wchar_t *tmpstring = NULL;
+    if (mk_wcwidth(string[0]) == 0) {
+        /*
+         * If we've been told to draw a sequence of _only_ combining
+         * characters, prefix a space so that they have something to
+         * combine with.
+         */
+        tmpstring = snewn(len+1, wchar_t);
+        memcpy(tmpstring+1, string, len * sizeof(wchar_t));
+        tmpstring[0] = L' ';
+        string = tmpstring;
+        len++;
+    }
+    pangofont_draw_internal(ctx, font, x, y, string, len, wide, bold,
+                            cellwidth, TRUE);
+    sfree(tmpstring);
+}
+
 /*
  * Dummy size value to be used when converting a
  * PangoFontDescription of a scalable font to a string for
@@ -1873,6 +1941,14 @@ void unifont_draw_text(unifont_drawctx *ctx, unifont *font,
     font->vt->draw_text(ctx, font, x, y, string, len, wide, bold, cellwidth);
 }
 
+void unifont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                            int x, int y, const wchar_t *string, int len,
+                            int wide, int bold, int cellwidth)
+{
+    font->vt->draw_combining(ctx, font, x, y, string, len, wide, bold,
+                             cellwidth);
+}
+
 /* ----------------------------------------------------------------------
  * Multiple-font wrapper. This is a type of unifont which encapsulates
  * up to two other unifonts, permitting missing glyphs in the main
@@ -1889,6 +1965,10 @@ void unifont_draw_text(unifont_drawctx *ctx, unifont *font,
 static void multifont_draw_text(unifont_drawctx *ctx, unifont *font,
                                 int x, int y, const wchar_t *string, int len,
                                 int wide, int bold, int cellwidth);
+static void multifont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                     int x, int y, const wchar_t *string,
+                                     int len, int wide, int bold,
+                                     int cellwidth);
 static void multifont_destroy(unifont *font);
 
 struct multifont {
@@ -1903,6 +1983,7 @@ static const struct unifont_vtable multifont_vtable = {
     multifont_destroy,
     NULL,
     multifont_draw_text,
+    multifont_draw_combining,
     NULL,
     NULL,
     NULL,
@@ -1963,9 +2044,15 @@ static void multifont_destroy(unifont *font)
     sfree(font);
 }
 
-static void multifont_draw_text(unifont_drawctx *ctx, unifont *font, int x,
+typedef void (*unifont_draw_func_t)(unifont_drawctx *ctx, unifont *font,
+                                    int x, int y, const wchar_t *string,
+                                    int len, int wide, int bold,
+                                    int cellwidth);
+
+static void multifont_draw_main(unifont_drawctx *ctx, unifont *font, int x,
                                 int y, const wchar_t *string, int len,
-                                int wide, int bold, int cellwidth)
+                                int wide, int bold, int cellwidth,
+                                int cellinc, unifont_draw_func_t draw)
 {
     struct multifont *mfont = (struct multifont *)font;
     unifont *f;
@@ -1987,13 +2074,30 @@ static void multifont_draw_text(unifont_drawctx *ctx, unifont *font, int x,
          */
         f = ok ? mfont->main : mfont->fallback;
         if (f)
-            unifont_draw_text(ctx, f, x, y, string, i, wide, bold, cellwidth);
+            draw(ctx, f, x, y, string, i, wide, bold, cellwidth);
         string += i;
         len -= i;
-        x += i * cellwidth;
+        x += i * cellinc;
     }
 }
 
+static void multifont_draw_text(unifont_drawctx *ctx, unifont *font, int x,
+                                int y, const wchar_t *string, int len,
+                                int wide, int bold, int cellwidth)
+{
+    multifont_draw_main(ctx, font, x, y, string, len, wide, bold,
+                        cellwidth, cellwidth, unifont_draw_text);
+}
+
+static void multifont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                                     int x, int y, const wchar_t *string,
+                                     int len, int wide, int bold,
+                                     int cellwidth)
+{
+    multifont_draw_main(ctx, font, x, y, string, len, wide, bold,
+                        cellwidth, 0, unifont_draw_combining);
+}
+
 #if GTK_CHECK_VERSION(2,0,0)
 
 /* ----------------------------------------------------------------------

diff --git a/unix/gtkfont.h b/unix/gtkfont.h
index f99097f2ffd485d99674b3700deb65d247cbfdd2..6aa8ce2eb302d662fe4936aafbd77694dd401f82 100644 (file)
--- a/unix/gtkfont.h
+++ b/unix/gtkfont.h
@@ -136,6 +136,12 @@ void unifont_destroy(unifont *font);
 void unifont_draw_text(unifont_drawctx *ctx, unifont *font,
                        int x, int y, const wchar_t *string, int len,
                        int wide, int bold, int cellwidth);
+/* Same as unifont_draw_text, but expects 'string' to contain one
+ * normal char plus combining chars, and overdraws them all in the
+ * same character cell. */
+void unifont_draw_combining(unifont_drawctx *ctx, unifont *font,
+                            int x, int y, const wchar_t *string, int len,
+                            int wide, int bold, int cellwidth);
 
 /*
  * This function behaves exactly like the low-level unifont_create,

diff --git a/unix/gtkwin.c b/unix/gtkwin.c
index 88eadeb40def92c7e03c2a7ecdaf06491dfd5f5d..74146b4377b8bb268918e962bd12a7fc762f1a40 100644 (file)
--- a/unix/gtkwin.c
+++ b/unix/gtkwin.c
@@ -3397,7 +3397,7 @@ void do_text_internal(Context ctx, int x, int y, wchar_t *text, int len,
 {
     struct draw_ctx *dctx = (struct draw_ctx *)ctx;
     struct gui_data *inst = dctx->inst;
-    int ncombining, combining;
+    int ncombining;
     int nfg, nbg, t, fontid, shadow, rlen, widefactor, bold;
     int monochrome =
         gdk_visual_get_depth(gtk_widget_get_visual(inst->area)) == 1;
@@ -3494,11 +3494,20 @@ void do_text_internal(Context ctx, int x, int y, wchar_t *text, int len,
                    rlen*widefactor*inst->font_width, inst->font_height);
 
     draw_set_colour(dctx, nfg);
-    for (combining = 0; combining < ncombining; combining++) {
+    if (ncombining > 1) {
+        assert(len == 1);
+        unifont_draw_combining(&dctx->uctx, inst->fonts[fontid],
+                               x*inst->font_width+inst->window_border,
+                               (y*inst->font_height+inst->window_border+
+                                inst->fonts[0]->ascent),
+                               text, ncombining, widefactor > 1,
+                               bold, inst->font_width);
+    } else {
         unifont_draw_text(&dctx->uctx, inst->fonts[fontid],
                           x*inst->font_width+inst->window_border,
-                          y*inst->font_height+inst->window_border+inst->fonts[0]->ascent,
-                          text + combining, len, widefactor > 1,
+                          (y*inst->font_height+inst->window_border+
+                           inst->fonts[0]->ascent),
+                          text, len, widefactor > 1,
                           bold, inst->font_width);
     }