On Fri, Apr 29, 2022 at 08:53:38AM +0600, NRK wrote:
> On Fri, Mar 25, 2022 at 10:57:42PM +0100, git_AT_suckless.org wrote:
> > commit 77526f756e23e362081ac807521f901f2e5cd5e6
> > Author: NRK <nrk_AT_disroot.org>
> > AuthorDate: Thu Mar 24 00:37:55 2022 +0600
> > Commit: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
> > 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
> > _AT_@ -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++) {
> > _AT_@ -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);
> > }
> >
> > _AT_@ -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;
> > _AT_@ -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 <nrk_AT_disroot.org>
> 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
> _AT_@ -23,7 +23,7 @@ config.h:
> $(OBJ): arg.h config.h config.mk drw.h
>
> dmenu: dmenu.o drw.o util.o
> - $(CC) -o $_AT_ dmenu.o drw.o util.o $(LDFLAGS)
> + $(CC) -o $_AT_ dmenu.o drw.o util.o $(LDFLAGS) -lutf8proc
>
> stest: stest.o
> $(CC) -o $_AT_ stest.o $(LDFLAGS)
> diff --git a/dmenu.c b/dmenu.c
> index 8e1c670..f7646a7 100644
> --- a/dmenu.c
> +++ b/dmenu.c
> _AT_@ -606,6 +606,7 @@ run(void)
> }
> }
>
> +#include <utf8proc.h>
> static void
> setup(void)
> {
> _AT_@ -617,6 +618,7 @@ setup(void)
> XWindowAttributes wa;
> XClassHint ch = {"dmenu", "dmenu"};
> struct item *item;
> + char *longest = NULL;
> #ifdef XINERAMA
> XineramaScreenInfo *info;
> Window pw;
> _AT_@ -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
> _AT_@ -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 <nrk_AT_disroot.org>
> 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
> _AT_@ -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
> _AT_@ -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;
> _AT_@ -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 */
> _AT_@ -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
Received on Fri Apr 29 2022 - 10:32:04 CEST