search

joukos / PaperTTY

PaperTTY - Python module to render a TTY or VNC on e-ink
942 stars 101 forks source link

Font height fix #105

Closed mcarr823 closed 11 months ago

mcarr823 commented 12 months ago

This PR contains two commits. The first is simply to reduce repeat code, where recalculate_font was almost identical to another code block. The other is to work around a bug in PIL's font height calculation.

PaperTTY currently uses PIL 7.1.2. PIL 8.0.0 introduced a method for retrieving the bbox of a font, so that can be used to more accurately get the size of a font PIL 10.0.0 removed getsize altogether in favor of using bbox due to problems with the old method

As for how this relates to PaperTTY, the version and manner in which PIL is being used has caused an issue with some fonts in terminal mode. More specifically, the text draw command in PIL 7.1.2 isn't taking the font descent into account properly, in some cases causing text overlap.

For example, I've been testing this with UbuntuMono-R font at size 24 on the 10.3" e-paper waveshare screen. I get text overlap when typing "g" on one line and "b" on the next. I've tested other sizes and fonts and observed the same overlap.

I suspect that someone already noticed this problem and that is how the "auto" spacing ended up with the value of metrics_descent - 2 and the font height was calculated as self.font_height = metrics_ascent + self.spacing Those two together roughly match the actual font height and somewhat works around the PIL bug. You can see a visual representation of how ascent, descent, etc. work in PIL here: https://stackoverflow.com/questions/43060479/how-to-get-the-font-pixel-height-using-pils-imagefont-class

The currently approach mostly works. However, the default spacing for terminal mode is 0, not auto. And even in auto mode, -2 is only accurate for certain font sizes. This means that trial and error is currently necessary to get the right sizing for a particular font and font size combination.

This PR attempts to fix the problem by doing two things.

  1. metrics_descent is added to the font height calculation, so the font height is no longer just the ascent
  2. PIL's text draw method is put in a loop and drawn onto the image object line by line. This puts responsibility for positioning onto PaperTTY instead of PIL, thus preventing its font height calculation from causing overlap

I haven't encountered any issues with this approach during my own testing, nor did I find any measurable performance impact from the change to the draw behavior. It still only performs one draw to the screen, after all. It's just the manner in which text is placed on the image which has changed.

One side-effect that is worth mentioning, however, is that text won't seem as compact as before, since the text is no longer overlapping. You can still make it more compact by setting the spacing as a negative value to reintroduce overlap. Also, if this PR is merged, it might be worth revising the "auto" spacing, since the old value won't be as relevant anymore.

joukos commented 11 months ago

lgtm, but I need to build a new package at some point so I'll try it further then. Let's merge now so it's not forgotten :)