Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large strings

From: NRK <nrk_AT_disroot.org>
Date: Mon, 21 Mar 2022 19:00:32 +0600

+ if (ew + ext.xOff + lpad > w || b + utf8charlen > sizeof(buf) - 1) {
+ /* Only draw ellipsis if we have not recently started another font */
+ if (render && ellipsis_b > 3) {
+ ew = ellipsis_ew;
                                                [...]
+ /* Record the last buffer index where the ellipsis would still fit */
+ if (ew + ellipsis_width + lpad <= w) {
+ ellipsis_ew = ew;
+ ellipsis_b = b;
+ }

I think both of the `+ lpad` needs be removed. Otherwise it incorrectly
truncates the prompt as well.

        ./dmenu < /dev/null -p "p" # empty prompt
        ./dmenu < /dev/null -p "prompt" # truncated prompt

Also, I didn't quite get why there's a `ellipsis_b > 3` in there.

+ for (i = 0; i < utf8charlen; i++)
+ buf[b++] = *text++;

I'm kinda wondering if `buf` is even worth it or not. We could just render the
"..." separately. On my system atleast, there is no noticeable performance
difference, but removing `buf` from the equation (IMO) makes things more
simpler and easier to follow.

The following is what I've gotten so far, it's working fine and I haven't
noticed any regressions. The only "issue" is that it doesn't render the
ellipsis in case font changes, but current upstream dmenu doesn't seem to do it
either.

- NRK

diff --git a/drw.c b/drw.c
index 4cdbcbe..80dcad2 100644
--- a/drw.c
+++ b/drw.c
_AT_@ -251,20 +251,17 @@ drw_rect(Drw *drw, int x, int y, unsigned int w, unsigned int h, int filled, int
 int
 drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lpad, const char *text, int invert)
 {
- char buf[1024];
- int ty;
- unsigned int ew;
+ unsigned int ew = 0, ellipsis_ew = 0, ellipsis_width = 0, tmpw;
         XftDraw *d = NULL;
         Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
- int utf8strlen, utf8charlen, render = x || y || w || h;
+ int utf8strlen, utf8charlen, ellipsis_len, render = x || y || w || h;
         long utf8codepoint = 0;
         const char *utf8str;
         FcCharSet *fccharset;
         FcPattern *fcpattern;
         FcPattern *match;
         XftResult result;
- int charexists = 0;
+ int charexists = 0, truncate = 0;
 
         if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
                 return 0;
_AT_@ -283,7 +280,7 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
 
         usedfont = drw->fonts;
         while (1) {
- utf8strlen = 0;
+ utf8strlen = ellipsis_len = ew = ellipsis_ew = 0;
                 utf8str = text;
                 nextfont = NULL;
                 while (*text) {
_AT_@ -292,8 +289,27 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
                                 charexists = charexists || XftCharExists(drw->dpy, curfont->xfont, utf8codepoint);
                                 if (charexists) {
                                         if (curfont == usedfont) {
+ if (!ellipsis_width)
+ drw_font_getexts(curfont, "...", 3, &ellipsis_width, NULL);
+ drw_font_getexts(curfont, text, utf8charlen, &tmpw, NULL);
+ if (ew + tmpw > w) {
+ /* Only draw ellipsis if we have not recently started another font */
+ if (render && ellipsis_len > 0) {
+ ew = ellipsis_ew;
+ utf8strlen = ellipsis_len;
+ }
+ truncate = 1;
+ break;
+ }
+
+ /* Record the last text index where the ellipsis would still fit */
+ if (ew + ellipsis_width <= w) {
+ ellipsis_ew = ew;
+ ellipsis_len = utf8strlen;
+ }
                                                 utf8strlen += utf8charlen;
                                                 text += utf8charlen;
+ ew += tmpw;
                                         } else {
                                                 nextfont = curfont;
                                         }
_AT_@ -301,36 +317,28 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
                                 }
                         }
 
- if (!charexists || nextfont)
+ if (truncate || !charexists || nextfont)
                                 break;
                         else
                                 charexists = 0;
                 }
 
                 if (utf8strlen) {
- drw_font_getexts(usedfont, utf8str, utf8strlen, &ew, NULL);
- /* shorten text if necessary */
- for (len = MIN(utf8strlen, sizeof(buf) - 1); len && ew > w; len--)
- drw_font_getexts(usedfont, utf8str, len, &ew, NULL);
-
- if (len) {
- memcpy(buf, utf8str, len);
- buf[len] = '\0';
- if (len < utf8strlen)
- for (i = len; i && i > len - 3; buf[--i] = '.')
- ; /* NOP */
-
- if (render) {
- ty = y + (h - usedfont->h) / 2 + usedfont->xfont->ascent;
+ if (render) {
+ int ty = y + (h - usedfont->h) / 2 + usedfont->xfont->ascent;
+ XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
+ usedfont->xfont, x, ty, (XftChar8 *)utf8str, utf8strlen);
+ if (truncate) {
                                         XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
- usedfont->xfont, x, ty, (XftChar8 *)buf, len);
+ usedfont->xfont, x + ew, ty,
+ (XftChar8 *)"...", 3);
                                 }
- x += ew;
- w -= ew;
                         }
+ x += ew;
+ w -= ew;
                 }
 
- if (!*text) {
+ if (truncate || !*text) {
                         break;
                 } else if (nextfont) {
                         charexists = 0;
Received on Mon Mar 21 2022 - 14:00:32 CET