make sure Skia DWrite matches the given HFONT exactly (tdf#137122)

The problem appears to be that we use our private copy of the Dejavu
Sans fonts, installed using AddFontResourceExW( FR_PRIVATE ),
but that's not for whatever reason available for DirectWrite,
which made CreateFontFromLOGFONT() find the system-installed Dejavu
Sans, and if there was a difference between the two, then incorrect
glyph ids were used. Use CreateFontFaceFromHdc(), which seems
to match exactly. For private fonts this later fails the check
with GetSystemFontCollection(), which would be nice to handle
somehow, but I don't know how to get a font collection containing
those fonts other than modifying SalFont to use DirectWrite
API to install the fonts.

Change-Id: I10d8fcb618f3b4decbb0198274331d7beaf843d6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111522
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111773
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

2 files changed, 35 insertions, 14 deletions

diff --git a/vcl/inc/skia/win/gdiimpl.hxx b/vcl/inc/skia/win/gdiimpl.hxx
index 12d2b2c10c63..2dd67826beeb 100644
--- a/vcl/inc/skia/win/gdiimpl.hxx
+++ b/vcl/inc/skia/win/gdiimpl.hxx
@@ -11,8 +11,9 @@
 #define INCLUDED_VCL_INC_SKIA_WIN_GDIIMPL_HXX
 
 #include <memory>
-#include <vcl/dllapi.h>
+#include <systools/win32/comtools.hxx>
 
+#include <vcl/dllapi.h>
 #include <skia/gdiimpl.hxx>
 #include <win/salgdi.h>
 #include <win/wingdiimpl.hxx>
@@ -75,10 +76,10 @@ public:
 protected:
     virtual void createWindowContext(bool forceRaster = false) override;
     virtual void performFlush() override;
-    sk_sp<SkTypeface> createDirectWriteTypeface(const LOGFONTW& logFont);
+    sk_sp<SkTypeface> createDirectWriteTypeface(HDC hdc, HFONT hfont);
     static void initFontInfo();
-    inline static IDWriteFactory* dwriteFactory;
-    inline static IDWriteGdiInterop* dwriteGdiInterop;
+    inline static sal::systools::COMReference<IDWriteFactory> dwriteFactory;
+    inline static sal::systools::COMReference<IDWriteGdiInterop> dwriteGdiInterop;
     inline static sk_sp<SkFontMgr> dwriteFontMgr;
     inline static bool dwriteDone = false;
     static SkFont::Edging fontEdging;
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index d3d9ea1b5cb0..b7e661896fa4 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -143,7 +143,7 @@ static HRESULT checkResult(HRESULT hr, const char* file, size_t line)
 #define CHECKHR(funct) (funct)
 #endif
 
-sk_sp<SkTypeface> WinSkiaSalGraphicsImpl::createDirectWriteTypeface(const LOGFONTW& logFont)
+sk_sp<SkTypeface> WinSkiaSalGraphicsImpl::createDirectWriteTypeface(HDC hdc, HFONT hfont)
 {
     if (!dwriteDone)
     {
@@ -152,25 +152,43 @@ sk_sp<SkTypeface> WinSkiaSalGraphicsImpl::createDirectWriteTypeface(const LOGFON
                                             reinterpret_cast<IUnknown**>(&dwriteFactory)))))
         {
             if (SUCCEEDED(CHECKHR(dwriteFactory->GetGdiInterop(&dwriteGdiInterop))))
-                dwriteFontMgr = SkFontMgr_New_DirectWrite(dwriteFactory);
+                dwriteFontMgr = SkFontMgr_New_DirectWrite(dwriteFactory.get());
             else
-                dwriteFactory->Release();
+                dwriteFactory.clear();
         }
         dwriteDone = true;
     }
     if (!dwriteFontMgr)
         return nullptr;
-    IDWriteFont* font = nullptr;
-    IDWriteFontFace* fontFace;
-    IDWriteFontFamily* fontFamily;
-    if (FAILED(CHECKHR(dwriteGdiInterop->CreateFontFromLOGFONT(&logFont, &font))))
+
+    // tdf#137122: We need to get the exact same font as HFONT refers to,
+    // since VCL core computes things like glyph ids based on that, and getting
+    // a different font could lead to mismatches (e.g. if there's a slightly
+    // different version of the same font installed system-wide).
+    // For that CreateFromFaceFromHdc() is necessary. The simpler
+    // CreateFontFromLOGFONT() seems to search for the best matching font,
+    // which may not be the exact font. Our private fonts are installed
+    // using AddFontResourceExW( FR_PRIVATE ) and that apparently does
+    // not make them available to DirectWrite (at least, they are not
+    // included the DWrite system font collection). For such cases, we'll
+    // need to fall back to Skia's GDI-based font rendering.
+    HFONT oldFont = SelectFont(hdc, hfont);
+    auto restoreFont = [hdc, oldFont]() { SelectFont(hdc, oldFont); };
+    sal::systools::COMReference<IDWriteFontFace> fontFace;
+    if (FAILED(CHECKHR(dwriteGdiInterop->CreateFontFaceFromHdc(hdc, &fontFace))))
+        return nullptr;
+    sal::systools::COMReference<IDWriteFontCollection> collection;
+    if (FAILED(CHECKHR(dwriteFactory->GetSystemFontCollection(&collection))))
         return nullptr;
-    if (FAILED(CHECKHR(font->CreateFontFace(&fontFace))))
+    sal::systools::COMReference<IDWriteFont> font;
+    // Do not use CHECKHR() here, as said above, this fails for our fonts.
+    if (FAILED(collection->GetFontFromFontFace(fontFace.get(), &font)))
         return nullptr;
+    sal::systools::COMReference<IDWriteFontFamily> fontFamily;
     if (FAILED(CHECKHR(font->GetFontFamily(&fontFamily))))
         return nullptr;
     return sk_sp<SkTypeface>(
-        SkCreateTypefaceDirectWrite(dwriteFontMgr, fontFace, font, fontFamily));
+        SkCreateTypefaceDirectWrite(dwriteFontMgr, fontFace.get(), font.get(), fontFamily.get()));
 }
 
 bool WinSkiaSalGraphicsImpl::DrawTextLayout(const GenericSalLayout& rLayout)
@@ -187,7 +205,7 @@ bool WinSkiaSalGraphicsImpl::DrawTextLayout(const GenericSalLayout& rLayout)
         assert(false);
         return false;
     }
-    sk_sp<SkTypeface> typeface = createDirectWriteTypeface(logFont);
+    sk_sp<SkTypeface> typeface = createDirectWriteTypeface(mWinParent.getHDC(), hLayoutFont);
     GlyphOrientation glyphOrientation = GlyphOrientation::Apply;
     if (!typeface) // fall back to GDI text rendering
     {
@@ -249,6 +267,8 @@ void WinSkiaSalGraphicsImpl::initFontInfo()
 void WinSkiaSalGraphicsImpl::ClearDevFontCache()
 {
     dwriteFontMgr.reset();
+    dwriteFactory.clear();
+    dwriteGdiInterop.clear();
     dwriteDone = false;
     initFontInfo(); // get font info again, just in case
 }