tdf#162259: correctly handle font width on Windows

Unlike other platforms, on Windows, the font width is not relative to
font height, but to average width of font's glyphs. This is mentioned
in LogicalFontInstance::GetScale.

1. In VclProcessor2D::RenderTextSimpleOrDecoratedPortionPrimitive2D,
when calculating the correction for width / height (introduced in
commit cc3663bbaed4f65d64154e5f9abb51a5f622f710, 2024-04-20), the
already applied X scale is now calculated using unscaled font's width.

2. Commit 8557ea84c9336ba8061246f1f46ddb6e02f413a1 (Exclude getHScale
from DirectWrite font rendering, 2024-04-08) was effectively reverted,
because I was wrong assuming that the code there was unnecessary.

3. Commit 2092df2a9044f1c2ae4379f48a3201e5867575a8 (tdf#161154: pass
"scaling is done externally" information down the stack, 2024-05-18)
was also reverted.

Change-Id: I8cff39b67a6efd380f7807f5655f401bdb62cc3a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171382
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins

10 files changed, 127 insertions, 45 deletions

diff --git a/drawinglayer/source/processor2d/vclprocessor2d.cxx b/drawinglayer/source/processor2d/vclprocessor2d.cxx
index 3da588b1a096..718c725fc88f 100644
--- a/drawinglayer/source/processor2d/vclprocessor2d.cxx
+++ b/drawinglayer/source/processor2d/vclprocessor2d.cxx
@@ -397,6 +397,25 @@ void VclProcessor2D::RenderTextSimpleOrDecoratedPortionPrimitive2D(
                           / (aResultFontSize.Width() ? aResultFontSize.Width()
                                                      : aResultFontSize.Height());
 
+#ifdef _WIN32
+                    if (aResultFontSize.Width()
+                        && aResultFontSize.Width() != aResultFontSize.Height())
+                    {
+                        // See getVclFontFromFontAttribute in drawinglayer/source/primitive2d/textlayoutdevice.cxx
+                        vcl::Font aUnscaledTest(aFont);
+                        aUnscaledTest.SetFontSize({ 0, aResultFontSize.Height() });
+                        const FontMetric aUnscaledFontMetric(
+                            Application::GetDefaultDevice()->GetFontMetric(aUnscaledTest));
+                        if (aUnscaledFontMetric.GetAverageFontWidth() > 0)
+                        {
+                            double nExistingXScale = static_cast<double>(aResultFontSize.Width())
+                                                     / aUnscaledFontMetric.GetAverageFontWidth();
+                            nFontScalingFixX
+                                = aFontScaling.getX() / aFontScaling.getY() / nExistingXScale;
+                        }
+                    }
+#endif
+
                     if (!rtl_math_approxEqual(nFontScalingFixY, 1.0)
                         || !rtl_math_approxEqual(nFontScalingFixX, 1.0))
                     {
@@ -431,25 +450,21 @@ void VclProcessor2D::RenderTextSimpleOrDecoratedPortionPrimitive2D(
             mpOutputDevice->SetFont(aFont);
             mpOutputDevice->SetTextColor(Color(aRGBFontColor));
 
+            if (!aDXArray.empty())
             {
-                // For D2DWriteTextOutRenderer, we must pass a flag to not use font scaling
-                auto guard = mpOutputDevice->ScopedNoFontScaling();
-                if (!aDXArray.empty())
-                {
-                    const SalLayoutGlyphs* pGlyphs = SalLayoutGlyphsCache::self()->GetLayoutGlyphs(
-                        mpOutputDevice, rTextCandidate.getText(), rTextCandidate.getTextPosition(),
-                        rTextCandidate.getTextLength());
-                    mpOutputDevice->DrawTextArray(
-                        aStartPoint, rTextCandidate.getText(), aDXArray,
-                        rTextCandidate.getKashidaArray(), rTextCandidate.getTextPosition(),
-                        rTextCandidate.getTextLength(), SalLayoutFlags::NONE, pGlyphs);
-                }
-                else
-                {
-                    mpOutputDevice->DrawText(aStartPoint, rTextCandidate.getText(),
-                                             rTextCandidate.getTextPosition(),
-                                             rTextCandidate.getTextLength());
-                }
+                const SalLayoutGlyphs* pGlyphs = SalLayoutGlyphsCache::self()->GetLayoutGlyphs(
+                    mpOutputDevice, rTextCandidate.getText(), rTextCandidate.getTextPosition(),
+                    rTextCandidate.getTextLength());
+                mpOutputDevice->DrawTextArray(
+                    aStartPoint, rTextCandidate.getText(), aDXArray,
+                    rTextCandidate.getKashidaArray(), rTextCandidate.getTextPosition(),
+                    rTextCandidate.getTextLength(), SalLayoutFlags::NONE, pGlyphs);
+            }
+            else
+            {
+                mpOutputDevice->DrawText(aStartPoint, rTextCandidate.getText(),
+                                         rTextCandidate.getTextPosition(),
+                                         rTextCandidate.getTextLength());
             }
 
             // Restore previous layout mode
diff --git a/include/vcl/outdev.hxx b/include/vcl/outdev.hxx
index 2fddb3c3f24a..e353acd2938e 100644
--- a/include/vcl/outdev.hxx
+++ b/include/vcl/outdev.hxx
@@ -21,7 +21,6 @@
 
 #include <sal/config.h>
 
-#include <comphelper/flagguard.hxx>
 #include <tools/gen.hxx>
 #include <tools/ref.hxx>
 #include <tools/solar.h>
@@ -267,8 +266,6 @@ private:
     mutable bool                    mbRefPoint : 1;
     mutable bool                    mbEnableRTL : 1;
 
-    bool mbNoFontScaling = false; // Used only by D2DWriteTextOutRenderer
-
 protected:
     mutable std::shared_ptr<vcl::font::PhysicalFontCollection> mxFontCollection;
     mutable std::shared_ptr<ImplFontCache> mxFontCache;
@@ -351,8 +348,6 @@ public:
     /// request XSpriteCanvas render interface
     css::uno::Reference< css::rendering::XSpriteCanvas > GetSpriteCanvas() const;
 
-    auto ScopedNoFontScaling() { return comphelper::FlagRestorationGuard(mbNoFontScaling, true); }
-
 protected:
 
     /** Acquire a graphics device that the output device uses to draw on.
diff --git a/include/vcl/vcllayout.hxx b/include/vcl/vcllayout.hxx
index 9370c69ded2e..dd0747eae3ec 100644
--- a/include/vcl/vcllayout.hxx
+++ b/include/vcl/vcllayout.hxx
@@ -22,7 +22,6 @@
 #include <basegfx/point/b2dpoint.hxx>
 #include <basegfx/polygon/b2dpolypolygon.hxx>
 #include <basegfx/range/b2drectangle.hxx>
-#include <comphelper/flagguard.hxx>
 #include <i18nlangtag/languagetag.hxx>
 #include <tools/gen.hxx>
 #include <tools/degree.hxx>
@@ -121,9 +120,6 @@ public:
 
     virtual SalLayoutGlyphs GetGlyphs() const;
 
-    auto ScopedFontScaling(bool v) { return comphelper::FlagRestorationGuard(mbScaleFont, v); }
-    bool ScaleFont() const { return mbScaleFont; }
-
 protected:
     // used by layout engines
     SalLayout();
@@ -132,8 +128,6 @@ private:
     SalLayout(const SalLayout&) = delete;
     SalLayout& operator=(const SalLayout&) = delete;
 
-    bool mbScaleFont = true; // Used only by D2DWriteTextOutRenderer
-
 protected:
     int             mnMinCharPos;
     int             mnEndCharPos;
diff --git a/sd/qa/unit/PNGExportTests.cxx b/sd/qa/unit/PNGExportTests.cxx
index c2af95329ca2..78135e3a5eba 100644
--- a/sd/qa/unit/PNGExportTests.cxx
+++ b/sd/qa/unit/PNGExportTests.cxx
@@ -941,4 +941,65 @@ CPPUNIT_TEST_FIXTURE(SdPNGExportTest, testTdf155048)
     }
 }
 
+CPPUNIT_TEST_FIXTURE(SdPNGExportTest, testTdf162259)
+{
+    // The top X in the SVG, having no skew, used a fast rendering path, and was output much wider
+    // than the bottom one, which has a skew. Test the rendered pixels inside the known boundaries.
+
+    loadFromFile(u"svg/tdf162259.svg");
+
+    auto xGraphicExporter = drawing::GraphicExportFilter::create(getComponentContext());
+    CPPUNIT_ASSERT(xGraphicExporter);
+
+    auto xSupplier = mxComponent.queryThrow<css::drawing::XDrawPagesSupplier>();
+    auto xPage = xSupplier->getDrawPages()->getByIndex(0).queryThrow<css::lang::XComponent>();
+    xGraphicExporter->setSourceDocument(xPage);
+
+    // 101 x 151 is current width x height ratio of the loaded SVG. FIXME: it should be 100 x 150.
+    css::uno::Sequence<css::beans::PropertyValue> aFilterData{
+        comphelper::makePropertyValue(u"PixelWidth"_ustr, sal_Int32(101)),
+        comphelper::makePropertyValue(u"PixelHeight"_ustr, sal_Int32(151)),
+    };
+
+    css::uno::Sequence<css::beans::PropertyValue> aDescriptor{
+        comphelper::makePropertyValue(u"URL"_ustr, maTempFile.GetURL()),
+        comphelper::makePropertyValue(u"FilterName"_ustr, u"PNG"_ustr),
+        comphelper::makePropertyValue(u"FilterData"_ustr, aFilterData)
+    };
+
+    xGraphicExporter->filter(aDescriptor);
+    BitmapEx bmp = vcl::PngImageReader(*maTempFile.GetStream(StreamMode::READ)).read();
+
+    tools::Rectangle topX(12, 21, 37, 60);
+    int topNonWhites = 0;
+    tools::Rectangle bottomX(13, 83, 37, 126);
+    int bottomNonWhites = 0;
+
+    // Check that there is nothing outside the X recrangles
+    for (tools::Long x = 0; x < bmp.GetSizePixel().Width(); ++x)
+    {
+        for (tools::Long y = 0; y < bmp.GetSizePixel().Height(); ++y)
+        {
+            if (topX.Contains(Point{ x, y }))
+            {
+                if (bmp.GetPixelColor(x, y) != COL_WHITE)
+                    ++topNonWhites;
+            }
+            else if (bottomX.Contains(Point{ x, y }))
+            {
+                if (bmp.GetPixelColor(x, y) != COL_WHITE)
+                    ++bottomNonWhites;
+            }
+            else
+            {
+                OString msg("Pixel: "_ostr + OString::number(x) + "," + OString::number(y));
+                CPPUNIT_ASSERT_EQUAL_MESSAGE(msg.getStr(), COL_WHITE, bmp.GetPixelColor(x, y));
+            }
+        }
+    }
+
+    CPPUNIT_ASSERT_GREATER(350, topNonWhites); // 399 in my testing
+    CPPUNIT_ASSERT_GREATER(350, bottomNonWhites); // 362 in my testing
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sd/qa/unit/data/svg/tdf162259.svg b/sd/qa/unit/data/svg/tdf162259.svg
new file mode 100644
index 000000000000..96e7bd930c8d
--- /dev/null
+++ b/sd/qa/unit/data/svg/tdf162259.svg
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1" width="100" height="150" viewBox="0 0 100 150">
+ <g>
+  <text style="font-size:5.9px;font-family:Liberation Serif"
+        transform="scale(6,10)"
+        x="2" y="6">
+   <tspan>X</tspan>
+  </text>
+  <text style="font-size:5.9px;font-family:Liberation Serif"
+        transform="scale(6,10) skewY(5)"
+        x="2" y="12">
+   <tspan>X</tspan>
+  </text>
+ </g>
+</svg>
diff --git a/sw/source/uibase/sidebar/QuickFindPanel.cxx b/sw/source/uibase/sidebar/QuickFindPanel.cxx
index 4da7639ca8c2..0df67a256742 100644
--- a/sw/source/uibase/sidebar/QuickFindPanel.cxx
+++ b/sw/source/uibase/sidebar/QuickFindPanel.cxx
@@ -10,6 +10,8 @@
 
 #include "QuickFindPanel.hxx"
 #include <com/sun/star/lang/IllegalArgumentException.hpp>
+
+#include <comphelper/scopeguard.hxx>
 #include <svl/srchitem.hxx>
 #include <view.hxx>
 #include <swmodule.hxx>
diff --git a/vcl/inc/win/winlayout.hxx b/vcl/inc/win/winlayout.hxx
index 31066a7db28a..cfb36e825b54 100644
--- a/vcl/inc/win/winlayout.hxx
+++ b/vcl/inc/win/winlayout.hxx
@@ -36,6 +36,8 @@ class WinFontInstance : public LogicalFontInstance
 public:
     ~WinFontInstance() override;
 
+    float getHScale() const;
+
     void SetGraphics(WinSalGraphics*);
     WinSalGraphics* GetGraphics() const { return m_pGraphics; }
 
diff --git a/vcl/source/outdev/text.cxx b/vcl/source/outdev/text.cxx
index 2d41dd412888..8bbd12160bd1 100644
--- a/vcl/source/outdev/text.cxx
+++ b/vcl/source/outdev/text.cxx
@@ -449,7 +449,6 @@ void OutputDevice::ImplDrawSpecialText( SalLayout& rSalLayout )
 
 void OutputDevice::ImplDrawText( SalLayout& rSalLayout )
 {
-    auto guard = rSalLayout.ScopedFontScaling(!mbNoFontScaling);
     if( mbInitClipRegion )
         InitClipRegion();
     if( mbOutputClipped )
diff --git a/vcl/win/gdi/DWriteTextRenderer.cxx b/vcl/win/gdi/DWriteTextRenderer.cxx
index 1731a1e4c379..f25fe80cd79d 100644
--- a/vcl/win/gdi/DWriteTextRenderer.cxx
+++ b/vcl/win/gdi/DWriteTextRenderer.cxx
@@ -99,7 +99,7 @@ HRESULT checkResult(HRESULT hr, const char* location)
 class WinFontTransformGuard
 {
 public:
-    WinFontTransformGuard(ID2D1RenderTarget* pRenderTarget,
+    WinFontTransformGuard(ID2D1RenderTarget* pRenderTarget, float hscale,
                           const GenericSalLayout& rLayout, const D2D1_POINT_2F& rBaseline,
                           bool bIsVertical);
     ~WinFontTransformGuard();
@@ -247,17 +247,18 @@ bool D2DWriteTextOutRenderer::performRender(GenericSalLayout const & rLayout, Sa
     {
         mpRT->BeginDraw();
 
+        const float hscale = rWinFont.getHScale();
         int nStart = 0;
         basegfx::B2DPoint aPos;
         const GlyphItem* pGlyph;
         while (rLayout.GetNextGlyph(&pGlyph, aPos, nStart))
         {
             UINT16 glyphIndices[] = { static_cast<UINT16>(pGlyph->glyphId()) };
-            FLOAT glyphAdvances[] = { static_cast<FLOAT>(pGlyph->newWidth()) };
+            FLOAT glyphAdvances[] = { static_cast<FLOAT>(pGlyph->newWidth()) / hscale };
             DWRITE_GLYPH_OFFSET glyphOffsets[] = { { 0.0f, 0.0f }, };
-            D2D1_POINT_2F baseline = { static_cast<FLOAT>(aPos.getX() - bounds.Left()),
+            D2D1_POINT_2F baseline = { static_cast<FLOAT>(aPos.getX() - bounds.Left()) / hscale,
                                        static_cast<FLOAT>(aPos.getY() - bounds.Top()) };
-            WinFontTransformGuard aTransformGuard(mpRT, rLayout, baseline, pGlyph->IsVertical());
+            WinFontTransformGuard aTransformGuard(mpRT, hscale, rLayout, baseline, pGlyph->IsVertical());
             DWRITE_GLYPH_RUN glyphs = {
                 pFontFace,
                 lfEmHeight,
@@ -305,22 +306,12 @@ IDWriteFontFace* D2DWriteTextOutRenderer::GetDWriteFace(const WinFontInstance& r
     return pFontFace;
 }
 
-WinFontTransformGuard::WinFontTransformGuard(ID2D1RenderTarget* pRenderTarget,
+WinFontTransformGuard::WinFontTransformGuard(ID2D1RenderTarget* pRenderTarget, float hscale,
                                              const GenericSalLayout& rLayout,
                                              const D2D1_POINT_2F& rBaseline,
                                              bool bIsVertical)
     : mpRenderTarget(pRenderTarget)
 {
-    const float hscale = [&rLayout]
-    {
-        if (!rLayout.ScaleFont())
-            return 1.0;
-        const auto& rPattern = rLayout.GetFont().GetFontSelectPattern();
-        if (!rPattern.mnHeight || !rPattern.mnWidth)
-            return 1.0;
-        return rPattern.mnWidth * rLayout.GetFont().GetAverageWidthFactor() / rPattern.mnHeight;
-    }();
-
     Degree10 angle = rLayout.GetOrientation();
     if (bIsVertical)
         angle += 900_deg10;
diff --git a/vcl/win/gdi/winlayout.cxx b/vcl/win/gdi/winlayout.cxx
index 0c64759e1ab8..19eaae2ecee7 100644
--- a/vcl/win/gdi/winlayout.cxx
+++ b/vcl/win/gdi/winlayout.cxx
@@ -146,6 +146,14 @@ WinFontInstance::~WinFontInstance()
         ::DeleteFont(m_hFont);
 }
 
+float WinFontInstance::getHScale() const
+{
+    const vcl::font::FontSelectPattern& rPattern = GetFontSelectPattern();
+    if (!rPattern.mnHeight || !rPattern.mnWidth)
+        return 1.0;
+    return rPattern.mnWidth * GetAverageWidthFactor() / rPattern.mnHeight;
+}
+
 void WinFontInstance::ImplInitHbFont(hb_font_t* /*pHbFont*/)
 {
     assert(m_pGraphics);