vcl/qa/cppunit/complextext.cxx | 39 +++++++++++++++++++++++++++++++++++++++ vcl/source/gdi/sallayout.cxx | 8 ++++++++ 2 files changed, 47 insertions(+)
New commits: commit 98057a0e54f39160121f7c88b19250e6085d5343 Author: Jonathan Clark <[email protected]> AuthorDate: Tue Mar 25 07:17:48 2025 -0600 Commit: Jonathan Clark <[email protected]> CommitDate: Wed Mar 26 14:18:23 2025 +0100 tdf#165510 vcl: Fix incorrect adjustment of bidi MultiSalLayouts The final step in laying out text with font substitutions is to drop invalid glyphs from the base layout and position glyphs from substitution layouts in their place. This change fixes a logic error in this algorithm which was causing tofu and overlapping characters in the frequent case of an all-LTR base layout and an all-RTL substitution layout containing multiple fallback runs. Change-Id: I0d8b03671a77c7d4c41c9ea0f32b526ed44477fe Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183340 Tested-by: Jenkins Reviewed-by: Jonathan Clark <[email protected]> diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx index 693cde25b206..c8a9e6e7c3cd 100644 --- a/vcl/qa/cppunit/complextext.cxx +++ b/vcl/qa/cppunit/complextext.cxx @@ -758,4 +758,43 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf163215) CPPUNIT_ASSERT(aFoundPositions2.at(4)); } +CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf165510) +{ + ScopedVclPtrInstance<VirtualDevice> pOutDev; + + vcl::Font aBaseFont{ u"Liberation Sans"_ustr, u"Regular"_ustr, Size{ 0, 72 } }; + pOutDev->SetFont(aBaseFont); + + vcl::Font aFallbackFont{ u"Noto Sans"_ustr, u"Regular"_ustr, Size{ 0, 72 } }; + pOutDev->ForceFallbackFont(aFallbackFont); + + auto aText = u"ab(ح)cd(د)ef"_ustr; + auto pLayout = pOutDev->ImplLayout(aText, /*nIndex*/ 0, /*nLen*/ aText.getLength()); + + // Fallback must have happened for this test to be meaningful + auto pMultiLayout = dynamic_cast<MultiSalLayout*>(pLayout.get()); + CPPUNIT_ASSERT(pMultiLayout); + + std::vector<int> aCharIndices; + + const GlyphItem* pGlyph = nullptr; + basegfx::B2DPoint stPos; + int nCurrPos = 0; + while (pLayout->GetNextGlyph(&pGlyph, stPos, nCurrPos)) + { + aCharIndices.push_back(pGlyph->charPos()); + } + + // tdf#165510 caused failure to remove dropped glyphs from the base layout. + // Without the fix, the base layout will contain an errant copy of char 8. + // { 0, 1, 2, 4, 5, 6, 7, 8, 9, 10, 11, 3, 8 }; + std::vector<int> aRefCharIndices{ 0, 1, 2, 4, 5, 6, 7, 9, 10, 11, 3, 8 }; + + CPPUNIT_ASSERT_EQUAL(aRefCharIndices.size(), aCharIndices.size()); + for (size_t i = 0; i < aRefCharIndices.size(); ++i) + { + CPPUNIT_ASSERT_EQUAL(aRefCharIndices.at(i), aCharIndices.at(i)); + } +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx index 742ebdfe61c5..ce4e4ad73cb4 100644 --- a/vcl/source/gdi/sallayout.cxx +++ b/vcl/source/gdi/sallayout.cxx @@ -1025,6 +1025,14 @@ void MultiSalLayout::ImplAdjustMultiLayout(vcl::text::ImplLayoutArgs& rArgs, { if (maFallbackRuns[i].GetRun(&nRunStart, &nRunEnd, &bRtl)) { + // tdf#165510: Need to use the direction of the current character, + // not the direction of the fallback run. + nActiveCharIndex = nActiveCharPos - mnMinCharPos; + if (nActiveCharIndex >= 0) + { + bRtl = vRtl[nActiveCharIndex]; + } + if (bRtl) { if (nRunStart > nActiveCharPos)
