[PATCH] cmd/fontsrv: fix geometry (in Linux X11)

[idrarig...@gmail.com]

There have been a few GitHub issues related to clipped fonts: #18
another was to use the factor in originy besides height:

f->height = (int)((face->ascender - face->descender) * 1.35);
f->originy = face->descender * 1.35;

This helped with a few fonts, but not all of them.

The commit message said that 1.35 was determined experimentally,
which made me curious. I decided to take a look at this code. If I'm
right, 1.35 is close to what it should've been, 1.33, that is, 96/72,
which matches the multiplicative factor used when calculating the
pixel size for the subfonts:

pixel_size = (dpi*size)/72.0;

where dpi is a constant, 96.

But instead of doing that I preferred to remove the scaling entirely.
Some testing to show the height declared in the font file matches the
height of the subfont images:

% 9p read font/FloraStd-Medium/16a/font | sed 1q
20 15
% 9p read font/FloraStd-Medium/16a/x0060.bit | topng | file -
/dev/stdin: PNG image data, 232 x 20, 8-bit/color RGB, non-interlaced
% 9p read font/FloraStd-Medium/24a/font | sed 1q
29 23
% 9p read font/FloraStd-Medium/24a/x0060.bit | topng | file -
/dev/stdin: PNG image data, 344 x 29, 8-bit/color RGB, non-interlaced
---

Hi list, this patch works for me, hopefully may help others. It may
not apply cleanly to the vanilla p9p branch, I think I also applied a
patch from github.com/jxy to my branch. I had to re-send via a gmail
account, couldn't send using my nico...@sdf.org address. Hopefully the
list won't get this twice.

src/cmd/fontsrv/main.c | 2 +-
src/cmd/fontsrv/x11.c | 11 +++++------
2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/cmd/fontsrv/main.c b/src/cmd/fontsrv/main.c
index f55407ad..21bac125 100644
--- a/src/cmd/fontsrv/main.c
+++ b/src/cmd/fontsrv/main.c
@@ -314,7 +314,7 @@ xread(Req *r)
ascent = 0;
if(f->unit > 0) {
height = f->height * (int)QSIZE(path)/f->unit + 0.99999999;
- ascent = height - (int)(-f->originy * (int)QSIZE(path)/f->unit + 0.99999999);
+ ascent = (f->height + f->originy) * (int)QSIZE(path)/f->unit + 0.99999999;
}
if(f->loadheight != nil)
f->loadheight(f, QSIZE(path), &height, &ascent);
diff --git a/src/cmd/fontsrv/x11.c b/src/cmd/fontsrv/x11.c
index c1d10197..c9cc25d8 100644
--- a/src/cmd/fontsrv/x11.c
+++ b/src/cmd/fontsrv/x11.c
@@ -11,7 +11,6 @@

static FcConfig *fc;
static FT_Library lib;
-static int dpi = 96;

void
loadfonts(void)
@@ -78,8 +77,8 @@ load(XFont *f)
return;
}
f->unit = face->units_per_EM;
- f->height = (int)((face->ascender - face->descender) * 1.35);
- f->originy = face->descender * 1.35; // bbox.yMin (or descender) is negative, because the baseline is y-coord 0
+ f->height = face->height;
+ f->originy = face->descender;

for(charcode=FT_Get_First_Char(face, &glyph_index); glyph_index != 0;
charcode=FT_Get_Next_Char(face, charcode, &glyph_index)) {
@@ -125,16 +124,16 @@ mksubfont(XFont *xf, char *name, int lo, int hi, int size, int antialias)
return nil;
}

- e = FT_Set_Char_Size(face, 0, size<<6, dpi, dpi);
+ e = FT_Set_Char_Size(face, 0, size<<6, 0, 0);
if(e){
fprint(2, "FT_Set_Char_Size failed\n");
FT_Done_Face(face);
return nil;
}

- pixel_size = (dpi*size)/72.0;
+ pixel_size = size;
w = x = (int)((face->max_advance_width) * pixel_size/xf->unit + 0.99999999);
- y = (int)((face->ascender - face->descender) * pixel_size/xf->unit + 0.99999999);
+ y = (int)(face->height * pixel_size/xf->unit + 0.99999999);
y0 = (int)(-face->descender * pixel_size/xf->unit + 0.99999999);

m = allocmemimage(Rect(0, 0, x*(hi+1-lo)+1, y+1), antialias ? GREY8 : GREY1);
--
2.20.1

[Nicola Girardi]

There have been a few GitHub issues #18 #372 #375 related to clipped
fonts. A first fix was committed to change the height scaling factor
from 1.2 to 1.35, another was later added to use the same scaling
factor for originy too:

f->height = (int)((face->ascender - face->descender) * 1.35);
f->originy = face->descender * 1.35;

This helped with a few fonts, but I still got a few clipped fonts.

A commit message said that 1.35 was determined experimentally, If I'm
right, 1.35 is close to what it should be, 1.33, that is, 96/72,

which matches the multiplicative factor used when calculating the
pixel size for the subfonts:

pixel_size = (dpi*size)/72.0;

where dpi is a constant, 96. But instead of updating the scaling
factor I preferred removing it entirely.

Testing done: As a sanity check, I wrote a script to do check that
the declared font height (in the font file)matches the height of the
subfont images, for all fonts and font sizes in my system. I also did
a visual check of a random selection of fonts and saw no vertically
clipped fonts.

This patch does not address the fact that italic fonts are sometimes
clipped on the right, sometimes on the left.

This patch applies on top of
https://github.com/jxy/plan9port/commit/292e9a1e88e9a45e594eb02f39009be4e6667fb3
which I'm not sure was merged in the official p9p.
---
src/cmd/fontsrv/x11.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)