tdf#107718: Fix script itemization of vertical text

Mixed script vertical text was not correctly splitting runs at script
changes, resulting in Hangul text being mixed with Han text in the same
run breaking the Hangul composition.

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

4 files changed, 64 insertions, 1 deletions

diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx
index 631a24379a7f..bcf96cb4f99b 100644
--- a/vcl/qa/cppunit/complextext.cxx
+++ b/vcl/qa/cppunit/complextext.cxx
@@ -479,4 +479,56 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf153440)
 #endif
 }
 
+CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf107718)
+{
+#if !defined _WIN32 // TODO: Fails on jenkins but passes locally
+    vcl::Font aFont(u"Source Han Sans", u"Regular", Size(0, 72));
+
+    ScopedVclPtrInstance<VirtualDevice> pOutDev;
+
+    bool bAdded = addFont(pOutDev, u"tdf107718.otf", u"Source Han Sans");
+    CPPUNIT_ASSERT_EQUAL(true, bAdded);
+
+    OUString aText(u"\u4E16\u1109\u1168\u11BC\u302E");
+    for (bool bVertical : { false, true })
+    {
+        aFont.SetVertical(bVertical);
+        pOutDev->SetFont(aFont);
+
+        auto pLayout = pOutDev->ImplLayout(aText, 0, -1, Point(0, 0), 0, {}, {});
+
+        int nStart = 0;
+        DevicePoint aPos;
+        const GlyphItem* pGlyphItem;
+        while (pLayout->GetNextGlyph(&pGlyphItem, aPos, nStart))
+        {
+            // Check that we found a font for all characters, a zero glyph ID
+            // means no font was found so the rest of the test would be
+            // meaningless.
+            CPPUNIT_ASSERT(pGlyphItem->glyphId());
+
+            // Assert that we are indeed doing vertical layout for vertical
+            // font since the bug does not happen for horizontal text.
+            CPPUNIT_ASSERT_EQUAL(bVertical, pGlyphItem->IsVertical());
+
+            // For the second glyph, assert that it is a composition of characters 1 to 4
+            // Without the fix this fails with:
+            // - Expected: 4
+            // - Actual  : 1
+            if (nStart == 2)
+            {
+                CPPUNIT_ASSERT_EQUAL(1, pGlyphItem->charPos());
+                CPPUNIT_ASSERT_EQUAL(4, pGlyphItem->charCount());
+            }
+        }
+
+        // Assert there are only three glyphs
+        // Without the fix this fails with:
+        // - Expected: 3
+        // - Actual  : 5
+        CPPUNIT_ASSERT_EQUAL(3, nStart);
+    }
+#endif
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qa/cppunit/data/tdf107718.otf b/vcl/qa/cppunit/data/tdf107718.otf
new file mode 100644
index 000000000000..b892a29a886a
--- /dev/null
+++ b/vcl/qa/cppunit/data/tdf107718.otf
diff --git a/vcl/qa/cppunit/data/tdf107718.otf.readme b/vcl/qa/cppunit/data/tdf107718.otf.readme
new file mode 100644
index 000000000000..b3f740ef58ad
--- /dev/null
+++ b/vcl/qa/cppunit/data/tdf107718.otf.readme
@@ -0,0 +1,11 @@
+This is a subset copy of Source Han Sans font licensed under Open Font License and
+obtained from (the Static Super OTC):
+
+  https://github.com/adobe-fonts/source-han-sans/releases/tag/2.004R
+
+And subset using hb-subset to contain only the one glyph used in the test:
+
+  hb-subset SourceHanSans.ttc --face-index=25 --unicodes="u4E16,u1109,u1168,u11BC,u302E,uC185,u0020" -o tdf107718.otf
+
+U+C185 is not directly used in the test but we need its glyphs. The space is
+added to the subset as it seems needed to get the font to work on Windows.
diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx
index 907197c70c5e..7ea3ba687485 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -391,7 +391,7 @@ bool GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
                             aDirection = bRightToLeft ? HB_DIRECTION_RTL : HB_DIRECTION_LTR;
                         }
 
-                        if (aSubRuns.empty() || aSubRuns.back().maDirection != aDirection)
+                        if (aSubRuns.empty() || aSubRuns.back().maDirection != aDirection || aSubRuns.back().maScript != aScript)
                             aSubRuns.push_back({ nPrevIdx, nIdx, aScript, aDirection });
                         else
                             aSubRuns.back().mnEnd = nIdx;