Re: [hackers] [dmenu] import new drw from libsl and minor fixes.

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Fri, 3 Jun 2016 18:21:58 +0200

On Sun, May 22, 2016 at 10:34:30PM +0200, Markus Teich wrote:
> - extract drawitem function (code deduplication)
> - fix bug where inputw was not correctly calculated from the widest item, but
> just from the one with the longest strlen() which is not the same.
> - minor code style fixes (indentation, useless line breaks)
> ---

... snip snip ...

Hey,

I reviewed the drw patches for dmenu and it looks pretty good to me, but I
noticed a regression, dmenu seems really slow to load on my machine in the
function readstdin(). Was the font cache behaviour removed from drw?

As a test I did:

        ./dmenu -l 20 < dmenu.c

and it takes about 4 seconds to load. On another user his machine it took
18 seconds each time!

I have written a hacky patch to workaround the issue, it stores the value of
textw and so only calculates the width per item once, but it can probably be
done better in another way. Anyway here is the patch:


diff --git a/dmenu.c b/dmenu.c
index f802553..11710a7 100644
--- a/dmenu.c
+++ b/dmenu.c
_AT_@ -23,12 +23,14 @@
                              * MAX(0, MIN((y)+(h),(r).y_org+(r).height) - MAX((y),(r).y_org)))
 #define LENGTH(X) (sizeof X / sizeof X[0])
 #define TEXTW(X) (drw_fontset_getwidth(drw, (X)) + lrpad)
+#define TEXTWITEM(X) ((X)->textw = (X)->textw ? (X)->textw : (drw_fontset_getwidth(drw, ((X)->text)) + lrpad))
 
 /* enums */
 enum { SchemeNorm, SchemeSel, SchemeOut, SchemeLast }; /* color schemes */
 
 struct item {
         char *text;
+ size_t textw;
         struct item *left, *right;
         int out;
 };
_AT_@ -81,10 +83,10 @@ calcoffsets(void)
                 n = mw - (promptw + inputw + TEXTW("<") + TEXTW(">"));
         /* calculate which items will begin the next page and previous page */
         for (i = 0, next = curr; next; next = next->right)
- if ((i += (lines > 0) ? bh : TEXTW(next->text)) > n)
+ if ((i += (lines > 0) ? bh : TEXTWITEM(next)) > n)
                         break;
         for (i = 0, prev = curr; prev && prev->left; prev = prev->left)
- if ((i += (lines > 0) ? bh : TEXTW(prev->left->text)) > n)
+ if ((i += (lines > 0) ? bh : TEXTWITEM(prev->left)) > n)
                         break;
 }
 
_AT_@ -164,7 +166,7 @@ drawmenu(void)
                 }
                 x += w;
                 for (item = curr; item != next; item = item->right)
- x = drawitem(item, x, 0, MIN(TEXTW(item->text), mw - x - TEXTW(">")));
+ x = drawitem(item, x, 0, MIN(TEXTWITEM(item), mw - x - TEXTW(">")));
                 if (next) {
                         w = TEXTW(">");
                         drw_setscheme(drw, scheme[SchemeNorm]);
_AT_@ -471,7 +473,7 @@ readstdin(void)
                 if (!(items[i].text = strdup(buf)))
                         die("cannot strdup %u bytes:", strlen(buf) + 1);
                 items[i].out = 0;
- if (inputw < (tmpmax = TEXTW(items[i].text)))
+ if (inputw < (tmpmax = TEXTWITEM(&items[i])))
                         inputw = tmpmax;
         }
         if (items)

-- 
Kind regards,
Hiltjo
Received on Fri Jun 03 2016 - 18:21:58 CEST

This archive was generated by hypermail 2.3.0 : Fri Jun 03 2016 - 18:24:23 CEST