Re: [dev] [st] Font Cache handling, attached patch

From: Gary Allen Vollink <gary_AT_vollink.com>
Date: Thu, 5 Oct 2017 02:59:09 -0400

> On Oct 2, 2017, at 10:06 PM, Gary Allen Vollink <gary_AT_vollink.com> wrote:
>
> I noticed the following AFTER I sent the initial patch. I seriously
> doubt that this is new or unexpected behavior since the frc member
> unicodep is used to catch the individual rune that a font was opened
> for (and then eliminate that ONE rune from a new open on the next
> cache search pass). "break;"
>
> The problem with this is - outside of the odd Emoji - when dealing
> with Asian languages, if Glyphs can't be found it's because someone is
> looking at a document that expects to have runes in the Asian Extended
> D or E Unicode ranges, and they don't have that font available (in
> monospace). That means occurrences of repeating, not-found runes are
> going to be much less common than new, unique not-found runes, and
> those font slots QUICKLY run out.
>
> Maybe ALSO dealing with the ever-growing list of rune-not-found is the
> right way to handle this situation (it will cut down on
> open-then-close), by expanding this patch or in a completely separate
> patch. The problem is a growing list of not-found makes for code that
> sucks (using ever more memory over time). wchar_t isn't huge, but
> these Unicode ranges are particularly large.
>
> Thank you,
> Gary Allen

I revised this looking to improve on what I started. Presented here for discussion.

First, unicodep, was removed.

Second, I introduce a wchar_t array called noglyph of a hard-coded 1024 length to take care of the differential between frc[16] and the likelihood of running into a high number of unique glyphs.

This deeply expands the advantage of not trying to open a new font for a glyph we have already discovered is not found, and includes my original patch of re-closing the font (and keeping it out of the frc cache) since that font is both already open and isn’t being used to represent a glyph anyway.

In handling the noglyph array, I do a FIFO (instead of a LIFO as the frc does). This seems to give the highest likelihood that anything removed from the array will already be off-screen.

Side-note, I’m curious as to objections to switching the frc Font Cache array to FIFO as well (with the hope that the earliest fallback is least likely to still be on-screen).


Check-in comment:

If fontconfig returns a font that has no matching glyph, close the now opened but unused font. Track glyph-less rune separately.


diff -U 3 a/x.c b/x.c
--- a/x.c 2017-09-30 15:46:51.713848500 -0400
+++ b/x.c 2017-10-04 23:08:23.162156200 -0400
_AT_@ -1,6 +1,7 @@
 /* See LICENSE for license details. */
 #include <errno.h>
 #include <locale.h>
+#include <wchar.h>
 #include <signal.h>
 #include <stdint.h>
 #include <sys/select.h>
_AT_@ -151,9 +152,12 @@
 typedef struct {
         XftFont *font;
         int flags;
- Rune unicodep;
 } Fontcache;
 
+/* Runes not found array. */
+static wchar_t noglyph[1024];
+static int noglyphlen = 0;
+
 /* Fontcache is an array now. A new font will be appended to the array. */
 static Fontcache frc[16];
 static int frclen = 0;
_AT_@ -997,7 +1001,7 @@
         FcPattern *fcpattern, *fontpattern;
         FcFontSet *fcsets[] = { NULL };
         FcCharSet *fccharset;
- int i, f, numspecs = 0;
+ int i, f, r, numspecs = 0;
 
         for (i = 0, xp = winx, yp = winy + font->ascent; i < len; ++i) {
                 /* Fetch rune and mode for current glyph. */
_AT_@ -1029,7 +1033,8 @@
 
                 /* Lookup character index with default font. */
                 glyphidx = XftCharIndex(xw.dpy, font->match, rune);
- if (glyphidx) {
+ /* OR if already failed to find a glyph for this rune. */
+ if ((glyphidx) || (wmemchr(noglyph, rune, noglyphlen))) {
                         specs[numspecs].font = font->match;
                         specs[numspecs].glyph = glyphidx;
                         specs[numspecs].x = (short)xp;
_AT_@ -1045,11 +1050,6 @@
                         /* Everything correct. */
                         if (glyphidx && frc[f].flags == frcflags)
                                 break;
- /* We got a default font for a not found glyph. */
- if (!glyphidx && frc[f].flags == frcflags
- && frc[f].unicodep == rune) {
- break;
- }
                 }
 
                 /* Nothing was found. Use fontconfig to find matching font. */
_AT_@ -1087,7 +1087,6 @@
                         if (frclen >= LEN(frc)) {
                                 frclen = LEN(frc) - 1;
                                 XftFontClose(xw.dpy, frc[frclen].font);
- frc[frclen].unicodep = 0;
                         }
 
                         frc[frclen].font = XftFontOpenPattern(xw.dpy,
_AT_@ -1096,12 +1095,23 @@
                                 die("XftFontOpenPattern failed seeking fallback font: %s\n",
                                         strerror(errno));
                         frc[frclen].flags = frcflags;
- frc[frclen].unicodep = rune;
 
                         glyphidx = XftCharIndex(xw.dpy, frc[frclen].font, rune);
 
- f = frclen;
- frclen++;
+ if (!glyphidx) {
+ XftFontClose(xw.dpy, frc[frclen].font);
+ if (noglyphlen >= LEN(noglyph)) {
+ /* Get rid of the oldest not found rune */
+ for (r=1; r <= LEN(noglyph); r++) {
+ noglyph[r-1] = noglyph[r];
+ }
+ noglyphlen = LEN(noglyph) - 1;
+ }
+ noglyph[noglyphlen++] = rune;
+ } else {
+ f = frclen;
+ frclen++;
+ }
 
                         FcPatternDestroy(fcpattern);
                         FcCharSetDestroy(fccharset);
Received on Thu Oct 05 2017 - 08:59:09 CEST