Hello all, The current calculation of interline spacing is fundamentally flawed, at least on "unx". The implementation of ServerFont::FetchFontMetric() in gcach_ftyp.cxx does the following: * gets ascent/descent from FT_Size_Metrics which is fine * sets internal leading to the value of line gap while this should be the external leading * if the font has an OS/2 table - recalculates the metrics based on WinAscent/WinDescent which is absolutely wrong as these metrics should never be used to control line spacing - external leading get set based on a combination of metrics from HHEA table and WinAscent/WinDescent - for CJK it "adds half of the external leading to the ascent, the other half is added to the descent" because "the line height for Asian fonts is too small", my reaction reading this was "WTF!" The main problem here is that OS/2 Win metrics should never be used for controlling line spacing[1] and Typo metrics should used instead[2]. The Win metrics in the font are set to be higher than the highest/deepest glyph in the font, else Windows will clip portions of the glyphs above those values, for most fonts Win and Typo values can be the same, but for some special fonts that have very big glyphs like Neo Euler[3], XITS Math[4] or Cambria Math the Win values are much bigger resulting in huge line spacing, see attached screen shots. #38683[5] is another example of problems caused by this. The attached patch is an attempt to bring some sanity to the situation: * external leading = height - ascent + descent (line gap) * internal leading = ascent + descent - EM_size * if the font has an OS/2 table typo metrics are used * no need for HHEA stuff as freetype derives its size metrics from it anyway. * removed the bogus CJK stuff * removed check for freetype < 2.0 which we don't support anyway This only done for 'unx' similar work is needed for 'win' and 'aqua', but I'm not familiar with these platforms and can't test on it. [1] http://www.microsoft.com/typography/otspec/os2.htm#wa [2] http://www.microsoft.com/typography/otspec/os2.htm#sta [3] https://github.com/khaledhosny/euler-otf [4] http://en.wikipedia.org/wiki/XITS_font_project [5] https://bugs.freedesktop.org/show_bug.cgi?id=38683 Regards, Khaled -- Khaled Hosny Egyptian Arab
Attachment:
Screenshot-before.png
Description: PNG image
Attachment:
Screenshot-after.png
Description: PNG image
diff --git a/vcl/unx/generic/glyphs/gcach_ftyp.cxx b/vcl/unx/generic/glyphs/gcach_ftyp.cxx index e2cf92d..5610b01 100644 --- a/vcl/unx/generic/glyphs/gcach_ftyp.cxx +++ b/vcl/unx/generic/glyphs/gcach_ftyp.cxx @@ -976,16 +976,12 @@ void ServerFont::FetchFontMetric( ImplFontMetricData& rTo, long& rFactor ) const const FT_Size_Metrics& rMetrics = maFaceFT->size->metrics; rTo.mnAscent = (+rMetrics.ascender + 32) >> 6; -#if (FTVERSION < 2000) - rTo.mnDescent = (+rMetrics.descender + 32) >> 6; -#else rTo.mnDescent = (-rMetrics.descender + 32) >> 6; -#endif - rTo.mnIntLeading = ((rMetrics.height + 32) >> 6) - (rTo.mnAscent + rTo.mnDescent); + rTo.mnExtLeading = ((rMetrics.height + 32) >> 6) - (rTo.mnAscent + rTo.mnDescent); + rTo.mnIntLeading = (rTo.mnAscent + rTo.mnDescent) - ((maFaceFT->units_per_EM + 32) >> 6); rTo.mnSlant = 0; const TT_OS2* pOS2 = (const TT_OS2*)FT_Get_Sfnt_Table( maFaceFT, ft_sfnt_os2 ); - const TT_HoriHeader* pHHEA = (const TT_HoriHeader*)FT_Get_Sfnt_Table( maFaceFT, ft_sfnt_hhea ); if( pOS2 && (pOS2->version != 0xFFFF) ) { // map the panose info from the OS2 table to their VCL counterparts @@ -1017,55 +1013,13 @@ void ServerFont::FetchFontMetric( ImplFontMetricData& rTo, long& rFactor ) const default: rTo.mePitch = PITCH_DONTKNOW; break; } - // #108862# sanity check, some fonts treat descent as signed !!! - int nDescent = pOS2->usWinDescent; - if( nDescent > 5*maFaceFT->units_per_EM ) - nDescent = (short)pOS2->usWinDescent; // interpret it as signed! - const double fScale = (double)GetFontSelData().mnHeight / maFaceFT->units_per_EM; - if( pOS2->usWinAscent || pOS2->usWinDescent ) // #i30551# + if( pOS2->sTypoAscender || pOS2->sTypoDescender ) { - rTo.mnAscent = (long)( +pOS2->usWinAscent * fScale + 0.5 ); - rTo.mnDescent = (long)( +nDescent * fScale + 0.5 ); - rTo.mnIntLeading = (long)( (+pOS2->usWinAscent + pOS2->usWinDescent - maFaceFT->units_per_EM) * fScale + 0.5 ); - } - rTo.mnExtLeading = 0; - if( (pHHEA != NULL) && (pOS2->usWinAscent || pOS2->usWinDescent) ) - { - int nExtLeading = pHHEA->Line_Gap; - nExtLeading -= (pOS2->usWinAscent + pOS2->usWinDescent); - nExtLeading += (pHHEA->Ascender - pHHEA->Descender); - if( nExtLeading > 0 ) - rTo.mnExtLeading = (long)(nExtLeading * fScale + 0.5); - } - - // Check for CJK capabilities of the current font - // #107888# workaround for Asian... - // TODO: remove when ExtLeading fully implemented - sal_Bool bCJKCapable = ((pOS2->ulUnicodeRange2 & 0x2DF00000) != 0); - - if ( bCJKCapable && (pOS2->usWinAscent || pOS2->usWinDescent) ) - { - rTo.mnIntLeading += rTo.mnExtLeading; - - // #109280# The line height for Asian fonts is too small. - // Therefore we add half of the external leading to the - // ascent, the other half is added to the descent. - const long nHalfTmpExtLeading = rTo.mnExtLeading / 2; - const long nOtherHalfTmpExtLeading = rTo.mnExtLeading - - nHalfTmpExtLeading; - - // #110641# external leading for Asian fonts. - // The factor 0.3 has been verified during experiments. - const long nCJKExtLeading = (long)(0.30 * (rTo.mnAscent + rTo.mnDescent)); - - if ( nCJKExtLeading > rTo.mnExtLeading ) - rTo.mnExtLeading = nCJKExtLeading - rTo.mnExtLeading; - else - rTo.mnExtLeading = 0; - - rTo.mnAscent += nHalfTmpExtLeading; - rTo.mnDescent += nOtherHalfTmpExtLeading; + rTo.mnAscent = (long)( pOS2->sTypoAscender * fScale + 0.5 ); + rTo.mnDescent = (long)( -pOS2->sTypoDescender * fScale + 0.5 ); + rTo.mnExtLeading = (long)( pOS2->sTypoLineGap * fScale + 0.5 ); + rTo.mnIntLeading = (long)( (pOS2->sTypoAscender - pOS2->sTypoDescender - maFaceFT->units_per_EM) * fScale + 0.5 ); } }