fix Skia Windows text rendering

This is quite some trial and error, but now it seems all CJK text
rendering works properly. I tested tdf#136081, tdf#137907, tdf#103785,
tdf#106295, tdf#114209 and tdf#141715.

Change-Id: I40e893f66281b0a1a0e814feec3f782ceeb0c535
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115620
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

4 files changed, 38 insertions, 30 deletions

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index 08d9f6ee64cc..14fa5df6012b 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -217,13 +217,8 @@ public:
     void drawShader(const SalTwoRect& rPosAry, const sk_sp<SkShader>& shader,
                     SkBlendMode blendMode = SkBlendMode::kSrcOver);
 
-    enum class GlyphOrientation
-    {
-        Apply,
-        Ignore
-    };
     void drawGenericLayout(const GenericSalLayout& layout, Color textColor, const SkFont& font,
-                           const SkFont& verticalFont, GlyphOrientation glyphOrientation);
+                           const SkFont& verticalFont);
 
 protected:
     // To be called before any drawing.
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index b559231cd906..fbc77b4112ae 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -2060,8 +2060,7 @@ static double toCos(Degree10 degree10th) { return SkScalarCos(toRadian(degree10t
 static double toSin(Degree10 degree10th) { return SkScalarSin(toRadian(degree10th)); }
 
 void SkiaSalGraphicsImpl::drawGenericLayout(const GenericSalLayout& layout, Color textColor,
-                                            const SkFont& font, const SkFont& verticalFont,
-                                            GlyphOrientation glyphOrientation)
+                                            const SkFont& font, const SkFont& verticalFont)
 {
     SkiaZone zone;
     std::vector<SkGlyphID> glyphIds;
@@ -2076,13 +2075,9 @@ void SkiaSalGraphicsImpl::drawGenericLayout(const GenericSalLayout& layout, Colo
     while (layout.GetNextGlyph(&pGlyph, aPos, nStart))
     {
         glyphIds.push_back(pGlyph->glyphId());
-        Degree10 angle(0); // 10th of degree
-        if (glyphOrientation == GlyphOrientation::Apply)
-        {
-            angle = layout.GetOrientation();
-            if (pGlyph->IsVertical())
-                angle += 900_deg10; // 90 degree
-        }
+        Degree10 angle = layout.GetOrientation();
+        if (pGlyph->IsVertical())
+            angle += 900_deg10;
         SkRSXform form = SkRSXform::Make(toCos(angle), toSin(angle), aPos.X(), aPos.Y());
         glyphForms.emplace_back(std::move(form));
         verticals.emplace_back(pGlyph->IsVertical());
@@ -2099,7 +2094,8 @@ void SkiaSalGraphicsImpl::drawGenericLayout(const GenericSalLayout& layout, Colo
     SAL_INFO("vcl.skia.trace", "drawtextblob(" << this << "): " << getBoundRect() << ", "
                                                << glyphIds.size() << " glyphs, " << textColor);
 
-    // Vertical glyphs need a different font, so each run draw only consecutive horizontal or vertical glyphs.
+    // Vertical glyphs need a different font, so split drawing into runs that each
+    // draw only consecutive horizontal or vertical glyphs.
     std::vector<bool>::const_iterator pos = verticals.cbegin();
     std::vector<bool>::const_iterator end = verticals.cend();
     while (pos != end)
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index e245f488433a..2e0e7fc8ee12 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -203,6 +203,7 @@ bool WinSkiaSalGraphicsImpl::DrawTextLayout(const GenericSalLayout& rLayout)
     const SkiaWinFontInstance* pWinFont
         = static_cast<const SkiaWinFontInstance*>(&rLayout.GetFont());
     const HFONT hLayoutFont = pWinFont->GetHFONT();
+    double hScale = pWinFont->getHScale();
     LOGFONTW logFont;
     if (GetObjectW(hLayoutFont, sizeof(logFont), &logFont) == 0)
     {
@@ -216,34 +217,51 @@ bool WinSkiaSalGraphicsImpl::DrawTextLayout(const GenericSalLayout& rLayout)
         bool dwrite = true;
         if (!typeface) // fall back to GDI text rendering
         {
-            // If lfWidth is kept, then with fHScale != 1 characters get too wide, presumably
+            // If lfWidth is kept, then with hScale != 1 characters get too wide, presumably
             // because the horizontal scaling gets applied twice if GDI is used for drawing (tdf#141715).
-            // Using lfWidth /= fHScale gives slightly incorrect sizes, for a reason I don't understand.
+            // Using lfWidth /= hScale gives slightly incorrect sizes, for a reason I don't understand.
             // LOGFONT docs say that 0 means GDI will find out the right value on its own somehow,
             // and it apparently works.
             logFont.lfWidth = 0;
+            // Reset LOGFONT orientation, the proper orientation is applied by drawGenericLayout(),
+            // and keeping this would make it get applied once more when doing the actual GDI drawing.
+            // Resetting it here does not seem to cause any problem.
+            logFont.lfOrientation = 0;
+            logFont.lfEscapement = 0;
             typeface.reset(SkCreateTypefaceFromLOGFONT(logFont));
             dwrite = false;
+            if (!typeface)
+                return false;
         }
         // Cache the typeface.
         const_cast<SkiaWinFontInstance*>(pWinFont)->SetSkiaTypeface(typeface, dwrite);
     }
-    // lfHeight actually depends on DPI, so it's not really font height as such,
-    // but for LOGFONT-based typefaces Skia simply sets lfHeight back to this value
-    // directly.
-    double fontHeight = logFont.lfHeight;
-    if (fontHeight < 0)
-        fontHeight = -fontHeight;
-    SkFont font(typeface, fontHeight, pWinFont->getHScale(), 0);
+
+    SkFont font(typeface);
     font.setEdging(logFont.lfQuality == NONANTIALIASED_QUALITY ? SkFont::Edging::kAlias
                                                                : fontEdging);
+
+    const FontSelectPattern& rFSD = pWinFont->GetFontSelectPattern();
+    int nHeight = rFSD.mnHeight;
+    int nWidth = rFSD.mnWidth ? rFSD.mnWidth : nHeight;
+    if (nWidth == 0 || nHeight == 0)
+        return false;
+    font.setSize(nHeight);
+    font.setScaleX(hScale);
+
+    // Unlike with Freetype-based font handling, use height even in vertical mode,
+    // additionally multiply it by horizontal scale to get the proper
+    // size and then scale the width back, otherwise the height would
+    // not be correct. I don't know why this is inconsistent.
+    SkFont verticalFont(font);
+    verticalFont.setSize(nHeight * hScale);
+    verticalFont.setScaleX(1.0 / hScale);
+
     assert(dynamic_cast<SkiaSalGraphicsImpl*>(mWinParent.GetImpl()));
     SkiaSalGraphicsImpl* impl = static_cast<SkiaSalGraphicsImpl*>(mWinParent.GetImpl());
     COLORREF color = ::GetTextColor(mWinParent.getHDC());
     Color salColor(GetRValue(color), GetGValue(color), GetBValue(color));
-    impl->drawGenericLayout(rLayout, salColor, font, font,
-                            pWinFont->GetSkiaDWrite() ? GlyphOrientation::Apply
-                                                      : GlyphOrientation::Ignore);
+    impl->drawGenericLayout(rLayout, salColor, font, verticalFont);
     return true;
 }
 
diff --git a/vcl/skia/x11/textrender.cxx b/vcl/skia/x11/textrender.cxx
index c1086b57545a..01587233247a 100644
--- a/vcl/skia/x11/textrender.cxx
+++ b/vcl/skia/x11/textrender.cxx
@@ -65,8 +65,7 @@ void SkiaTextRender::DrawTextLayout(const GenericSalLayout& rLayout, const SalGr
 
     assert(dynamic_cast<SkiaSalGraphicsImpl*>(rGraphics.GetImpl()));
     SkiaSalGraphicsImpl* impl = static_cast<SkiaSalGraphicsImpl*>(rGraphics.GetImpl());
-    impl->drawGenericLayout(rLayout, mnTextColor, font, verticalFont,
-                            SkiaSalGraphicsImpl::GlyphOrientation::Apply);
+    impl->drawGenericLayout(rLayout, mnTextColor, font, verticalFont);
 }
 
 void SkiaTextRender::ClearDevFontCache() { fontManager.reset(); }