Related: tdf#124109 vcl: make glyph item caching work with kashida glyphs

This was broken because GenericSalLayout::LayoutText() sets the
SalLayoutFlags::KashidaJustification flag as a side-effect of building
the glyph list. So in case we cache the list and not build it, the flag
will be missing. This means that later in
GenericSalLayout::ApplyDXArray() kashida glyphs won't be inserted.

With this, the workaround in sw can be removed.

Change-Id: Ic18211bf50ca673daa238e8950a381915e4b3096
Reviewed-on: https://gerrit.libreoffice.org/69566
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

6 files changed, 39 insertions, 12 deletions

diff --git a/sw/source/core/txtnode/fntcache.cxx b/sw/source/core/txtnode/fntcache.cxx
index e749bc2bdac4..ed39672d4f5a 100644
--- a/sw/source/core/txtnode/fntcache.cxx
+++ b/sw/source/core/txtnode/fntcache.cxx
@@ -1492,7 +1492,6 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
 
         long nSpaceAdd = rInf.GetSpace() / SPACING_PRECISION_FACTOR;
         bool bNoHalfSpace = false;
-        bool bCacheLayout = true;
 
         if ( rInf.GetFont() && rInf.GetLen() )
         {
@@ -1535,12 +1534,7 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
                     if ( pSI && pSI->CountKashida() &&
                          pSI->KashidaJustify( pKernArray.get(), pScrArray.get(), rInf.GetIdx(),
                                               rInf.GetLen(), nSpaceAdd ) != -1 )
-                    {
                         nSpaceAdd = 0;
-                        // Layout can't be reused in this case, it would lead to missing gaps in
-                        // place of kashida.
-                        bCacheLayout = false;
-                    }
                     else
                         bNoHalfSpace = true;
                 }
@@ -1821,10 +1815,7 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
                             ? (rInf.GetIdx() ? 1 : 0)
                             : sal_Int32(rInf.GetIdx());
                 aGlyphsKey = SwTextGlyphsKey{ &rInf.GetOut(), *pStr, nTmpIdx, nLen };
-                if (bCacheLayout)
-                    pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
-                else
-                    pGlyphs = nullptr;
+                pGlyphs = lcl_CreateLayout(aGlyphsKey, m_aTextGlyphs[aGlyphsKey]);
                 rInf.GetOut().DrawTextArray( aTextOriginPos, *pStr, pKernArray.get(),
                                              nTmpIdx , nLen, SalLayoutFlags::NONE, pGlyphs );
                 if (bBullet)
diff --git a/vcl/CppunitTest_vcl_complextext.mk b/vcl/CppunitTest_vcl_complextext.mk
index 56b9bcd3f72f..540110af99a8 100644
--- a/vcl/CppunitTest_vcl_complextext.mk
+++ b/vcl/CppunitTest_vcl_complextext.mk
@@ -28,6 +28,7 @@ $(eval $(call gb_CppunitTest_use_libraries,vcl_complextext, \
 	comphelper \
 	cppu \
 	cppuhelper \
+	i18nlangtag \
 	sal \
 	svt \
 	test \
diff --git a/vcl/inc/impglyphitem.hxx b/vcl/inc/impglyphitem.hxx
index 24690216db63..987df438894c 100644
--- a/vcl/inc/impglyphitem.hxx
+++ b/vcl/inc/impglyphitem.hxx
@@ -23,6 +23,7 @@
 #include <tools/gen.hxx>
 #include <vcl/dllapi.h>
 #include <vcl/glyphitem.hxx>
+#include <vcl/outdev.hxx>
 #include <vector>
 
 #include "fontinstance.hxx"
@@ -103,6 +104,7 @@ public:
 
 private:
     mutable rtl::Reference<LogicalFontInstance> m_rFontInstance;
+    SalLayoutFlags mnFlags = SalLayoutFlags::NONE;
 
     SalLayoutGlyphsImpl(SalLayoutGlyphs& rGlyphs, LogicalFontInstance& rFontInstance)
         : m_rFontInstance(&rFontInstance)
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index e71359e48165..535838aae645 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -73,7 +73,7 @@ public:
     bool    PosIsInAnyRun( int nCharPos ) const;
 };
 
-class ImplLayoutArgs
+class VCL_DLLPUBLIC ImplLayoutArgs
 {
 public:
     // string related inputs
@@ -158,7 +158,7 @@ private:
     bool            mbIncomplete;
 };
 
-class VCL_PLUGIN_PUBLIC GenericSalLayout : public SalLayout
+class VCL_DLLPUBLIC GenericSalLayout : public SalLayout
 {
     friend void MultiSalLayout::AdjustLayout(ImplLayoutArgs&);
 
diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx
index 96332585673b..99b05481ed01 100644
--- a/vcl/qa/cppunit/complextext.cxx
+++ b/vcl/qa/cppunit/complextext.cxx
@@ -19,8 +19,10 @@ static std::ostream& operator<<(std::ostream& rStream, const std::vector<long>&
 #include <test/bootstrapfixture.hxx>
 
 #include <vcl/wrkwin.hxx>
+#include <vcl/virdev.hxx>
 // workaround MSVC2015 issue with std::unique_ptr
 #include <sallayout.hxx>
+#include <salgdi.hxx>
 
 #include <osl/file.hxx>
 #include <osl/process.h>
@@ -45,6 +47,7 @@ public:
 #if HAVE_MORE_FONTS
     /// Play with font measuring etc.
     void testArabic();
+    void testKashida();
 #endif
 #if defined(_WIN32)
     void testTdf95650(); // Windows-only issue
@@ -53,6 +56,7 @@ public:
     CPPUNIT_TEST_SUITE(VclComplexTextTest);
 #if HAVE_MORE_FONTS
     CPPUNIT_TEST(testArabic);
+    CPPUNIT_TEST(testKashida);
 #endif
 #if defined(_WIN32)
     CPPUNIT_TEST(testTdf95650);
@@ -136,6 +140,29 @@ void VclComplexTextTest::testArabic()
     (void)aRect; (void)aRectRot;
 #endif
 }
+
+void VclComplexTextTest::testKashida()
+{
+    // Cache the glyph list of some Arabic text.
+    ScopedVclPtrInstance<VirtualDevice> pOutputDevice;
+    auto aText
+        = OUString::fromUtf8(u8"ﻊﻨﺻﺭ ﺎﻠﻓﻮﺴﻓﻭﺭ ﻊﻨﺻﺭ ﻒﻟﺰﻳ ﺺﻠﺑ. ﺖﺘﻛﻮﻧ ﺎﻟﺩﻭﺭﺓ ﺎﻟﺭﺎﺒﻋﺓ ﻢﻧ ١٥ ﻊﻨﺻﺭﺍ.");
+    std::unique_ptr<SalLayout> pLayout = pOutputDevice->ImplLayout(
+        aText, 0, aText.getLength(), Point(0, 0), 0, nullptr, SalLayoutFlags::GlyphItemsOnly);
+    const SalLayoutGlyphs* pGlyphs = pLayout->GetGlyphs();
+    CPPUNIT_ASSERT(pGlyphs);
+    SalLayoutGlyphs aGlyphs = *pGlyphs;
+
+    // Now lay it out using the cached glyph list.
+    ImplLayoutArgs aLayoutArgs(aText, 0, aText.getLength(), SalLayoutFlags::NONE,
+                               pOutputDevice->GetFont().GetLanguageTag(), nullptr);
+    pLayout = pOutputDevice->GetGraphics()->GetTextLayout(0);
+    CPPUNIT_ASSERT(pLayout->LayoutText(aLayoutArgs, &aGlyphs));
+
+    // Without the accompanying fix in place, this test would have failed with 'assertion failed'.
+    // The kashida justification flag was lost when going via the glyph cache.
+    CPPUNIT_ASSERT(aLayoutArgs.mnFlags & SalLayoutFlags::KashidaJustification);
+}
 #endif
 
 #if defined(_WIN32)
diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx
index bfa4e1a2f503..6187ec6fed5c 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -273,6 +273,8 @@ bool GenericSalLayout::LayoutText(ImplLayoutArgs& rArgs, const SalLayoutGlyphs*
     {
         // Work with pre-computed glyph items.
         m_GlyphItems = *pGlyphs;
+        // Some flags are set as a side effect of text layout, restore them here.
+        rArgs.mnFlags |= pGlyphs->Impl()->mnFlags;
         return true;
     }
 
@@ -580,6 +582,10 @@ bool GenericSalLayout::LayoutText(ImplLayoutArgs& rArgs, const SalLayoutGlyphs*
 
     hb_buffer_destroy(pHbBuffer);
 
+    // Some flags are set as a side effect of text layout, save them here.
+    if (rArgs.mnFlags & SalLayoutFlags::GlyphItemsOnly)
+        m_GlyphItems.Impl()->mnFlags = rArgs.mnFlags;
+
     return true;
 }