[Emacs-diffs] scratch/large-fonts c76605f: Fix display of glyphless characters with problematic fonts

[Eli Zaretskii]

branch: scratch/large-fonts
commit c76605faa1f597e67df1e5c6cfae5230ff3a6a76
Author: Eli Zaretskii <address@hidden>
Commit: Eli Zaretskii <address@hidden>

    Fix display of glyphless characters with problematic fonts
    
    * src/w32term.c (x_draw_glyph_string_background): Force redraw of
    glyph string background also when the font in use claims
    preposterously large global height value.  Helps to remove
    artifacts left from previous displays when glyphless characters
    are displayed as hex code in a box.
    * src/xterm.c (x_draw_glyph_string_background): Force redraw of
    glyph string background also when the font in use claims
    preposterously large global height value.  Helps to remove
    artifacts left from previous displays when glyphless characters
    are displayed as hex code in a box.
    * src/w32font.c (w32font_draw): Fix background drawing for
    glyphless characters that display as acronyms or hex codes in a
    box.
    * src/xftfont.c (xftfont_draw): Fix background drawing for
    glyphless characters that display as acronyms or hex codes in a
    box.
    * src/xdisp.c (produce_glyphless_glyph): Compute reasonable values
    for it->ascent and it->descent when the font claims preposterously
    large global values.
    (FONT_TOO_HIGH): Move from here...
    * src/dispextern.h (FONT_TOO_HIGH): ...to here.
---
 src/dispextern.h |    6 ++++++
 src/w32font.c    |   25 ++++++++++++++++++++++---
 src/w32term.c    |    7 ++++++-
 src/xdisp.c      |   30 ++++++++++++++++++++++--------
 src/xftfont.c    |   22 ++++++++++++++++++++--
 src/xterm.c      |    5 +++++
 6 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index d9d4d23..1537d44 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1526,6 +1526,12 @@ struct glyph_string
       + (FRAME_LINE_HEIGHT ((F)) > FONT_HEIGHT ((FONT)))) / 2  \
    - (FONT_DESCENT (FRAME_FONT (F)) - FRAME_BASELINE_OFFSET (F)))
 
+/* A heuristic test for fonts that claim they need a preposterously
+   large vertical space.  The heuristics is in the factor of 3.  We
+   ignore the ascent and descent values reported by such fonts, and
+   instead go by the values reported for individual glyphs.  */
+#define FONT_TOO_HIGH(ft)  ((ft)->ascent + (ft)->descent > 3*(ft)->pixel_size)
+
 
 /***********************************************************************
                                Faces
diff --git a/src/w32font.c b/src/w32font.c
index 6306a84..1c2f966 100644
--- a/src/w32font.c
+++ b/src/w32font.c
@@ -650,12 +650,31 @@ w32font_draw (struct glyph_string *s, int from, int to,
       HBRUSH brush;
       RECT rect;
       struct font *font = s->font;
-
+      int ascent = font->ascent, descent = font->descent;
+
+      /* Font's global ascent and descent values might be
+        preposterously large for some fonts.  We fix here the case
+        when those fonts are used for display of glyphless
+        characters, because drawing background with font dimensions
+        in those cases makes the display illegible.  There's only one
+        more call to the draw method with with_background set to
+        true, and that's in x_draw_glyph_string_foreground, when
+        drawing the cursor, where we have no such heuristics
+        available.  FIXME.  */
+      if (s->first_glyph->type == GLYPHLESS_GLYPH
+         && (s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_HEX_CODE
+             || s->first_glyph->u.glyphless.method == 
GLYPHLESS_DISPLAY_ACRONYM))
+       {
+         ascent =
+           s->first_glyph->slice.glyphless.lower_yoff
+           - s->first_glyph->slice.glyphless.upper_yoff;
+         descent = 0;
+       }
       brush = CreateSolidBrush (s->gc->background);
       rect.left = x;
-      rect.top = y - font->ascent;
+      rect.top = y - ascent;
       rect.right = x + s->width;
-      rect.bottom = y + font->descent;
+      rect.bottom = y + descent;
       FillRect (s->hdc, &rect, brush);
       DeleteObject (brush);
     }
diff --git a/src/w32term.c b/src/w32term.c
index 0bc2e98..9c4f28f 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -1218,7 +1218,12 @@ x_draw_glyph_string_background (struct glyph_string *s, 
bool force_p)
        }
       else
 #endif
-        if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
+           if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
+              /* When xdisp.c ignores FONT_HEIGHT, we cannot trust
+                 font dimensions, since the actual glyphs might be
+                 much smaller.  So in that case we always clear the
+                 rectangle with background color.  */
+              || FONT_TOO_HIGH (s->font)
               || s->font_not_found_p
               || s->extends_to_end_of_line_p
               || force_p)
diff --git a/src/xdisp.c b/src/xdisp.c
index a1b7cf1..ed430a4 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -25296,12 +25296,6 @@ draw_glyphs (struct window *w, int x, struct glyph_row 
*row,
       }                                                        \
   }
 
-/* A heuristic test for fonts that claim they need a preposterously
-   large vertical space.  The heuristics is in the factor of 3.  We
-   ignore the ascent and descent values reported by such fonts, and
-   instead go by the values reported for individual glyphs.  */
-#define FONT_TOO_HIGH(ft)  ((ft)->ascent + (ft)->descent > 3*(ft)->pixel_size)
-
 /* Store one glyph for IT->char_to_display in IT->glyph_row.
    Called from x_produce_glyphs when IT->glyph_row is non-null.  */
 
@@ -26230,8 +26224,28 @@ produce_glyphless_glyph (struct it *it, bool 
for_no_font, Lisp_Object acronym)
      ASCII face.  */
   face = FACE_FROM_ID (it->f, it->face_id)->ascii_face;
   font = face->font ? face->font : FRAME_FONT (it->f);
-  it->ascent = FONT_BASE (font) + font->baseline_offset;
-  it->descent = FONT_DESCENT (font) - font->baseline_offset;
+  it->ascent = FONT_BASE (font);
+  it->descent = FONT_DESCENT (font);
+  /* Attempt to fix box height for fonts that claim preposterously
+     large height.  */
+  if (FONT_TOO_HIGH (font))
+    {
+      XChar2b char2b;
+
+      /* Get metrics of a reasonably sized ASCII character.  */
+      if (get_char_glyph_code ('{', font, &char2b))
+       {
+         struct font_metrics *pcm = get_per_char_metric (font, &char2b);
+
+         if (!(pcm->width == 0 && pcm->rbearing == 0 && pcm->lbearing == 0))
+           {
+             it->ascent = pcm->ascent;
+             it->descent = pcm->descent;
+           }
+       }
+    }
+  it->ascent += font->baseline_offset;
+  it->descent -= font->baseline_offset;
   base_height = it->ascent + it->descent;
   base_width = font->average_width;
 
diff --git a/src/xftfont.c b/src/xftfont.c
index 0e8b876..a1846e8 100644
--- a/src/xftfont.c
+++ b/src/xftfont.c
@@ -617,8 +617,26 @@ xftfont_draw (struct glyph_string *s, int from, int to, 
int x, int y,
     XftDrawSetClip (xft_draw, NULL);
 
   if (with_background)
-    XftDrawRect (xft_draw, &bg,
-                x, y - s->font->ascent, s->width, s->font->height);
+    {
+      int height = FONT_HEIGHT (s->font), ascent = FONT_BASE (s->font);
+
+      /* Font's global height and ascent values might be
+        preposterously large for some fonts.  We fix here the case
+        when those fonts are used for display of glyphless
+        characters, because drawing background with font dimensions
+        in those cases makes the display illegible.  There's only one
+        more call to the draw method with with_background set to
+        true, and that's in x_draw_glyph_string_foreground, when
+        drawing the cursor, where we have no such heuristics
+        available.  FIXME.  */
+      if (s->first_glyph->type == GLYPHLESS_GLYPH
+         && (s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_HEX_CODE
+             || s->first_glyph->u.glyphless.method == 
GLYPHLESS_DISPLAY_ACRONYM))
+       height = ascent =
+         s->first_glyph->slice.glyphless.lower_yoff
+         - s->first_glyph->slice.glyphless.upper_yoff;
+      XftDrawRect (xft_draw, &bg, x, y - ascent, s->width, height);
+    }
   code = alloca (sizeof (FT_UInt) * len);
   for (i = 0; i < len; i++)
     code[i] = ((XCHAR2B_BYTE1 (s->char2b + from + i) << 8)
diff --git a/src/xterm.c b/src/xterm.c
index 4f5dfed..58563ff 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1724,6 +1724,11 @@ x_draw_glyph_string_background (struct glyph_string *s, 
bool force_p)
          s->background_filled_p = true;
        }
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
+              /* When xdisp.c ignores FONT_HEIGHT, we cannot trust
+                 font dimensions, since the actual glyphs might be
+                 much smaller.  So in that case we always clear the
+                 rectangle with background color.  */
+              || FONT_TOO_HIGH (s->font)
               || s->font_not_found_p
               || s->extends_to_end_of_line_p
               || force_p)