configure.ac | 2 vcl/inc/impfontmetricdata.hxx | 3 vcl/inc/sft.hxx | 14 --- vcl/source/font/fontmetric.cxx | 168 +++++++++++++++++++++++++---------------- vcl/source/fontsubset/sft.cxx | 26 ------ 5 files changed, 107 insertions(+), 106 deletions(-)
New commits: commit fb417ee082afdd2e80a1f48aa420bb8d5cb97686 Author: Khaled Hosny <[email protected]> AuthorDate: Fri Sep 30 12:35:10 2022 +0200 Commit: خالد حسني <[email protected]> CommitDate: Fri Sep 30 16:17:22 2022 +0200 vcl: Apply variations to font metrics Use HarfBuzz API instead of reading the raw font table directly, since HarfBuzz will apply the font variations as needed. For non-variable fonts we still also use HarfBuzz font metrics API, but in a more fine-grained way to maintain backward-compatibility. Change-Id: If6b12a11ecb63356be92ef4f0714355ae77378f2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140799 Tested-by: Jenkins Reviewed-by: خالد حسني <[email protected]> diff --git a/configure.ac b/configure.ac index 340e87d75b55..ab2c744fb26d 100644 --- a/configure.ac +++ b/configure.ac @@ -10882,7 +10882,7 @@ libo_CHECK_SYSTEM_MODULE([graphite],[GRAPHITE],[graphite2 >= 0.9.3]) HARFBUZZ_CFLAGS_internal="-I${WORKDIR}/UnpackedTarball/harfbuzz/src" HARFBUZZ_LIBS_internal="-L${WORKDIR}/UnpackedTarball/harfbuzz/src/.libs -lharfbuzz" -libo_CHECK_SYSTEM_MODULE([harfbuzz],[HARFBUZZ],[harfbuzz-icu >= 2.1.0]) +libo_CHECK_SYSTEM_MODULE([harfbuzz],[HARFBUZZ],[harfbuzz-icu >= 2.6.0]) if test "$COM" = "MSC"; then # override the above GRAPHITE_LIBS="${WORKDIR}/LinkTarget/StaticLibrary/graphite.lib" diff --git a/vcl/inc/impfontmetricdata.hxx b/vcl/inc/impfontmetricdata.hxx index 0d6ab70e97b5..d5b244309047 100644 --- a/vcl/inc/impfontmetricdata.hxx +++ b/vcl/inc/impfontmetricdata.hxx @@ -25,7 +25,6 @@ #include <tools/long.hxx> #include <tools/ref.hxx> #include "fontattributes.hxx" -#include "sft.hxx" #include <vector> @@ -103,7 +102,7 @@ public: void ImplInitBaselines(LogicalFontInstance *pFontInstance); private: - bool ShouldUseWinMetrics(const vcl::TTGlobalFontInfo& rInfo) const; + bool ShouldUseWinMetrics(int, int, int, int, int, int) const; // font instance attributes from the font request tools::Long mnHeight; // Font size diff --git a/vcl/inc/sft.hxx b/vcl/inc/sft.hxx index 3b72df3826a8..780dd867d304 100644 --- a/vcl/inc/sft.hxx +++ b/vcl/inc/sft.hxx @@ -691,20 +691,6 @@ class TrueTypeFace; */ VCL_DLLPUBLIC bool GetTTGlobalFontHeadInfo(const AbstractTrueTypeFont *ttf, int& xMin, int& yMin, int& xMax, int& yMax, sal_uInt16& macStyle); -/** - * Returns fonts metrics. - * @see TTGlobalFontInfo - * - * @param hhea hhea table data - * @param os2 OS/2 table data - * @param info pointer to a TTGlobalFontInfo structure - * @ingroup sft - * - */ - void GetTTFontMetrics(const uint8_t *pHhea, size_t nHhea, - const uint8_t *pOs2, size_t nOs2, - TTGlobalFontInfo *info); - /*- private definitions */ /* indexes into TrueTypeFont::tables[] and TrueTypeFont::tlens[] */ diff --git a/vcl/source/font/fontmetric.cxx b/vcl/source/font/fontmetric.cxx index f03edd201982..95263c694b3b 100644 --- a/vcl/source/font/fontmetric.cxx +++ b/vcl/source/font/fontmetric.cxx @@ -21,10 +21,11 @@ #include <i18nlangtag/mslangid.hxx> #include <officecfg/Office/Common.hxx> +#include <sal/log.hxx> +#include <tools/stream.hxx> #include <unotools/configmgr.hxx> #include <vcl/metric.hxx> #include <vcl/outdev.hxx> -#include <sal/log.hxx> #include <font/FontSelectPattern.hxx> #include <font/PhysicalFontFace.hxx> @@ -332,16 +333,18 @@ void ImplFontMetricData::ImplInitFlags( const OutputDevice* pDev ) SetFullstopCenteredFlag( bCentered ); } -bool ImplFontMetricData::ShouldUseWinMetrics(const vcl::TTGlobalFontInfo& rInfo) const +bool ImplFontMetricData::ShouldUseWinMetrics(int nAscent, int nDescent, int nTypoAscent, + int nTypoDescent, int nWinAscent, + int nWinDescent) const { if (utl::ConfigManager::IsFuzzing()) return false; OUString aFontIdentifier( GetFamilyName() + "," - + OUString::number(rInfo.ascender) + "," + OUString::number(rInfo.descender) + "," - + OUString::number(rInfo.typoAscender) + "," + OUString::number(rInfo.typoDescender) + "," - + OUString::number(rInfo.winAscent) + "," + OUString::number(rInfo.winDescent)); + + OUString::number(nAscent) + "," + OUString::number(nDescent) + "," + + OUString::number(nTypoAscent) + "," + OUString::number(nTypoDescent) + "," + + OUString::number(nWinAscent) + "," + OUString::number(nWinDescent)); css::uno::Sequence<OUString> rWinMetricFontList( officecfg::Office::Common::Misc::FontsUseWinMetrics::get()); @@ -353,65 +356,116 @@ bool ImplFontMetricData::ShouldUseWinMetrics(const vcl::TTGlobalFontInfo& rInfo) return false; } -/* - * Calculate line spacing: - * - * - hhea metrics should be used, since hhea is a mandatory font table and - * should always be present. - * - But if OS/2 is present, it should be used since it is mandatory in - * Windows. - * OS/2 has Typo and Win metrics, but the later was meant to control - * text clipping not line spacing and can be ridiculously large. - * Unfortunately many Windows application incorrectly use the Win metrics - * (thanks to GDI’s TEXTMETRIC) and old fonts might be designed with this - * in mind, so OpenType introduced a flag for fonts to indicate that they - * really want to use Typo metrics. So for best backward compatibility: - * - Use Win metrics if available. - * - Unless USE_TYPO_METRICS flag is set, in which case use Typo metrics. -*/ -void ImplFontMetricData::ImplCalcLineSpacing(LogicalFontInstance *pFontInstance) +// These are “private” HarfBuzz metrics tags, they are supported by not exposed +// in the public header. They are safe ti use, HarfBuzz just does not want to +// advertise them. +constexpr auto ASCENT_OS2 = static_cast<hb_ot_metrics_tag_t>(HB_TAG('O', 'a', 's', 'c')); +constexpr auto DESCENT_OS2 = static_cast<hb_ot_metrics_tag_t>(HB_TAG('O', 'd', 's', 'c')); +constexpr auto LINEGAP_OS2 = static_cast<hb_ot_metrics_tag_t>(HB_TAG('O', 'l', 'g', 'p')); +constexpr auto ASCENT_HHEA = static_cast<hb_ot_metrics_tag_t>(HB_TAG('H', 'a', 's', 'c')); +constexpr auto DESCENT_HHEA = static_cast<hb_ot_metrics_tag_t>(HB_TAG('H', 'd', 's', 'c')); +constexpr auto LINEGAP_HHEA = static_cast<hb_ot_metrics_tag_t>(HB_TAG('H', 'l', 'g', 'p')); + +void ImplFontMetricData::ImplCalcLineSpacing(LogicalFontInstance* pFontInstance) { mnAscent = mnDescent = mnExtLeading = mnIntLeading = 0; auto* pFace = pFontInstance->GetFontFace(); + auto* pHbFont = pFontInstance->GetHbFont(); - auto aHhea(pFace->GetRawFontData(HB_TAG('h', 'h', 'e', 'a'))); - auto aOS_2(pFace->GetRawFontData(HB_TAG('O', 'S', '/', '2'))); - - vcl::TTGlobalFontInfo rInfo = {}; - GetTTFontMetrics(aHhea.data(), aHhea.size(), aOS_2.data(), aOS_2.size(), &rInfo); - - double nUPEM = pFace->UnitsPerEm(); - double fScale = mnHeight / nUPEM; + double fScale = 0; + pFontInstance->GetScale(nullptr, &fScale); double fAscent = 0, fDescent = 0, fExtLeading = 0; - // Try hhea table first. - // tdf#107605: Some fonts have weird values here, so check that ascender is - // +ve and descender is -ve as they normally should. - if (rInfo.ascender >= 0 && rInfo.descender <= 0) + auto aFvar(pFace->GetRawFontData(HB_TAG('f', 'v', 'a', 'r'))); + if (!aFvar.empty()) { - fAscent = rInfo.ascender * fScale; - fDescent = -rInfo.descender * fScale; - fExtLeading = rInfo.linegap * fScale; + // This is a variable font, trust HarfBuzz to give us the right metrics + // and apply variations to them. + hb_position_t nAscent, nDescent, nLineGap; + if (hb_ot_metrics_get_position(pHbFont, HB_OT_METRICS_TAG_HORIZONTAL_ASCENDER, &nAscent) + && hb_ot_metrics_get_position(pHbFont, HB_OT_METRICS_TAG_HORIZONTAL_DESCENDER, + &nDescent) + && hb_ot_metrics_get_position(pHbFont, HB_OT_METRICS_TAG_HORIZONTAL_LINE_GAP, + &nLineGap)) + { + fAscent = nAscent * fScale; + fDescent = -nDescent * fScale; + fExtLeading = nLineGap * fScale; + } } - - // But if OS/2 is present, prefer it. - if (rInfo.winAscent || rInfo.winDescent || - rInfo.typoAscender || rInfo.typoDescender) + else { - if (ShouldUseWinMetrics(rInfo) || (fAscent == 0.0 && fDescent == 0.0)) + // This is not a variable font, we try to chose the best metrics + // ourselves for backward comparability: + // + // - hhea metrics should be used, since hhea is a mandatory font table + // and should always be present. + // - But if OS/2 is present, it should be used since it is mandatory in + // Windows. + // OS/2 has Typo and Win metrics, but the later was meant to control + // text clipping not line spacing and can be ridiculously large. + // Unfortunately many Windows application incorrectly use the Win + // metrics (thanks to GDI’s TEXTMETRIC) and old fonts might be + // designed with this in mind, so OpenType introduced a flag for + // fonts to indicate that they really want to use Typo metrics. So + // for best backward compatibility: + // - Use Win metrics if available. + // - Unless USE_TYPO_METRICS flag is set, in which case use Typo + // metrics. + + // Try hhea table first. + hb_position_t nAscent = 0, nDescent = 0, nLineGap = 0; + if (hb_ot_metrics_get_position(pHbFont, ASCENT_HHEA, &nAscent) + && hb_ot_metrics_get_position(pHbFont, DESCENT_HHEA, &nDescent) + && hb_ot_metrics_get_position(pHbFont, LINEGAP_HHEA, &nLineGap)) { - fAscent = rInfo.winAscent * fScale; - fDescent = rInfo.winDescent * fScale; - fExtLeading = 0; + // tdf#107605: Some fonts have weird values here, so check that + // ascender is +ve and descender is -ve as they normally should. + if (nAscent >= 0 && nDescent <= 0) + { + fAscent = nAscent * fScale; + fDescent = -nDescent * fScale; + fExtLeading = nLineGap * fScale; + } } - const uint16_t kUseTypoMetricsMask = 1 << 7; - if (rInfo.fsSelection & kUseTypoMetricsMask && - rInfo.typoAscender >= 0 && rInfo.typoDescender <= 0) + // But if OS/2 is present, prefer it. + hb_position_t nTypoAscent, nTypoDescent, nTypoLineGap, nWinAscent, nWinDescent; + if (hb_ot_metrics_get_position(pHbFont, ASCENT_OS2, &nTypoAscent) + && hb_ot_metrics_get_position(pHbFont, DESCENT_OS2, &nTypoDescent) + && hb_ot_metrics_get_position(pHbFont, LINEGAP_OS2, &nTypoLineGap) + && hb_ot_metrics_get_position(pHbFont, HB_OT_METRICS_TAG_HORIZONTAL_CLIPPING_ASCENT, + &nWinAscent) + && hb_ot_metrics_get_position(pHbFont, HB_OT_METRICS_TAG_HORIZONTAL_CLIPPING_DESCENT, + &nWinDescent)) { - fAscent = rInfo.typoAscender * fScale; - fDescent = -rInfo.typoDescender * fScale; - fExtLeading = rInfo.typoLineGap * fScale; + if ((fAscent == 0.0 && fDescent == 0.0) + || ShouldUseWinMetrics(nAscent, nDescent, nTypoAscent, nTypoDescent, nWinAscent, + nWinDescent)) + { + fAscent = nWinAscent * fScale; + fDescent = nWinDescent * fScale; + fExtLeading = 0; + } + + bool bUseTypoMetrics = false; + { + // TODO: Use HarfBuzz API instead of raw access + // https://github.com/harfbuzz/harfbuzz/issues/1920 + sal_uInt16 fsSelection = 0; + auto aOS2(pFace->GetRawFontData(HB_TAG('O', 'S', '/', '2'))); + SvMemoryStream aStream(const_cast<uint8_t*>(aOS2.data()), aOS2.size(), + StreamMode::READ); + if (aStream.Seek(vcl::OS2_fsSelection_offset) == vcl::OS2_fsSelection_offset) + aStream.ReadUInt16(fsSelection); + bUseTypoMetrics = fsSelection & (1 << 7); + } + if (bUseTypoMetrics && nTypoAscent >= 0 && nTypoDescent <= 0) + { + fAscent = nTypoAscent * fScale; + fDescent = -nTypoDescent * fScale; + fExtLeading = nTypoLineGap * fScale; + } } } @@ -421,18 +475,6 @@ void ImplFontMetricData::ImplCalcLineSpacing(LogicalFontInstance *pFontInstance) if (mnAscent || mnDescent) mnIntLeading = mnAscent + mnDescent - mnHeight; - - SAL_INFO("vcl.gdi.fontmetric", GetFamilyName() - << ": fsSelection: " << rInfo.fsSelection - << ", typoAscender: " << rInfo.typoAscender - << ", typoDescender: " << rInfo.typoDescender - << ", typoLineGap: " << rInfo.typoLineGap - << ", winAscent: " << rInfo.winAscent - << ", winDescent: " << rInfo.winDescent - << ", ascender: " << rInfo.ascender - << ", descender: " << rInfo.descender - << ", linegap: " << rInfo.linegap - ); } void ImplFontMetricData::ImplInitBaselines(LogicalFontInstance *pFontInstance) diff --git a/vcl/source/fontsubset/sft.cxx b/vcl/source/fontsubset/sft.cxx index 441838e01c8e..deb18ff73177 100644 --- a/vcl/source/fontsubset/sft.cxx +++ b/vcl/source/fontsubset/sft.cxx @@ -2279,32 +2279,6 @@ std::unique_ptr<sal_uInt16[]> GetTTSimpleGlyphMetrics(AbstractTrueTypeFont const return res; } -// TODO, clean up table parsing and re-use it elsewhere in this file. -void GetTTFontMetrics(const uint8_t *pHhea, size_t nHhea, - const uint8_t *pOs2, size_t nOs2, - TTGlobalFontInfo *info) -{ - /* There are 3 different versions of OS/2 table: original (68 bytes long), - * Microsoft old (78 bytes long) and Microsoft new (86 bytes long,) - * Apple's documentation recommends looking at the table length. - */ - if (nOs2 >= OS2_V0_length) - { - info->fsSelection = GetUInt16(pOs2, OS2_fsSelection_offset); - info->typoAscender = GetInt16(pOs2, OS2_typoAscender_offset); - info->typoDescender = GetInt16(pOs2, OS2_typoDescender_offset); - info->typoLineGap = GetInt16(pOs2, OS2_typoLineGap_offset); - info->winAscent = GetUInt16(pOs2, OS2_winAscent_offset); - info->winDescent = GetUInt16(pOs2, OS2_winDescent_offset); - } - - if (nHhea >= HHEA_lineGap_offset + 2) { - info->ascender = GetInt16(pHhea, HHEA_ascender_offset); - info->descender = GetInt16(pHhea, HHEA_descender_offset); - info->linegap = GetInt16(pHhea, HHEA_lineGap_offset); - } -} - bool GetTTGlobalFontHeadInfo(const AbstractTrueTypeFont *ttf, int& xMin, int& yMin, int& xMax, int& yMax, sal_uInt16& macStyle) { sal_uInt32 table_size;
