Re: [hackers] [dmenu][PATCHes] Fix truncation issues and improve performance

From: NRK <nrk_AT_disroot.org>
Date: Wed, 23 Mar 2022 16:43:18 +0600

On Wed, Mar 23, 2022 at 09:13:52AM +0100, Hiltjo Posthuma wrote:
> Please continue working on this patch, it is appreciated.
> When it mostly works it can be put into libsl and dmenu and dwm.
>
> If possible please make the first iteration compatible with the current API.
> This would make reviewing easier.
>
> Afterwards it's no issue (although preferably not) if some API changes are
> needed.
>
> Reducing the changes would make this easier to review, as this code and also
> Xft etc are finicky (as you probably can already tell :)).

Thanks, will keep that in mind.

There was a small rendering bug in the 1st patch, instead overwriting
the rest of the area with the ellipsis, we were only overwriting
ellipsis's width.

I've fixed it locally to correctly keeps track of the width we need to
overwrite as well.

In the 2nd patch it's calling getwidth_clamp() inside MIN(..) macro,
which meant it was probably needlessly calculating the width twice. I
didn't even notice it because despite calculating width twice, it was
still faster than the vanilla version.

So far haven't found any issues with the 3rd patch, but in the commit
msg I only mentioned performance improvement, however I unknowingly
improved the correctness as well.

drw_font_getexts() was introduced inside readstdin() in commit 44c7de3,
which states:

    - fix bug where inputw was not correctly calculated from the widest item, but
      just from the one with the longest strlen() which is not the same. It's better
      now, but does not account for fallback fonts, since it would be too slow to
      calculate all the correct item widths on startup.

In the 3rd patch, we take into account the fallback fonts as well. So
not only was it faster, but it was also more correct.

Finally, with these 3 patches applied, I can dump some decently sized
(1500 bytes) emoji file into dmenu and not have it *FREEZE* when trying
to scroll up and down. However it was still noticeably lagging.

The offender? Once again drw_text(). This time the problem case was when
we don't have any font in our font list which could render the char. In
which case we call XftFontMatch() which is INSANELY slow.

The main problem is that we're calling it *even* on codepoints where we
*know* there is no match. The solution to that was just to keep a small
cache which keeps track of codepoints where there's no match and avoid
calling XftFontMatch in those cases.

With the final piece of the puzzle solved, I can now deal with unicode
files (with missing fonts) without any lag whatsoever. It feels just as
fast as dealing with ascii text.

There was a couple other _unrelated_ issues I noticed as well, will send
patches for them later on.

As for the current patches, I'll play around a bit more with them, look
out for any lurking bugs and clean them up before sending again.

- NRK
Received on Wed Mar 23 2022 - 11:43:18 CET