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

i3bar: Fix font rendering for Caveat font #4357

Closed
wants to merge 2 commits into from

Conversation

mychris
Copy link

@mychris mychris commented Mar 1, 2021

Hi,

I noticed that font rendering for i3bar has some problems with certain fonts.
Using the google font Caveat for instance, the text in the bar is always cut off at the right side. For the workspace buttons as well as for the text in the status bar.

I don't know if this is a regression, or not, since I just tried out the font today and noticed it.

I think this is because the ink extent is not accounted for?

Computes the logical and ink extents of layout.
Logical extents are usually what you want for positioning things.
Note that both extents may have non-zero x and y.
You may want to use those to offset where you render the layout.
Not doing that is a very typical bug that shows up as right-to-left layouts not being correctly positioned in a layout with a set width.

I tried to fix the issue, by adding the ink extent to the text width, when it is calculated. I first tried to adjust the rendering position of the text, but this resulted in text no longer being centered for certain other fonts. I tried the patch with different fonts I used before with i3bar and it seems to work. I am not so experienced with pango, so I am happy for any more insights.

I executed the test suite and it passes:

All tests successful.
Files=252, Tests=3712,  4 wallclock secs ( 0.75 usr +  0.15 sys =  0.90 CPU)
Result: PASS

Do you know of some fonts which gave you trouble in the past? I can do some more experiments to see if my solution is robust enough.

Any comments are welcome.

@Airblader
Copy link
Member

Airblader commented Mar 2, 2021

Thanks for your contribution! Font rendering is frustratingly difficult to get right even with libraries like pango. :-( Unfortunately, the test suite will be of not much use here, but I'm happy to hear you tried it with various fonts already. I don't really know too much about the depths of font rendering, but I'm willing to give this a shot on the next branch and we'll see if it causes issues somewhere.

Let's see if @orestisfl has some thoughts on this maybe.

@Airblader
Copy link
Member

Side note, I haven't dug much into it, but awesome also makes calls to get_pixel_extents.

@orestisfl
Copy link
Member

No thoughts here either

@mychris
Copy link
Author

mychris commented Mar 2, 2021

Thanks for your feedback. Yes, font rendering is indeed more than difficult.

I would be fine with an approach of trying it out and bailing out if there are any complaints, but I found some issues with my suggested solution.

I tried some more fonts, and Akaya Telivigala is worse with my patch than without. Using my patched version and this font, some status text is now elided (is this the right word? I see the ... ellipse instead of the word). As it turns out, the x offset for the ink extent can also be negative (didn't expect that for some reason).

Adding the x offset of the ink extent conditionally feels weird, so I will do some more tinkering and maybe look into awesome, as suggested.

@Airblader
Copy link
Member

@mychris Thanks for giving this some good testing. Let us know when you're done :-)

@mychris
Copy link
Author

mychris commented Mar 4, 2021

Ok, this is the best result I got. It's not conditionally adding the ink offset, but taking the max value of the (ink width + ink offset) or the layout offset.

I used pango-view to look at the extents of the fonts (it shows both, the ink and the logical extent) and it is a bit weird, since there are all kinds of combinations:

  • Ink extent smaller then logical extent
  • Ink extent with the same size as logical extent
  • negative offset of the ink extent (ink starts to the left of the logical extent)
  • positive offset of the ink extent

I start to believe that the best solution would be, to always draw the text last, so it can be drawn above the border/spacer area, if the font has such a layout that the ink extent is outside the logical extent.

With the new patch, the worst case is that fonts are no longer centered. The old width was logical_extent.width. Now, we take ink_extent.width + ink_extent.x as the new width, if it is bigger. I tried out all fonts installed on my system, and the text is always rendered completely. I also couldn't find a font which is cut off.

@psychon
Copy link
Contributor

psychon commented Mar 5, 2021

I never understood i3's rendering code, but... Why are you lying to cairo about the size of the drawable?

i3/libi3/font.c

Lines 84 to 90 in fff8b0c

static void draw_text_pango(const char *text, size_t text_len,
xcb_drawable_t drawable, xcb_visualtype_t *visual, int x, int y,
int max_width, bool pango_markup) {
/* Create the Pango layout */
/* root_visual_type is cached in load_pango_font */
cairo_surface_t *surface = cairo_xcb_surface_create(conn, drawable,
visual, x + max_width, y + savedFont->height);

This tells cairo that the drawable has size x + max_width. I somehow doubt that the drawable really has this size. It likely is larger.

If you told cairo the real size of the drawable, it could draw things in the right side and would not clip away parts of things.

Also, this is just ugly (sorry!): width = ((ink_extent.width + ink_extent.x) > logical_extent.width) ? (ink_extent.width + ink_extent.x) : logical_extent.width;

So... the logical extents should be used for placing text and the ink extents are only needed if you want to do partial redraws / need to figure out how far the text reaches.

Should I try taking a look at this code and figuring out where text is clipped and how to get rid of that?
(Also, what exactly does "has some problems with certain fonts." mean? Is this only about "Using the google font Caveat for instance, the text in the bar is always cut off at the right side." or are there also other issues?)

@mychris
Copy link
Author

mychris commented Mar 5, 2021

This tells cairo that the drawable has size x + max_width. I somehow doubt that the drawable really has this size. It likely is larger.

If you told cairo the real size of the drawable, it could draw things in the right side and would not clip away parts of things.

That's what I try to do. I am not familiar with the i3 code at all. The max_width is the size calculated by predict_text_width_pango AFAIK. I tried to change the calculation in that function and return the "real size", but it turns out to be quite complicated for me.

My problem is that I found fonts, for which the ink extent can be partially outside of the logical extent. For instance, you can have a text, for which the logical extent has a width of 50, and the ink extent has an offset of 3 and a width of 49, hence it is 2 pixel outside of the logical extent (that was the case for me with Caveat).
For another font, Akaya Telivigala and some text, the logical extent was 50, the ink extent was also 50, but the x offset of the ink extent was -1.

So I try to figure out how to calculate the "real size". If you could elaborate (or show code) on how to do it, I would be very much interested in that, since I don't have much experience in this.

So... the logical extents should be used for placing text and the ink extents are only needed if you want to do partial redraws / need to figure out how far the text reaches.

That's also what I read, but since the ink extent can be outside of the logical extent, I am not so sure about this. Maybe I also don't get what you mean by "placing text", because the text is, for sure, not always completely inside the logical extent. That's also what pango-view shows you.

Should I try taking a look at this code and figuring out where text is clipped and how to get rid of that?

That would be very much appreciated.

Also, what exactly does "has some problems with certain fonts." mean? Is this only about "Using the google font Caveat for instance, the text in the bar is always cut off at the right side." or are there also other issues?

Yes, it is only about the Caveat font, which is clipped at the right edge. I didn't find any other issue in next. The other font (Akaya Telivigala) I found while I was testing my stuff and figured that this one also has some interesting properties (see above).

@Airblader
Copy link
Member

@psychon Thanks for taking a look here, I was considering tagging you, but didn't want to bother you either. I'm sure our font rendering code could do with improvements. If you have time to look at that it would definitely be much appreciated.

@Airblader Airblader closed this in b23c887 Mar 5, 2021
thraizz pushed a commit to thraizz/i3-tiling-drag that referenced this pull request Sep 22, 2021
i3 actually manages to have two different cairo surfaces referring to
the same drawable. One comes from the code in draw_util. The second is
temporarily created while rendering text via draw_text(). No idea how
well cairo handles this case.

This commit instead changes the code to pass the already existing cairo
surface from the caller through.

This might or might not fix i3/i3#4357. My
thinking here is that cairo now knows the actual size of the drawable
and thus does not clip the drawing to a smaller size.

Signed-off-by: Uli Schlachter <psychon@znc.in>
LinoBigatti pushed a commit to LinoBigatti/i3-rounded that referenced this pull request Nov 3, 2021
i3 actually manages to have two different cairo surfaces referring to
the same drawable. One comes from the code in draw_util. The second is
temporarily created while rendering text via draw_text(). No idea how
well cairo handles this case.

This commit instead changes the code to pass the already existing cairo
surface from the caller through.

This might or might not fix i3/i3#4357. My
thinking here is that cairo now knows the actual size of the drawable
and thus does not clip the drawing to a smaller size.

Signed-off-by: Uli Schlachter <psychon@znc.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants