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

Reply via email to