Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to use Xft for text drawing #3

Merged
merged 4 commits into from Aug 16, 2016

Conversation

Projects
None yet
2 participants
@horgh
Copy link
Contributor

commented Jul 29, 2016

Hi,

I found that there were issues displaying certain characters. The program appeared to be handling UTF8 characters improperly. To resolve this I used Xft functions to draw the text for the buttons. As a bonus this also lets the bar use more fonts than before.

I tried for a while to resolve the problem through updates using Xlib functions, but eventually gave up. I did make some progress, but I could not get all characters working. For example, a browser title including ’ or ™ would corrupt the bar's title such that everything after the first occurrence would be hidden. My partial solution involved adding setlocale(LC_ALL, ""). I tried switching to Xutf8DrawString() as well. Even with these updates, certain characters would not display correctly (using terminus by the way). ™ for example kept stubbornly displaying as "b.

Maybe there is a way to get it working with Xlib but I could not figure it out.

I went looking at dmenu's source as I saw you used it for a reference for some functionality. I found that they had in 2015 switched to using Xft for drawing text (and probably more). I decided to try doing the same. Initially I had some similar issues happening (some UTF8 encoded runes displaying as n in terminus). The key was the use of XftCharExists() and loading additional fonts if that returned false. This is basically the same thing dmenu is doing. I wondered if there is a similar thing we could do using Xlib, but have been unable to find similar functions.

I extracted some functions from dmenu, and used other parts from it as a reference but rewrote them. I put some straight copied code into drw.{c,h}.

Anyway this commit introduces additional dependencies which is not ideal but it is working nicely for me.

I'd be happy to change anything you like here.

horgh added some commits Jul 29, 2016

Use Xft instead of Xlib to draw text.
This allows us to use additional fonts. It also resolves issues
displaying UTF8.

This includes code from dmenu. Specifically code related to its
drw_text() function.
@horgh

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2016

By the way, I noticed ratpoison itself is using Xft for fonts (by default), so that's a point in favour of using it here too I think.

I did see ratpoison does not do the per character check and additional font loading I implemented here though. In cases like for terminus where a character is not available it draws it as "n". Not optimal, but this pull request would be a lot smaller if we dropped that logic.

Don't require scalable font for secondary fonts
I found that requiring FC_SCALABLE would mean preferring other font
faces/families when loading secondary fonts, even if the requested font
pattern does have fonts that would satisfy the character. This is the
case in my testing with Terminus at least. We load iso-8859-1 version of
terminus for some reason, but if we require a Unicode code point that
isn't in that, we can load the Unicode version of Terminus with this
change. Before this change, we would fall back to another font.

I also add setlocale(LC_CTYPE, "") though this doesn't change behaviour.
I think it is just a good practice (it does change behaviour if using
Xlib fonts).
@dimatura

This comment has been minimized.

Copy link
Owner

commented Aug 1, 2016

Cool! Thanks for the PR. Let me try it out!

@dimatura

This comment has been minimized.

Copy link
Owner

commented Aug 16, 2016

Looks good to me! (Both in code and aesthetics). Xft is not really a terrible dependency too have IMO. Thanks!

@dimatura dimatura merged commit 48da5f1 into dimatura:master Aug 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.