tdf#152048: Fix measuring text width with font fallback

This is a regression from:

commit 43a5400063b17ed4ba4cbb38bdf5da4a991f60e2
Author: Khaled Hosny <khaled@libreoffice.org>
Date:   Thu May 25 10:59:18 2023 +0300

tdf#152048: Fix underline width for Kashida-justified text

It fixed measuring width when there is Kashida justification, but broke
it for font fallback, because of the way MultiSalLayout measures the
text width takes the maximum of the text widths for all its layouts, but
layouts with missing glyphs are now giving wrong text width as they are
measuring the width of .notdef (GID 0) glyph.

This change makes MultiSalLayout measure the text width by iterating
over all its valid glyphs.

Change-Id: I8b19e0d44326c6f0afe6e504ab6d90c6fb3575f9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152933
Tested-by: Jenkins
Reviewed-by: خالد حسني <khaled@libreoffice.org>

3 files changed, 59 insertions, 29 deletions

diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index 35496750c286..d48edaaf67f2 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -62,6 +62,7 @@ class MultiSalLayout final : public SalLayout
 public:
     void            DrawText(SalGraphics&) const override;
     sal_Int32       GetTextBreak(DeviceCoordinate nMaxWidth, DeviceCoordinate nCharExtra, int nFactor) const override;
+    DeviceCoordinate GetTextWidth() const final override;
     DeviceCoordinate FillDXArray(std::vector<DeviceCoordinate>* pDXArray, const OUString& rStr) const override;
     void            GetCaretPositions(int nArraySize, sal_Int32* pCaretXArray) const override;
     bool            GetNextGlyph(const GlyphItem** pGlyph, DevicePoint& rPos, int& nStart,
diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx
index 36fe89c9c151..9473d5b7b696 100644
--- a/vcl/qa/cppunit/complextext.cxx
+++ b/vcl/qa/cppunit/complextext.cxx
@@ -400,4 +400,25 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf152048)
 #endif
 }
 
+CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf152048_2)
+{
+#if HAVE_MORE_FONTS
+    vcl::Font aFont(u"Noto Naskh Arabic", u"Regular", Size(0, 72));
+
+    ScopedVclPtrInstance<VirtualDevice> pOutDev;
+    pOutDev->SetFont(aFont);
+
+    // get an compare the default text array
+    KernArray aCharWidths;
+    auto nTextWidth = pOutDev->GetTextArray(u"ع a ع", &aCharWidths);
+
+    // Text width should always be equal to the width of the last glyph in the
+    // kern array.
+    // Without the fix this fails with:
+    // - Expected: 158
+    // - Actual  : 118
+    CPPUNIT_ASSERT_EQUAL(aCharWidths.back(), sal_Int32(nTextWidth));
+#endif
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 1ed2a37ad71b..3cdd71642b5b 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -1009,45 +1009,53 @@ sal_Int32 MultiSalLayout::GetTextBreak( DeviceCoordinate nMaxWidth, DeviceCoordi
     return -1;
 }
 
-DeviceCoordinate MultiSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths, const OUString& rStr ) const
+DeviceCoordinate MultiSalLayout::GetTextWidth() const
 {
-    DeviceCoordinate nMaxWidth = 0;
+    // Measure text width. There might be holes in each SalLayout due to
+    // missing chars, so we use GetNextGlyph() to get the glyphs across all
+    // layouts.
+    int nStart = 0;
+    DevicePoint aPos;
+    const GlyphItem* pGlyphItem;
 
-    // prepare merging of fallback levels
-    std::vector<DeviceCoordinate> aTempWidths;
-    const int nCharCount = mnEndCharPos - mnMinCharPos;
-    if( pCharWidths )
+    DeviceCoordinate nWidth = 0;
+    while (GetNextGlyph(&pGlyphItem, aPos, nStart))
+        nWidth += pGlyphItem->newWidth();
+
+    return nWidth;
+}
+
+DeviceCoordinate MultiSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths, const OUString& rStr ) const
+{
+    if (pCharWidths)
     {
+        // prepare merging of fallback levels
+        std::vector<DeviceCoordinate> aTempWidths;
+        const int nCharCount = mnEndCharPos - mnMinCharPos;
         pCharWidths->clear();
         pCharWidths->resize(nCharCount, 0);
-    }
 
-    for( int n = mnLevel; --n >= 0; )
-    {
-        // query every fallback level
-        DeviceCoordinate nTextWidth = mpLayouts[n]->FillDXArray( &aTempWidths, rStr );
-        if( !nTextWidth )
-            continue;
-        // merge results from current level
-        if( nMaxWidth < nTextWidth )
-            nMaxWidth = nTextWidth;
-        if( !pCharWidths )
-            continue;
-        // calculate virtual char widths using most probable fallback layout
-        for( int i = 0; i < nCharCount; ++i )
+        for (int n = mnLevel; --n >= 0;)
         {
-            // #i17359# restriction:
-            // one char cannot be resolved from different fallbacks
-            if( (*pCharWidths)[i] != 0 )
-                continue;
-            DeviceCoordinate nCharWidth = aTempWidths[i];
-            if( !nCharWidth )
-                continue;
-            (*pCharWidths)[i] = nCharWidth;
+            // query every fallback level
+            mpLayouts[n]->FillDXArray(&aTempWidths, rStr);
+
+            // calculate virtual char widths using most probable fallback layout
+            for (int i = 0; i < nCharCount; ++i)
+            {
+                // #i17359# restriction:
+                // one char cannot be resolved from different fallbacks
+                if ((*pCharWidths)[i] != 0)
+                    continue;
+                DeviceCoordinate nCharWidth = aTempWidths[i];
+                if (!nCharWidth)
+                    continue;
+                (*pCharWidths)[i] = nCharWidth;
+            }
         }
     }
 
-    return nMaxWidth;
+    return GetTextWidth();
 }
 
 void MultiSalLayout::GetCaretPositions( int nMaxIndex, sal_Int32* pCaretXArray ) const