[Fontconfig] [PATCH] don't artificially embolden fixed-width fonts

[Adam Williamson]

On Sat, 2015-02-14 at 09:12 -0800, Adam Williamson wrote:
> On Sat, 2015-02-14 at 14:02 +0100, Raimund Steger wrote:
> > On 02/12/15 01:32, Raimund Steger wrote:
> > > > > > [...]
> > > 
> > > I'll have a look at them over the weekend. I think it should be 
> > > possible to isolate the interesting part.
> > 
> > I've taken a look at freetype-entire-infinality-patchset-20130514- 
> > 01.patch now and I think the relevant part is only the following 
> > (the patch contains other changes to emboldening, but these 
> > address unrelated things like pixel snapping):
> > 
> > diff --git a/src/base/ftsynth.c b/src/base/ftsynth.c
> > index 241d37f..3d5a593 100644
> > --- a/src/base/ftsynth.c
> > +++ b/src/base/ftsynth.c
> > @@ -141,6 +173,9 @@
> > 
> >       slot->metrics.width       += xstr;
> >       slot->metrics.height      += ystr;
> > +#ifdef FT_CONFIG_OPTION_INFINALITY_PATCHSET
> > +    if ( !use_various_tweaks )
> > +#endif
> >       slot->metrics.horiAdvance += xstr;
> >       slot->metrics.vertAdvance += ystr;
> > 
> > If I replace the "if ( !use_various_tweaks )" with "if ( 
> > !FT_IS_FIXED_WIDTH( face ) )", I seem to come up with a patch that 
> > keeps
> > the advance width for monospace fonts and leaves the rest 
> > unchanged. (Suggestion attached; note it's only created with quilt 
> > from a 2.5.5 tarball.) I've put up screenshots of Lucida Console at
> > http://steg0.eu/saurus/2015/02.emboldenmono/.
> > 
> > Could this already be it...??
> 
> Ah, that's interesting!
> 
> So my next move in cases like this tends to be to think "OK, so why 
> isn't it that way now?" But it certainly looks good. I quite enjoy 
> playing git Sherlock Holmes, so I'll have a poke through and see if I
> can find any skeletons in the closet that suggest that change might 
> cause any problems, but it sure looks promising! Thanks a lot for 
> figuring that out.

Well I can give a couple of thoughts:

1) your patch definitely works for me - thanks! It doesn't look bad at 
all either (with Droid Sans Mono).

2) I haven't been able to find any previous *upstream* attempt to 
change this that turned out badly, or discussion of changing it that 
was rejected...so seems like a good idea to send it upstream to 
freetype.

3) I *did* find some interesting history in infinality's patch set, 
though. There's a repo lying around on gitorious called 'lcd-
filtering':

https://gitorious.org/lcd-filtering/lcd-filtering

which happens to have re-synced with infinality quite often, so it 
acts as a sort of proxy for infinality's history. There's a rather 
interesting point in that repo. If you check it out at the state of 
commit 3735605ed6cb27a3cb70ff72864082c48d4b2090 - or just look at it 
online, 
https://gitorious.org/lcd-filtering/lcd-filtering/source/3735605ed6cb27a3cb70ff72864082c48d4b2090:media-libs/freetype/files - it has two freetype ebuilds, one for 2.4.2 and one for 2.4.3. Both 
have a patch named 'enhance-emboldening.patch'. If you look at the 
2.4.2 one, it's functionally identical to yours:

--- freetype-2.4.2.orig/src/base/ftsynth.c  2010-01-05 12:54:49.000000000 -0600
+++ freetype-2.4.2/src/base/ftsynth.c 2010-10-02 21:23:40.161154162 -0500
@@ -146,7 +146,8 @@
     slot->metrics.width        += xstr;
     slot->metrics.height       += ystr;
     slot->metrics.horiBearingY += ystr;
-    slot->metrics.horiAdvance  += xstr;
+    /* need to disable only when font is monospace */
+    if ( !( slot->face->face_flags & FT_FACE_FLAG_FIXED_WIDTH ) ) slot->metrics.horiAdvance  += xstr;
     slot->metrics.vertBearingX -= xstr / 2;
     slot->metrics.vertBearingY += ystr;
     slot->metrics.vertAdvance  += ystr;

but then at some point after, infinality decided he just preferred no 
horizontal advance on anything at all, and changed it to the current 
version. the 2.4.3 version of enhance-emboldening.patch has this 
instead:

@@ -146,7 +146,8 @@
     slot->metrics.width        += xstr;
     slot->metrics.height       += ystr;
     slot->metrics.horiBearingY += ystr;
-    slot->metrics.horiAdvance  += xstr;
+    /* Don't add any horiAdvance - Personal preference */
+    /*slot->metrics.horiAdvance  += xstr;*/
     slot->metrics.vertBearingX -= xstr / 2;
     slot->metrics.vertBearingY += ystr;
     slot->metrics.vertAdvance  += ystr;

So...it seems infinality figured this out 4.5 years ago but then 
decided to make the change much more 'personal preference'-y, and it 
got buried in history :/

Not sure if that changes much, just an interesting historical note. It 
certainly seems like a change worth suggesting upstream.
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net