Hi NRK,
I think you've gotten it backwards, this patch should *faster* if the
> string is too long, since we're incrementing up to w instead of
> decerementing down it it.
Yes you are right of course, I was mixing up the two approaches.
I have attached the changes I was experimenting with.
I still don't like the amount of effort required for drawing an ellipsis
though so I attached another patch file that doesn't try to draw the
ellipsis for comparison.
-Stein
On Sat, Mar 19, 2022 at 5:18 PM Silvan Jegen <s.je...@gmail.com> wrote:
> Hi
>
> On Sat, Mar 19, 2022 at 12:59 PM Stein Gunnar Bakkeby <bakk...@gmail.com>
> wrote:
> >
> > Hi NRK,
> >
> > I was tinkering with this for a few hours earlier this week and there
> are several issues with this area of code and not specifically related to
> this modification of the patch.
> >
> > The handling of adding the ellipsis (...) when the text is too long is
> rather hacky and can cause several issues.
> >
> > The general idea is that:
> > - we work out how many consecutive characters we can print for the
> same font
> > - then we check if the length of the string exceeds the remaining
> drawable space
> > - if it is too long then we check the length of the string for every
> character until we find that it no longer fits
> > - then we replace the last three bytes with periods (.)
> >
> > These are the problematic areas I have found:
> >
> > 1. In the original code the text is allowed to bleed into the right
> padding as long as it fits (I presume this is intentional). If you add one
> more character then the text is too long and it will crop of four
> characters to add the ellipsis. This has to do with that the for loop
> reduces len once more than necessary.
> >
> > 2. The adding of the ellipsis naively assumes that the last characters
> in the string are single-byte UTF-8 characters. As such when it replaces
> the last bytes of the string then you can end up with split multi-byte
> UTF-8 characters which can have various effects.
>
> I *think* I somewhat improved this issue in `vis-menu` (which is based
> on `dmenu`, IIRC):
>
>
> https://github.com/Shugyousha/vis/commit/d59b98d934815e54320ad000eebfdaaf8fee344d
>
> Note that this is not well tested and still has issues though.
>
>
> Cheers,
>
> Silvan
>
>
--
Stein Gunnar Bakkeby
OpenBet Developer
bakk...@gmail.com
From a69c9a63ae08b2201ee3ef615a356ace129b7f48 Mon Sep 17 00:00:00 2001
From: Bakkeby <bakk...@gmail.com>
Date: Sun, 20 Mar 2022 13:59:23 +0100
Subject: [PATCH] Fixing split multi-byte issue when drawing ellipsis
---
dmenu.c | 9 +++---
drw.c | 85 ++++++++++++++++++++++++++-------------------------------
drw.h | 3 +-
3 files changed, 46 insertions(+), 51 deletions(-)
diff --git a/dmenu.c b/dmenu.c
index eca67ac..5e1edd6 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -541,7 +541,7 @@ readstdin(void)
{
char buf[sizeof text], *p;
size_t i, imax = 0, size = 0;
- unsigned int tmpmax = 0;
+ XGlyphInfo ext;
/* read each line from stdin and add it to the item list */
for (i = 0; fgets(buf, sizeof buf, stdin); i++) {
@@ -553,9 +553,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;
+ XftTextExtentsUtf8(drw->fonts->dpy, drw->fonts->xfont, (XftChar8 *)buf, strlen(buf), &ext);
+ if (ext.xOff > inputw) {
+ inputw = ext.xOff;
imax = i;
}
}
@@ -768,6 +768,7 @@ main(int argc, char *argv[])
drw = drw_create(dpy, screen, root, wa.width, wa.height);
if (!drw_fontset_create(drw, fonts, LENGTH(fonts)))
die("no fonts could be loaded.");
+ ellipsis_width = TEXTW("...") - lrpad;
lrpad = drw->fonts->h;
#ifdef __OpenBSD__
diff --git a/drw.c b/drw.c
index 4cdbcbe..b82dab4 100644
--- a/drw.c
+++ b/drw.c
@@ -15,6 +15,7 @@ static const unsigned char utfbyte[UTF_SIZ + 1] = {0x80, 0, 0xC0, 0xE0, 0xF0}
static const unsigned char utfmask[UTF_SIZ + 1] = {0xC0, 0x80, 0xE0, 0xF0, 0xF8};
static const long utfmin[UTF_SIZ + 1] = { 0, 0, 0x80, 0x800, 0x10000};
static const long utfmax[UTF_SIZ + 1] = {0x10FFFF, 0x7F, 0x7FF, 0xFFFF, 0x10FFFF};
+unsigned int ellipsis_width = 0;
static long
utf8decodebyte(const char c, size_t *i)
@@ -252,18 +253,18 @@ 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;
+ int ty, stop = 0;
+ unsigned int ew = 0, ellipsis_ew = 0;
XftDraw *d = NULL;
Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
- int utf8strlen, utf8charlen, render = x || y || w || h;
+ size_t i, b = 0, ellipsis_b = 0;
+ int utf8charlen, render = x || y || w || h;
long utf8codepoint = 0;
- const char *utf8str;
FcCharSet *fccharset;
FcPattern *fcpattern;
FcPattern *match;
XftResult result;
+ XGlyphInfo ext;
int charexists = 0;
if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
@@ -283,8 +284,8 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
usedfont = drw->fonts;
while (1) {
- utf8strlen = 0;
- utf8str = text;
+ ellipsis_ew = ew = 0;
+ ellipsis_b = b = 0;
nextfont = NULL;
while (*text) {
utf8charlen = utf8decode(text, &utf8codepoint, UTF_SIZ);
@@ -292,8 +293,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) {
- utf8strlen += utf8charlen;
- text += utf8charlen;
+ XftTextExtentsUtf8(curfont->dpy, curfont->xfont, (XftChar8 *)text, utf8charlen, &ext);
+ 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;
+ b = ellipsis_b;
+ for (i = 0; i < 3; i++)
+ buf[b++] = '.';
+ }
+ stop = 1;
+ break;
+ }
+
+ /* Record the last buffer index where the ellipsis would still fit */
+ if (ew + ellipsis_width + lpad <= w) {
+ ellipsis_ew = ew;
+ ellipsis_b = b;
+ }
+ for (i = 0; i < utf8charlen; i++)
+ buf[b++] = *text++;
+ ew += ext.xOff;
} else {
nextfont = curfont;
}
@@ -301,36 +321,24 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
}
}
- if (!charexists || nextfont)
+ if (stop || !charexists || nextfont)
break;
else
charexists = 0;
}
+ buf[b] = '\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;
- XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
- usedfont->xfont, x, ty, (XftChar8 *)buf, len);
- }
- x += ew;
- w -= ew;
+ if (b) {
+ if (render) {
+ ty = y + (h - usedfont->h) / 2 + usedfont->xfont->ascent;
+ XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
+ usedfont->xfont, x, ty, (XftChar8 *)buf, b);
}
+ x += ew;
+ w -= ew;
}
- if (!*text) {
+ if (stop || !*text) {
break;
} else if (nextfont) {
charexists = 0;
@@ -397,21 +405,6 @@ drw_fontset_getwidth(Drw *drw, const char *text)
return drw_text(drw, 0, 0, 0, 0, 0, text, 0);
}
-void
-drw_font_getexts(Fnt *font, const char *text, unsigned int len, unsigned int *w, unsigned int *h)
-{
- XGlyphInfo ext;
-
- if (!font || !text)
- return;
-
- XftTextExtentsUtf8(font->dpy, font->xfont, (XftChar8 *)text, len, &ext);
- if (w)
- *w = ext.xOff;
- if (h)
- *h = font->h;
-}
-
Cur *
drw_cur_create(Drw *drw, int shape)
{
diff --git a/drw.h b/drw.h
index 4c67419..7c43b3d 100644
--- a/drw.h
+++ b/drw.h
@@ -26,6 +26,8 @@ typedef struct {
Fnt *fonts;
} Drw;
+extern unsigned int ellipsis_width;
+
/* Drawable abstraction */
Drw *drw_create(Display *dpy, int screen, Window win, unsigned int w, unsigned int h);
void drw_resize(Drw *drw, unsigned int w, unsigned int h);
@@ -35,7 +37,6 @@ void drw_free(Drw *drw);
Fnt *drw_fontset_create(Drw* drw, const char *fonts[], size_t fontcount);
void drw_fontset_free(Fnt* set);
unsigned int drw_fontset_getwidth(Drw *drw, const char *text);
-void drw_font_getexts(Fnt *font, const char *text, unsigned int len, unsigned int *w, unsigned int *h);
/* Colorscheme abstraction */
void drw_clr_create(Drw *drw, Clr *dest, const char *clrname);
--
2.35.1
From 26effbb589dbb175bc35020910cd09a45501ae38 Mon Sep 17 00:00:00 2001
From: Bakkeby <bakk...@gmail.com>
Date: Sun, 20 Mar 2022 13:59:23 +0100
Subject: [PATCH] Fixing split multi-byte issue and removing ellipsis drawing
---
dmenu.c | 8 +++----
drw.c | 71 ++++++++++++++++++++-------------------------------------
drw.h | 1 -
3 files changed, 29 insertions(+), 51 deletions(-)
diff --git a/dmenu.c b/dmenu.c
index eca67ac..22eecae 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -541,7 +541,7 @@ readstdin(void)
{
char buf[sizeof text], *p;
size_t i, imax = 0, size = 0;
- unsigned int tmpmax = 0;
+ XGlyphInfo ext;
/* read each line from stdin and add it to the item list */
for (i = 0; fgets(buf, sizeof buf, stdin); i++) {
@@ -553,9 +553,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;
+ XftTextExtentsUtf8(drw->fonts->dpy, drw->fonts->xfont, (XftChar8 *)buf, strlen(buf), &ext);
+ if (ext.xOff > inputw) {
+ inputw = ext.xOff;
imax = i;
}
}
diff --git a/drw.c b/drw.c
index 4cdbcbe..556b79a 100644
--- a/drw.c
+++ b/drw.c
@@ -252,18 +252,18 @@ 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;
+ int ty, stop = 0;
+ unsigned int ew = 0;
XftDraw *d = NULL;
Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
- int utf8strlen, utf8charlen, render = x || y || w || h;
+ size_t i, b = 0;
+ int utf8charlen, render = x || y || w || h;
long utf8codepoint = 0;
- const char *utf8str;
FcCharSet *fccharset;
FcPattern *fcpattern;
FcPattern *match;
XftResult result;
+ XGlyphInfo ext;
int charexists = 0;
if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
@@ -283,8 +283,8 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
usedfont = drw->fonts;
while (1) {
- utf8strlen = 0;
- utf8str = text;
+ ew = 0;
+ b = 0;
nextfont = NULL;
while (*text) {
utf8charlen = utf8decode(text, &utf8codepoint, UTF_SIZ);
@@ -292,8 +292,14 @@ 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) {
- utf8strlen += utf8charlen;
- text += utf8charlen;
+ XftTextExtentsUtf8(curfont->dpy, curfont->xfont, (XftChar8 *)text, utf8charlen, &ext);
+ if (ew + ext.xOff + lpad > w || b + utf8charlen > sizeof(buf) - 1) {
+ stop = 1;
+ break;
+ }
+ for (i = 0; i < utf8charlen; i++)
+ buf[b++] = *text++;
+ ew += ext.xOff;
} else {
nextfont = curfont;
}
@@ -301,36 +307,24 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
}
}
- if (!charexists || nextfont)
+ if (stop || !charexists || nextfont)
break;
else
charexists = 0;
}
+ buf[b] = '\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;
- XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
- usedfont->xfont, x, ty, (XftChar8 *)buf, len);
- }
- x += ew;
- w -= ew;
+ if (b) {
+ if (render) {
+ ty = y + (h - usedfont->h) / 2 + usedfont->xfont->ascent;
+ XftDrawStringUtf8(d, &drw->scheme[invert ? ColBg : ColFg],
+ usedfont->xfont, x, ty, (XftChar8 *)buf, b);
}
+ x += ew;
+ w -= ew;
}
- if (!*text) {
+ if (stop || !*text) {
break;
} else if (nextfont) {
charexists = 0;
@@ -397,21 +391,6 @@ drw_fontset_getwidth(Drw *drw, const char *text)
return drw_text(drw, 0, 0, 0, 0, 0, text, 0);
}
-void
-drw_font_getexts(Fnt *font, const char *text, unsigned int len, unsigned int *w, unsigned int *h)
-{
- XGlyphInfo ext;
-
- if (!font || !text)
- return;
-
- XftTextExtentsUtf8(font->dpy, font->xfont, (XftChar8 *)text, len, &ext);
- if (w)
- *w = ext.xOff;
- if (h)
- *h = font->h;
-}
-
Cur *
drw_cur_create(Drw *drw, int shape)
{
diff --git a/drw.h b/drw.h
index 4c67419..1e910a0 100644
--- a/drw.h
+++ b/drw.h
@@ -35,7 +35,6 @@ void drw_free(Drw *drw);
Fnt *drw_fontset_create(Drw* drw, const char *fonts[], size_t fontcount);
void drw_fontset_free(Fnt* set);
unsigned int drw_fontset_getwidth(Drw *drw, const char *text);
-void drw_font_getexts(Fnt *font, const char *text, unsigned int len, unsigned int *w, unsigned int *h);
/* Colorscheme abstraction */
void drw_clr_create(Drw *drw, Clr *dest, const char *clrname);
--
2.35.1
