On Fri, Apr 29, 2022 at 08:53:38AM +0600, NRK wrote: > On Fri, Mar 25, 2022 at 10:57:42PM +0100, [email protected] wrote: > > commit 77526f756e23e362081ac807521f901f2e5cd5e6 > > Author: NRK <[email protected]> > > AuthorDate: Thu Mar 24 00:37:55 2022 +0600 > > Commit: Hiltjo Posthuma <[email protected]> > > CommitDate: Fri Mar 25 22:49:07 2022 +0100 > > > > inputw: improve correctness and startup performance > > > > a massive amount of time inside readstdin() is spent trying to get the > > max input width and then put it into inputw, only for it to get clamped > > down to mw/3 inside setup(). > > > > it makes more sense to calculate inputw inside setup() once we have mw > > available. similar to the last patch, i see noticeable startup > > performance improvement: > > > > before -> after > > 160ms -> 60ms > > > > additionally this will take fallback fonts into account compared to the > > previous version, so it's not only more performant but also more > > correct. > > > > diff --git a/dmenu.c b/dmenu.c > > index cde394b..d989d39 100644 > > --- a/dmenu.c > > +++ b/dmenu.c > > @@ -547,8 +547,7 @@ static void > > readstdin(void) > > { > > char buf[sizeof text], *p; > > - size_t i, imax = 0, size = 0; > > - unsigned int tmpmax = 0; > > + size_t i, size = 0; > > > > /* read each line from stdin and add it to the item list */ > > for (i = 0; fgets(buf, sizeof buf, stdin); i++) { > > @@ -560,15 +559,9 @@ readstdin(void) > > if (!(items[i].text = strdup(buf))) > > die("cannot strdup %u bytes:", strlen(buf) + 1); > > items[i].out = 0; > > - drw_font_getexts(drw->fonts, buf, strlen(buf), &tmpmax, NULL); > > - if (tmpmax > inputw) { > > - inputw = tmpmax; > > - imax = i; > > - } > > } > > if (items) > > items[i].text = NULL; > > - inputw = items ? TEXTW(items[imax].text) : 0; > > lines = MIN(lines, i); > > } > > > > @@ -614,12 +607,13 @@ static void > > setup(void) > > { > > int x, y, i, j; > > - unsigned int du; > > + unsigned int du, tmp; > > XSetWindowAttributes swa; > > XIM xim; > > Window w, dw, *dws; > > XWindowAttributes wa; > > XClassHint ch = {"dmenu", "dmenu"}; > > + struct item *item; > > #ifdef XINERAMA > > XineramaScreenInfo *info; > > Window pw; > > @@ -677,7 +671,12 @@ setup(void) > > mw = wa.width; > > } > > promptw = (prompt && *prompt) ? TEXTW(prompt) - lrpad / 4 : 0; > > - inputw = MIN(inputw, mw/3); > > + for (item = items; item && item->text; ++item) { > > + if ((tmp = textw_clamp(item->text, mw/3)) > inputw) { > > + if ((inputw = tmp) == mw/3) > > + break; > > + } > > + } > > match(); > > > > /* create menu window */ > > > > Hi, > > While this patch improves startup speed on larger strings, it caused a > startup performance regression when dealing with lots of small strings > *with* many missing glyphs[0]. > > Because this patch correctly takes fallback font into account, it'll > inevitably end up making lots of `XftFontMatch` calls, which are deadly > slow. > > For example, trying to load this [1] list of emojis on my machine now > takes slightly more than half a second. Whereas before it took about > 16ms. > > The following are a couple different solutions I can think of: > > 1. (Incorrectly) stop taking fallback font into account. > > 2. (Incorrectly) assume `more bytes == wider string`, which is not > correct thanks to unicode. > > 3. Try to get the width of the unicode code-point. I've attached a quick > and dirty patch using `utf8proc_charwidth()` from libutf8-proc. The > patch was just to confirm my hypothesis and not to be taken seriously. > > I'm not too well versed on unicode so I cannot tell how difficult > rolling such a function ourself would be. But some quick searches seems > to indicate it's not going to be trivial at all, specially if we take > "grapheme clusters" into account. > > So this option is probably getting ruled out. > > 4. The final option; just remove the dynamic input bar width entirely. > If you're unsure what I'm talking about, try the following: > > head -c 65536 < /dev/urandom | base64 | fold -w 1 | dmenu > > You will notice that the width of the input bar is rather small, which > allows for more options to fit into the screen. > > Now try replacing `-w 1` with `-w 30`, you will notice the width got > bigger. Now try `-w 200`, the width gets bigger, but will get clamped > down to 1/3rd of the monitor width. > > My suggestion here is to just have a consistent input bar width which > can be configured via config.h and cli arg. So for example: > > static float input_bar_percent = 0.24f; > > This would make the input bar width always 24% of the monitor width. > I've attached a patch for this as well. It's simpler and gives a more > static/predicable ui. > > [0]: https://github.com/bakkeby/dmenu-flexipatch/issues/11 > [1]: > https://github.com/LukeSmithxyz/voidrice/blob/master/.local/share/larbs/emoji > > - NRK
> From 7b6d422a6d49bfa472822dfb292eefde78a346a2 Mon Sep 17 00:00:00 2001 > From: NRK <[email protected]> > Date: Thu, 28 Apr 2022 13:04:16 +0600 > Subject: [PATCH] utf8proc > > --- > Makefile | 2 +- > dmenu.c | 21 ++++++++++++++++++--- > drw.c | 2 +- > 3 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index a03a95c..89b5ac7 100644 > --- a/Makefile > +++ b/Makefile > @@ -23,7 +23,7 @@ config.h: > $(OBJ): arg.h config.h config.mk drw.h > > dmenu: dmenu.o drw.o util.o > - $(CC) -o $@ dmenu.o drw.o util.o $(LDFLAGS) > + $(CC) -o $@ dmenu.o drw.o util.o $(LDFLAGS) -lutf8proc > > stest: stest.o > $(CC) -o $@ stest.o $(LDFLAGS) > diff --git a/dmenu.c b/dmenu.c > index 8e1c670..f7646a7 100644 > --- a/dmenu.c > +++ b/dmenu.c > @@ -606,6 +606,7 @@ run(void) > } > } > > +#include <utf8proc.h> > static void > setup(void) > { > @@ -617,6 +618,7 @@ setup(void) > XWindowAttributes wa; > XClassHint ch = {"dmenu", "dmenu"}; > struct item *item; > + char *longest = NULL; > #ifdef XINERAMA > XineramaScreenInfo *info; > Window pw; > @@ -675,11 +677,24 @@ setup(void) > } > promptw = (prompt && *prompt) ? TEXTW(prompt) - lrpad / 4 : 0; > for (item = items; !lines && item && item->text; ++item) { > - if ((tmp = textw_clamp(item->text, mw/3)) > inputw) { > - if ((inputw = tmp) == mw/3) > - break; > + extern size_t utf8decode(const char *c, long *u, size_t clen); > + char *l; > + int utf8charlen; > + long utf8codepoint; > + > + l = item->text; tmp = 0; > + while (l && *l) { > + utf8charlen = utf8decode(l, &utf8codepoint, 4); > + l += utf8charlen; > + tmp += utf8proc_charwidth(utf8codepoint); > + } > + > + if (tmp > inputw) { > + longest = item->text; > + inputw = tmp; > } > } > + if (lines == 0 && longest) inputw = textw_clamp(longest, mw/3); > match(); > > /* create menu window */ > diff --git a/drw.c b/drw.c > index ced7d37..3f5adc1 100644 > --- a/drw.c > +++ b/drw.c > @@ -35,7 +35,7 @@ utf8validate(long *u, size_t i) > return i; > } > > -static size_t > +size_t > utf8decode(const char *c, long *u, size_t clen) > { > size_t i, j, len, type; > -- > 2.35.1 > > From 91c4d9f8f8ef21613be770792254a964f8d4ec3b Mon Sep 17 00:00:00 2001 > From: NRK <[email protected]> > Date: Thu, 28 Apr 2022 15:44:01 +0600 > Subject: [PATCH] make the input bar width static > > --- > config.def.h | 6 ++++++ > dmenu.c | 13 +++++-------- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/config.def.h b/config.def.h > index 1edb647..7cd59da 100644 > --- a/config.def.h > +++ b/config.def.h > @@ -21,3 +21,9 @@ static unsigned int lines = 0; > * for example: " /?\"&[]" > */ > static const char worddelimiters[] = " "; > + > +/* > + * Width of the input bar, relative to the monitor's width. > + * Valid values = [0.10 .. 0.50] > + */ > +static float inputbar_width = 0.18f; > diff --git a/dmenu.c b/dmenu.c > index 839f6cc..ba43f7c 100644 > --- a/dmenu.c > +++ b/dmenu.c > @@ -610,13 +610,12 @@ static void > setup(void) > { > int x, y, i, j; > - unsigned int du, tmp; > + unsigned int du; > XSetWindowAttributes swa; > XIM xim; > Window w, dw, *dws; > XWindowAttributes wa; > XClassHint ch = {"dmenu", "dmenu"}; > - struct item *item; > #ifdef XINERAMA > XineramaScreenInfo *info; > Window pw; > @@ -674,12 +673,8 @@ setup(void) > mw = wa.width; > } > promptw = (prompt && *prompt) ? TEXTW(prompt) - lrpad / 4 : 0; > - for (item = items; item && item->text; ++item) { > - if ((tmp = textw_clamp(item->text, mw/3)) > inputw) { > - if ((inputw = tmp) == mw/3) > - break; > - } > - } > + inputbar_width = MAX(0.10f, MIN(0.50f, inputbar_width)); > + inputw = mw * inputbar_width; > match(); > > /* create menu window */ > @@ -742,6 +737,8 @@ main(int argc, char *argv[]) > } else if (i + 1 == argc) > usage(); > /* these options take one argument */ > + else if (!strcmp(argv[i], "-iw")) /* input bar width */ > + inputbar_width = atoi(argv[++i]) / 100.0f; > else if (!strcmp(argv[i], "-l")) /* number of lines in > vertical list */ > lines = atoi(argv[++i]); > else if (!strcmp(argv[i], "-m")) > -- > 2.35.1 > Thank you for the report, I will do some testing (this weekend probably). -- Kind regards, Hiltjo
