include/vcl/glyphitemcache.hxx | 5 +++--
vcl/qa/unx/generic/generic.cxx | 30 ++++++++++++++++++++++++++++++
vcl/source/gdi/impglyphitem.cxx | 7 ++++++-
3 files changed, 39 insertions(+), 3 deletions(-)
New commits:
commit ef1870810ec8c069e26538fd7626ad0656bed276
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Jul 31 13:08:42 2024 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Jul 31 16:56:26 2024 +0200
tdf#162280 vcl: consider font family type for glyph caching
Open tdf105820-1.doc from crashtesting in a dbgutil build, save as PDF,
we assert in checkGlyphsEqual().
This is a problem since commit 6dfac38bacd449c64a13363797b56aff49cf8f52
(tdf#162072 vcl, fontconfig: consider font-family-generic for
substitute, 2024-07-18), because now the font name no longer determines
the fallback name alone, so 2 paragraphs may share the cached vcl-level
text layout, even if the underlying fallback font is different. One
would naively expect that in the SalLayoutGlyphsCache::CachedGlyphsKey
ctor, the fontMetric.GetFamilyType() already has the correct family
type, but it turns out that is always the default, and only
outputDevice->GetFont().GetFamilyType() has the up to date setting:
debug:12372:12372: SalLayoutGlyphsCache ctor: output device font family
name is 'Verdana', output device font family type is roman, font metric family
name is 'Verdana', font metric family type is 'swiss'
debug:12372:12372: SalLayoutGlyphsCache ctor: output device font family
name is 'Verdana', output device font family type is swiss, font metric family
name is 'Verdana', font metric family type is 'swiss'
Fix the problem by explicitly including the output device font family
type in the cache key.
Note that this only happens in practice if the same font is used in the
document with different family types, which is probably never the
intention of the user. E.g. Verdana is meant to be sans, a serif Verdana
is some weird corner-case.
Change-Id: Id6cc60809a35a3dcdc6e83122a559ddfbe5d5d0c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171280
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/include/vcl/glyphitemcache.hxx b/include/vcl/glyphitemcache.hxx
index 79f05e606550..373f270a65bc 100644
--- a/include/vcl/glyphitemcache.hxx
+++ b/include/vcl/glyphitemcache.hxx
@@ -72,8 +72,7 @@ public:
{
}
-private:
- struct SAL_DLLPRIVATE CachedGlyphsKey
+ struct CachedGlyphsKey
{
OUString text;
sal_Int32 index;
@@ -94,6 +93,8 @@ private:
tools::Long w);
bool operator==(const CachedGlyphsKey& other) const;
};
+
+private:
struct CachedGlyphsHash
{
size_t operator()(const CachedGlyphsKey& key) const { return
key.hashValue; }
diff --git a/vcl/qa/unx/generic/generic.cxx b/vcl/qa/unx/generic/generic.cxx
index 7d71bec072e5..22176c28633c 100644
--- a/vcl/qa/unx/generic/generic.cxx
+++ b/vcl/qa/unx/generic/generic.cxx
@@ -10,6 +10,10 @@
#include <test/unoapi_test.hxx>
#include <vcl/font.hxx>
+#include <vcl/vclptr.hxx>
+#include <vcl/wintypes.hxx>
+#include <vcl/window.hxx>
+#include <vcl/glyphitemcache.hxx>
#include <font/FontSelectPattern.hxx>
#include <unx/fontmanager.hxx>
@@ -61,6 +65,32 @@ CPPUNIT_TEST_FIXTURE(Test, testFontFallbackSerif)
// i.e. we got a sans fallback for a serif pattern, which is clearly poor.
CPPUNIT_ASSERT_EQUAL(sResolvedSerif, sPlexFallback);
}
+
+CPPUNIT_TEST_FIXTURE(Test, testFontFallbackCaching)
+{
+ // Given a vcl-level text layout, created for the serif Verdana:
+ ScopedVclPtrInstance<vcl::Window> pWindow(nullptr, WB_APP | WB_STDWORK);
+ VclPtr<OutputDevice> pOutDev = pWindow->GetOutDev();
+ vcl::Font aFont;
+ aFont.SetFamilyName("Verdana");
+ aFont.SetFamily(FAMILY_ROMAN);
+ pOutDev->SetFont(aFont);
+ OUString aText = u"1-1-2017"_ustr;
+ sal_Int32 nIndex = 0;
+ sal_Int32 nLength = aText.getLength();
+ tools::Long nLogicWidth = 0;
+ SalLayoutGlyphsCache::CachedGlyphsKey aKey1(pOutDev, aText, nIndex,
nLength, nLogicWidth);
+
+ // When creating a layout for the sans Verdana:
+ aFont.SetFamily(FAMILY_SWISS);
+ pOutDev->SetFont(aFont);
+ SalLayoutGlyphsCache::CachedGlyphsKey aKey2(pOutDev, aText, nIndex,
nLength, nLogicWidth);
+
+ // Then make sure these two layouts don't match:
+ // Without the accompanying fix in place, this test would have failed, the
Noto Serif layout was
+ // reused for the Noto Sans layout if those were selected as fallbacks.
+ CPPUNIT_ASSERT(aKey1 != aKey2);
+}
}
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/vcl/source/gdi/impglyphitem.cxx b/vcl/source/gdi/impglyphitem.cxx
index 184e9f819b5a..3ac42bef226a 100644
--- a/vcl/source/gdi/impglyphitem.cxx
+++ b/vcl/source/gdi/impglyphitem.cxx
@@ -555,9 +555,14 @@ SalLayoutGlyphsCache::CachedGlyphsKey::CachedGlyphsKey(
o3tl::hash_combine(hashValue, artificialBold);
o3tl::hash_combine(hashValue, layoutMode);
o3tl::hash_combine(hashValue, digitLanguage.get());
+
+ // In case the font name is the same, but the font family differs, then
the font metric won't
+ // contain that custom font family, so explicitly include the font family
from the output device
+ // font.
+ o3tl::hash_combine(hashValue, outputDevice->GetFont().GetFamilyType());
}
-inline bool SalLayoutGlyphsCache::CachedGlyphsKey::operator==(const
CachedGlyphsKey& other) const
+bool SalLayoutGlyphsCache::CachedGlyphsKey::operator==(const CachedGlyphsKey&
other) const
{
return hashValue == other.hashValue && index == other.index && len ==
other.len
&& logicWidth == other.logicWidth && mapMode == other.mapMode &&
rtl == other.rtl
