Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large stringsa
On Mon, Mar 21, 2022 at 10:35:21PM +0100, Stein Gunnar Bakkeby wrote:
> you make some interesting points. I am curious as to what your queueing
> approach would look like.
>
> I played around some more simplifying the ellipsis drawing and removing buf
> as you suggested.
> This would solve all of the aforementioned problems as far as I can tell,
> but it can result in a partly drawn
> emoji for example when the ellipsis cuts it off (which I think is a fair
> tradeoff).
Hi,
Tried out your last patch, it's simpler indeed. But I still see the
prompt getting incorrectly cut off and the ellipsis still don't get
rendered in case of font change.
The queue patch solves the above problems, but as I've said it's quite
messy. I was planning to move everything over to the queue so that we
won't need to special case anything during string drawing.
But after looking at your current patch, I've scraped that idea. It's
simpler to just draw on top of the problem cases, overwriting them
instead.
The following diff fixes pretty much every problem for me. The
performance is better and I don't notice any problems with truncating.
Let me know if there's still some edge case unhandled.
- NRK
diff --git a/drw.c b/drw.c
index 4cdbcbe..8595a69 100644
--- a/drw.c
+++ b/drw.c
@@ -251,12 +251,10 @@ drw_rect(Drw *drw, int x, int y, unsigned int w, unsigned
int h, int filled, int
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, ellipsis_x = 0;
+ unsigned int tmpw, ew, ellipsis_w, ellipsis_len;
XftDraw *d = NULL;
Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
int utf8strlen, utf8charlen, render = x || y || w || h;
long utf8codepoint = 0;
const char *utf8str;
@@ -264,7 +262,7 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned
int h, unsigned int lp
FcPattern *fcpattern;
FcPattern *match;
XftResult result;
- int charexists = 0;
+ int charexists = 0, overflow = 0;
if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
return 0;
@@ -282,8 +280,9 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned
int h, unsigned int lp
}
usedfont = drw->fonts;
+ drw_font_getexts(usedfont, "...", 3, _w, NULL);
while (1) {
- utf8strlen = 0;
+ ew = ellipsis_len = utf8strlen = 0;
utf8str = text;
nextfont = NULL;
while (*text) {
@@ -291,9 +290,19 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned
int h, unsigned int lp
for (curfont = drw->fonts; curfont; curfont =
curfont->next) {
charexists = charexists ||
XftCharExists(drw->dpy, curfont->xfont, utf8codepoint);
if (charexists) {
- if (curfont == usedfont) {
+ drw_font_getexts(curfont, text,
utf8charlen, , NULL);
+ if (ew + ellipsis_w <= w) {
+ ellipsis_x = x + ew;
+ ellipsis_len = utf8strlen;
+ }
+
+ if (ew > w) {
+ overflow = 1;
+ utf8strlen = ellipsis_len;
+ } else if (curfont == usedfont) {
utf8strlen += utf8charlen;
text += utf8charlen;
+ ew += tmpw;
} else {
nextfont = curfont;
}
@@ -301,36 +310,25 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned
int h, unsigned int lp
}
}
- if (!charexists || nextfont)
+ if (overflow || !charexists || nextfont)
break;
else
charexists = 0;
}
if (utf8strlen) {
- drw_font_getexts(usedfont, utf8str, utf8strlen, ,
NULL);
- /* shorten text if necessary */
- for (len = MIN(utf8strlen, sizeof(buf) - 1); len && ew
> w; len--)
- drw_font_getexts(usedfont, utf8str, len, ,
NULL);
-
- if (len) {
- memcpy(buf, utf8str, len);
- buf[len] = '\0';
- if (len <
Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large strings
Hi NRK,
you make some interesting points. I am curious as to what your queueing
approach would look like.
I played around some more simplifying the ellipsis drawing and removing buf
as you suggested.
This would solve all of the aforementioned problems as far as I can tell,
but it can result in a partly drawn
emoji for example when the ellipsis cuts it off (which I think is a fair
tradeoff).
Thanks,
-Stein
On Mon, Mar 21, 2022 at 8:15 PM NRK wrote:
> On Mon, Mar 21, 2022 at 07:00:32PM +0600, NRK wrote:
> > The only "issue" is that it doesn't render the ellipsis in case font
> > changes, but current upstream dmenu doesn't seem to do it either.
>
> OK, I think I have a solution to this. The problem here, as far as I
> understand, is this:
>
> Let's say we have a maxwidth of 100, and printing the ellipsis takes 20.
> In this case as long as our string is below 80, we're fine; and if it's
> above 100 then we should print up to 80 and then print the ellipsis.
>
> The problem case is when we're between 80-100, and we need to change
> font. The current code always renders whatever is available when we
> change font, so if the text turns out to be bigger than 100 we've
> already rendered some of the text into the problem area where the
> ellipsis should've been.
>
> The solution I came up with is to simply not render anything if we're at
> the problem case, and instead put the problem cases into a queue and
> keep going forwards until either:
>
> 1) The text overflows, in which case we discard the queue and just print
>the ellipsis instead.
> 2) Encounter the end of text, which means the text barely fit into our
>maxwidth (this would happen with the prompt); in which case we just
>render the queue and don't print any ellipsis.
>
> I already have a patch that *roughly* does what I described above and
> with it applied I don't see any truncation problems anymore.
>
> However the patch is quite messy atm, and in order to do things cleanly
> and avoid special casing, I think a good portion of drw_text() will need
> to be overhauled to use a queue like this:
>
> struct { const char *str; int len; Fnt *fnt; } q[32] = {0};
>
> I'm not sure if such overhaul is going to be welcome or not, but if all
> of this sounds acceptable then I can proceed with cleaning things up and
> supplying the patch.
>
> - NRK
>
>
--
Stein Gunnar Bakkeby
OpenBet Developer
bakk...@gmail.com
From d52d59bc32a9e5bb3f7dfc0a88a0271b9870 Mon Sep 17 00:00:00 2001
From: Bakkeby
Date: Mon, 21 Mar 2022 22:25:51 +0100
Subject: [PATCH] drw_text: print whole utf-8 characters only and simplify
ellipsis handling
---
drw.c | 50 +-
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drw.c b/drw.c
index 4cdbcbe..ccb3b9a 100644
--- a/drw.c
+++ b/drw.c
@@ -251,12 +251,10 @@ drw_rect(Drw *drw, int x, int y, unsigned int w, unsigned int h, int filled, int
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;
XftDraw *d = NULL;
Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
int utf8strlen, utf8charlen, render = x || y || w || h;
long utf8codepoint = 0;
const char *utf8str;
@@ -264,7 +262,9 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
FcPattern *fcpattern;
FcPattern *match;
XftResult result;
- int charexists = 0;
+ int charexists = 0, overflow = 0;
+ XGlyphInfo ext;
+ const char *ellipsis = "...";
if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
return 0;
@@ -278,7 +278,7 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
DefaultVisual(drw->dpy, drw->screen),
DefaultColormap(drw->dpy, drw->screen));
x += lpad;
- w -= lpad;
+ w -= lpad * 2;
}
usedfont = drw->fonts;
@@ -286,12 +286,19 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
utf8strlen = 0;
utf8str = text;
nextfont = NULL;
+ ew = 0;
while (*text) {
utf8charlen = utf8decode(text, , UTF_SIZ);
for (curfont = drw->fonts; curfont; curfont = curfont->next) {
charexists = charexists || XftCharExists(drw->dpy, curfont->xfont, utf8codepoint);
if (charexists) {
if (curfont == usedfont) {
+ XftTextExtentsUtf8(usedfont->dpy, usedfont->xfont, (XftChar8 *)text, utf8charlen, );
+ if (ew + ext.xOff > w) {
+ overflow = 1;
+ break;
+ }
+ ew += ext.xOff;
utf8strlen += utf8charlen;
text += utf8charlen;
} else {
@@ -301,36 +308,29 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
}
}
- if (!charexists || nextfont)
+ if (overflow || !charexists || nextfont)
break;
else
charexists = 0;
}
if (utf8strlen) {
- drw_font_getexts(usedfont, utf8str, utf8strlen, , NULL);
-
Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large strings
On Mon, Mar 21, 2022 at 07:00:32PM +0600, NRK wrote:
> The only "issue" is that it doesn't render the ellipsis in case font
> changes, but current upstream dmenu doesn't seem to do it either.
OK, I think I have a solution to this. The problem here, as far as I
understand, is this:
Let's say we have a maxwidth of 100, and printing the ellipsis takes 20.
In this case as long as our string is below 80, we're fine; and if it's
above 100 then we should print up to 80 and then print the ellipsis.
The problem case is when we're between 80-100, and we need to change
font. The current code always renders whatever is available when we
change font, so if the text turns out to be bigger than 100 we've
already rendered some of the text into the problem area where the
ellipsis should've been.
The solution I came up with is to simply not render anything if we're at
the problem case, and instead put the problem cases into a queue and
keep going forwards until either:
1) The text overflows, in which case we discard the queue and just print
the ellipsis instead.
2) Encounter the end of text, which means the text barely fit into our
maxwidth (this would happen with the prompt); in which case we just
render the queue and don't print any ellipsis.
I already have a patch that *roughly* does what I described above and
with it applied I don't see any truncation problems anymore.
However the patch is quite messy atm, and in order to do things cleanly
and avoid special casing, I think a good portion of drw_text() will need
to be overhauled to use a queue like this:
struct { const char *str; int len; Fnt *fnt; } q[32] = {0};
I'm not sure if such overhaul is going to be welcome or not, but if all
of this sounds acceptable then I can proceed with cleaning things up and
supplying the patch.
- NRK
[hackers][surf] fix compiler warnings
- Replace %d with %ld specifiers for long unsigned int variables.
- g_variant_get expects Gvariant* instead of const Gvariant*.
Signed-off-by: Prathu Baronia
---
surf.c| 6 +++---
webext-surf.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/surf.c b/surf.c
index 03d8242..069d8ec 100644
--- a/surf.c
+++ b/surf.c
@@ -1237,7 +1237,7 @@ readsock(GIOChannel *s, GIOCondition ioc, gpointer unused)
return TRUE;
}
if (msgsz < 2) {
- fprintf(stderr, "surf: message too short: %d\n", msgsz);
+ fprintf(stderr, "surf: message too short: %ld\n", msgsz);
return TRUE;
}
@@ -1863,14 +1863,14 @@ msgext(Client *c, char type, const Arg *a)
if (spair[0] < 0)
return;
- if ((ret = snprintf(msg, sizeof(msg), "%c%c%c", c->pageid, type, a->i))
+ if ((ret = snprintf(msg, sizeof(msg), "%ld%c%c", c->pageid, type, a->i))
>= sizeof(msg)) {
fprintf(stderr, "surf: message too long: %d\n", ret);
return;
}
if (send(spair[0], msg, ret, 0) != ret)
- fprintf(stderr, "surf: error sending: %u%c%d (%d)\n",
+ fprintf(stderr, "surf: error sending: %ld%c%d (%d)\n",
c->pageid, type, a->i, ret);
}
diff --git a/webext-surf.c b/webext-surf.c
index d087219..f852c3e 100644
--- a/webext-surf.c
+++ b/webext-surf.c
@@ -25,7 +25,7 @@ msgsurf(guint64 pageid, const char *s)
size_t sln = strlen(s);
int ret;
- if ((ret = snprintf(msg, sizeof(msg), "%c%s", pageid, s))
+ if ((ret = snprintf(msg, sizeof(msg), "%ld%s", pageid, s))
>= sizeof(msg)) {
fprintf(stderr, "webext: msg: message too long: %d\n", ret);
return;
@@ -55,7 +55,7 @@ readsock(GIOChannel *s, GIOCondition c, gpointer unused)
}
if (msgsz < 2) {
- fprintf(stderr, "webext: readsock: message too short: %d\n",
+ fprintf(stderr, "webext: readsock: message too short: %ld\n",
msgsz);
return TRUE;
}
@@ -95,7 +95,7 @@
webkit_web_extension_initialize_with_user_data(WebKitWebExtension *e,
webext = e;
- g_variant_get(gv, "i", );
+ g_variant_get((GVariant *)gv, "i", );
gchansock = g_io_channel_unix_new(sock);
g_io_channel_set_encoding(gchansock, NULL, NULL);
--
2.17.1
Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large strings
+ 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;
[...]
+ /* Record the last buffer index
where the ellipsis would still fit */
+ if (ew + ellipsis_width + lpad
<= w) {
+ ellipsis_ew = ew;
+ ellipsis_b = b;
+ }
I think both of the `+ lpad` needs be removed. Otherwise it incorrectly
truncates the prompt as well.
./dmenu < /dev/null -p "p" # empty prompt
./dmenu < /dev/null -p "prompt" # truncated prompt
Also, I didn't quite get why there's a `ellipsis_b > 3` in there.
+ for (i = 0; i < utf8charlen;
i++)
+ buf[b++] = *text++;
I'm kinda wondering if `buf` is even worth it or not. We could just render the
"..." separately. On my system atleast, there is no noticeable performance
difference, but removing `buf` from the equation (IMO) makes things more
simpler and easier to follow.
The following is what I've gotten so far, it's working fine and I haven't
noticed any regressions. The only "issue" is that it doesn't render the
ellipsis in case font changes, but current upstream dmenu doesn't seem to do it
either.
- NRK
diff --git a/drw.c b/drw.c
index 4cdbcbe..80dcad2 100644
--- a/drw.c
+++ b/drw.c
@@ -251,20 +251,17 @@ drw_rect(Drw *drw, int x, int y, unsigned int w, unsigned
int h, int filled, int
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;
+ unsigned int ew = 0, ellipsis_ew = 0, ellipsis_width = 0, tmpw;
XftDraw *d = NULL;
Fnt *usedfont, *curfont, *nextfont;
- size_t i, len;
- int utf8strlen, utf8charlen, render = x || y || w || h;
+ int utf8strlen, utf8charlen, ellipsis_len, render = x || y || w || h;
long utf8codepoint = 0;
const char *utf8str;
FcCharSet *fccharset;
FcPattern *fcpattern;
FcPattern *match;
XftResult result;
- int charexists = 0;
+ int charexists = 0, truncate = 0;
if (!drw || (render && !drw->scheme) || !text || !drw->fonts)
return 0;
@@ -283,7 +280,7 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned
int h, unsigned int lp
usedfont = drw->fonts;
while (1) {
- utf8strlen = 0;
+ utf8strlen = ellipsis_len = ew = ellipsis_ew = 0;
utf8str = text;
nextfont = NULL;
while (*text) {
@@ -292,8 +289,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) {
+ if (!ellipsis_width)
+
drw_font_getexts(curfont, "...", 3, _width, NULL);
+ drw_font_getexts(curfont, text,
utf8charlen, , NULL);
+ if (ew + tmpw > w) {
+ /* Only draw ellipsis
if we have not recently started another font */
+ if (render &&
ellipsis_len > 0) {
+ ew =
ellipsis_ew;
+ utf8strlen =
ellipsis_len;
+ }
+ truncate = 1;
+ break;
+ }
+
+ /* Record the last text index
where the ellipsis would still fit */
+ if (ew + ellipsis_width <= w) {
+ ellipsis_ew = ew;
+ ellipsis_len =
utf8strlen;
+ }
utf8strlen += utf8charlen;
Re: [hackers] [st][PATCH v2] code-golfing: cleanup osc color related code
On Mon, Mar 21, 2022 at 09:24:50AM +0100, Roberto E. Vargas Caballero wrote:
> I think is better to keep every ternary in a line by itself, because it makes
> easier to read them:
>
> fprintf(stderr, "erresc: failed to fetch %s color %d\n",
> is_osc4 ? "osc4" : "osc",
> is_osc4 ? num : index);
Done.
> I think we force here to put braces around if and else because the body of the
> if part is more of one line. Again, I think is better to use a line for every
> ternary and have something like:
>
>
> if (n < 0 || n >= sizeof(buf)) {
> fprintf(stderr, "error: %s while printing %s response\n",
> n < 0 ? "snprintf failed" : "truncation occurred",
> is_osc4 ? "osc4" : "osc");
> } else {
> ttywrite(buf, n, 1);
> }
>
> > + if ((j = par - 10) < 0 || j >= LEN(osc_table))
> > + break; /* shouldn't be possible */
> >
> > if (!strcmp(p, "?"))
> > - osc_color_response(defaultcs, 12);
> > - else if (xsetcolorname(defaultcs, p))
> > - fprintf(stderr, "erresc: invalid cursor color:
> > %s\n", p);
> > + osc_color_response(par, osc_table[j].idx, 0);
> > + else if (xsetcolorname(osc_table[j].idx, p))
> > + fprintf(stderr, "erresc: invalid %s color:
> > %s\n",
> > + osc_table[j].str, p);
> > else
> > tfulldirt();
> > return;
>
> Same apply here, I think our style forces to have braces in every if and else
> if
> because there is a body with more of one lines.
I agree with that, but I wasn't sure what the preferred style here is
since I've seen a lot of multi-line bodies have no braces, most notably
on the DWM source code. Though, looking around a bit, the ST source
seems to be consistent with it's braces.
Attached the amended patch.
- NRK
>From cab20470fd6c6f3832f10427b1970df5acf78245 Mon Sep 17 00:00:00 2001
From: NRK
Date: Fri, 7 Jan 2022 23:21:04 +0600
Subject: [PATCH] code-golfing: cleanup osc color related code
* adds missing function prototype
* move xgetcolor() prototype to win.h (that's where all the other x.c
func prototype seems to be declared at)
* check for snprintf error/truncation
* reduces code duplication for osc 10/11/12
* unify osc_color_response() and osc4_color_response() into a single function
the latter two was suggested by Quentin Rameau in his patch review on
the hackers list.
---
st.c | 90 +--
st.h | 2 --
win.h | 1 +
3 files changed, 33 insertions(+), 60 deletions(-)
diff --git a/st.c b/st.c
index c71fa06..7275da5 100644
--- a/st.c
+++ b/st.c
@@ -161,6 +161,7 @@ static void csidump(void);
static void csihandle(void);
static void csiparse(void);
static void csireset(void);
+static void osc_color_response(int, int, int);
static int eschandle(uchar);
static void strdump(void);
static void strhandle(void);
@@ -1843,39 +1844,28 @@ csireset(void)
}
void
-osc4_color_response(int num)
+osc_color_response(int num, int index, int is_osc4)
{
int n;
char buf[32];
unsigned char r, g, b;
- if (xgetcolor(num, , , )) {
- fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num);
+ if (xgetcolor(is_osc4 ? num : index, , , )) {
+ fprintf(stderr, "erresc: failed to fetch %s color %d\n",
+ is_osc4 ? "osc4" : "osc",
+ is_osc4 ? num : index);
return;
}
- n = snprintf(buf, sizeof buf, "\033]4;%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
- num, r, r, g, g, b, b);
-
- ttywrite(buf, n, 1);
-}
-
-void
-osc_color_response(int index, int num)
-{
- int n;
- char buf[32];
- unsigned char r, g, b;
-
- if (xgetcolor(index, , , )) {
- fprintf(stderr, "erresc: failed to fetch osc color %d\n", index);
- return;
+ n = snprintf(buf, sizeof buf, "\033]%s%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
+ is_osc4 ? "4;" : "", num, r, r, g, g, b, b);
+ if (n < 0 || n >= sizeof(buf)) {
+ fprintf(stderr, "error: %s while printing %s response\n",
+ n < 0 ? "snprintf failed" : "truncation occurred",
+ is_osc4 ? "osc4" : "osc");
+ } else {
+ ttywrite(buf, n, 1);
}
-
- n = snprintf(buf, sizeof buf, "\033]%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
- num, r, r, g, g, b, b);
-
- ttywrite(buf, n, 1);
}
void
@@ -1883,6 +1873,11 @@ strhandle(void)
{
char *p = NULL, *dec;
int j, narg, par;
+ const struct { int idx; char *str; } osc_table[] = {
+ { defaultfg, "foreground" },
+ { defaultbg, "background" },
+ { defaultcs, "cursor" }
+ };
term.esc &= ~(ESC_STR_END|ESC_STR);
strparse();
@@ -1917,43 +1912,22 @@ strhandle(void)
}
return;
case 10:
- if (narg < 2)
-break;
-
- p = strescseq.args[1];
-
- if
Re: [hackers] [st][PATCH v2] code-golfing: cleanup osc color related code
Hi,
A few small nitpicks about formating (fell free to ignore them
if you want ;)):
On Sun, Mar 20, 2022 at 06:25:40PM +0600, NRK wrote:
> @@ -1843,39 +1844,25 @@ csireset(void)
> }
>
> void
> -osc4_color_response(int num)
> +osc_color_response(int num, int index, int is_osc4)
> {
> int n;
> char buf[32];
> unsigned char r, g, b;
>
> - if (xgetcolor(num, , , )) {
> - fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num);
> + if (xgetcolor(is_osc4 ? num : index, , , )) {
> + fprintf(stderr, "erresc: failed to fetch %s color %d\n",
> + is_osc4 ? "osc4" : "osc", is_osc4 ? num : index);
I think is better to keep every ternary in a line by itself, because it makes
easier to read them:
fprintf(stderr, "erresc: failed to fetch %s color %d\n",
is_osc4 ? "osc4" : "osc",
is_osc4 ? num : index);
> + n = snprintf(buf, sizeof buf,
> "\033]%s%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
> + is_osc4 ? "4;" : "", num, r, r, g, g, b, b);
> + if (n < 0 || n >= sizeof(buf))
> + fprintf(stderr, "error: %s while printing %s response\n", n < 0
> ?
> + "snprintf failed" : "truncation occurred", is_osc4 ?
> "osc4" : "osc");
> + else
> + ttywrite(buf, n, 1);
> }
I think we force here to put braces around if and else because the body of the
if part is more of one line. Again, I think is better to use a line for every
ternary and have something like:
if (n < 0 || n >= sizeof(buf)) {
fprintf(stderr, "error: %s while printing %s response\n",
n < 0 ? "snprintf failed" : "truncation occurred",
is_osc4 ? "osc4" : "osc");
} else {
ttywrite(buf, n, 1);
}
> + if ((j = par - 10) < 0 || j >= LEN(osc_table))
> + break; /* shouldn't be possible */
>
> if (!strcmp(p, "?"))
> - osc_color_response(defaultcs, 12);
> - else if (xsetcolorname(defaultcs, p))
> - fprintf(stderr, "erresc: invalid cursor color:
> %s\n", p);
> + osc_color_response(par, osc_table[j].idx, 0);
> + else if (xsetcolorname(osc_table[j].idx, p))
> + fprintf(stderr, "erresc: invalid %s color:
> %s\n",
> + osc_table[j].str, p);
> else
> tfulldirt();
> return;
Same apply here, I think our style forces to have braces in every if and else if
because there is a body with more of one lines.
Regards,
Re: [hackers] [dmenu|libsl][PATCH] optimize drw_text() for large strings
On Sun, Mar 20, 2022 at 02:56:33PM +0100, Stein Gunnar Bakkeby wrote: > 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 Hi Bakkeby, I haven't looked too deeply into the patch, but I see that you've moved the ew calculation inside the first loop rather than looping through again later down. This was more or less what I was planning to do as well, although I was motivated by performance reasons, while you seemed to be motivated by correctness reasons. Anyhow, it's a good thing that you've sent this mail as it would prevent some duplicated work on my end. As for your patch, I think it (just using a single loop instead of two) is a better approach than the one I sent, although the `ellipsis_width` shenanigan a bit questionable; I wonder if there's a cleaner way to do it. Anyhow, your patch also solves the performance problem with large strings on my end, and additionally I'm also seeing some startup time improvement when dumping large non-ascii strings into dmenu. - NRK
