vcl/qa/cppunit/complextext.cxx | 112 ++++++++++++++++++++++++++++++- vcl/qa/cppunit/data/tdf107718.otf |binary vcl/qa/cppunit/data/tdf107718.otf.readme | 11 +++ vcl/qa/cppunit/data/tdf153440.ttf |binary vcl/qa/cppunit/data/tdf153440.ttf.readme | 12 +++ vcl/source/gdi/CommonSalLayout.cxx | 2 6 files changed, 135 insertions(+), 2 deletions(-)
New commits: commit c44e725fd4cd0d943b7e138889c5621c8f00ddcd Author: Khaled Hosny <[email protected]> AuthorDate: Tue Jun 20 10:17:49 2023 +0300 Commit: خالد حسني <[email protected]> CommitDate: Tue Jun 20 16:55:54 2023 +0200 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: خالد حسني <[email protected]> (cherry picked from commit 1afc13b0b84353109827d78807836c01f030ab66) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153334 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 Binary files /dev/null and b/vcl/qa/cppunit/data/tdf107718.otf differ 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; commit 576349d137eb0bf8132f7b65f30cb9aa276e0b14 Author: Khaled Hosny <[email protected]> AuthorDate: Mon Jun 12 22:48:59 2023 +0300 Commit: خالد حسني <[email protected]> CommitDate: Tue Jun 20 16:55:46 2023 +0200 tdf#153440: Add test Change-Id: I0c036cd1d8022e6ff265aa67ab177c57eba0c4ac Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152934 Tested-by: Jenkins Reviewed-by: خالد حسني <[email protected]> (cherry picked from commit 6b178c6e88ec5a765ad2b3ccd98182286222f550) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153338 diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx index 9473d5b7b696..631a24379a7f 100644 --- a/vcl/qa/cppunit/complextext.cxx +++ b/vcl/qa/cppunit/complextext.cxx @@ -44,8 +44,27 @@ static std::ostream& operator<<(std::ostream& rStream, const std::vector<sal_Int class VclComplexTextTest : public test::BootstrapFixture { + OUString maDataUrl = u"/vcl/qa/cppunit/data/"; + public: - VclComplexTextTest() : BootstrapFixture(true, false) {} + OUString getFullUrl(std::u16string_view sFileName) + { + return m_directories.getURLFromSrc(maDataUrl) + sFileName; + } + + bool addFont(OutputDevice* pOutDev, std::u16string_view sFileName, + std::u16string_view sFamilyName) + { + OutputDevice::ImplClearAllFontData(true); + bool bAdded = pOutDev->AddTempDevFont(getFullUrl(sFileName), OUString(sFamilyName)); + OutputDevice::ImplRefreshAllFontData(true); + return bAdded; + } + + VclComplexTextTest() + : BootstrapFixture(true, false) + { + } }; CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testArabic) @@ -421,4 +440,43 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf152048_2) #endif } +CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf153440) +{ +#if HAVE_MORE_FONTS + vcl::Font aFont(u"Noto Naskh Arabic", u"Regular", Size(0, 72)); + + ScopedVclPtrInstance<VirtualDevice> pOutDev; + pOutDev->SetFont(aFont); + +#if !defined _WIN32 // TODO: Fails on jenkins but passes locally + // Add an emoji font so that we are sure a font will be found for the + // emoji. The font is subset and supports only 🌿. + bool bAdded = addFont(pOutDev, u"tdf153440.ttf", u"Noto Emoji"); + CPPUNIT_ASSERT_EQUAL(true, bAdded); +#endif + + for (auto& aString : { u"ع 🌿 ع", u"a 🌿 a" }) + { + OUString aText(aString); + bool bRTL = aText.startsWith(u"ع"); + + 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)) + { + // Assert glyph ID is not 0, if it is 0 then font fallback didn’t + // happen. + CPPUNIT_ASSERT(pGlyphItem->glyphId()); + + // Assert that we are indeed doing RTL layout for RTL text since + // the bug does not happen for LTR text. + CPPUNIT_ASSERT_EQUAL(bRTL, pGlyphItem->IsRTLGlyph()); + } + } +#endif +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/qa/cppunit/data/tdf153440.ttf b/vcl/qa/cppunit/data/tdf153440.ttf new file mode 100644 index 000000000000..933e74733b0a Binary files /dev/null and b/vcl/qa/cppunit/data/tdf153440.ttf differ diff --git a/vcl/qa/cppunit/data/tdf153440.ttf.readme b/vcl/qa/cppunit/data/tdf153440.ttf.readme new file mode 100644 index 000000000000..6fffab0decab --- /dev/null +++ b/vcl/qa/cppunit/data/tdf153440.ttf.readme @@ -0,0 +1,12 @@ +This is a subset copy of Noto Emoji font licensed under Open Font License and +obtained from: + + https://fonts.google.com/noto/specimen/Noto+Emoji + +And subset using hb-subset to contain only the one glyph used in the test: + + hb-subset static/NotoEmoji-Regular.ttf "🌿 " -o tdf153440.ttf --drop-tables=GSUB,STAT,vhea,vmtx + +The space is added to the subset as it seems needed to get the font to work on +Windows. (The --drop-tables argument is not necessary be saves a few bytes of +stuff we don’t need.)
