vcl/inc/sallayout.hxx | 1
vcl/qa/cppunit/complextext.cxx | 59 ++++++++++++++++++++++++++++++++++++
vcl/source/gdi/sallayout.cxx | 66 ++++++++++++++++++++++-------------------
3 files changed, 97 insertions(+), 29 deletions(-)
New commits:
commit 4ce43750e7878a813127bdc409d5251369d035f5
Author: Khaled Hosny <kha...@libreoffice.org>
AuthorDate: Mon Jun 12 22:29:11 2023 +0300
Commit: خالد حسني <kha...@libreoffice.org>
CommitDate: Tue Jun 13 01:53:19 2023 +0200
tdf#152048: Fix measuring text width with font fallback
This is a regression from:
commit 43a5400063b17ed4ba4cbb38bdf5da4a991f60e2
Author: Khaled Hosny <kha...@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: خالد حسني <kha...@libreoffice.org>
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
commit 507759b449415e897ac0a1de03e8dd685b1e50fe
Author: Khaled Hosny <kha...@libreoffice.org>
AuthorDate: Mon Jun 12 19:24:35 2023 +0300
Commit: خالد حسني <kha...@libreoffice.org>
CommitDate: Tue Jun 13 01:53:08 2023 +0200
tdf#152048: Add test
Change-Id: I321e9f400b0d96b9c3193547f184342ce0c69f7c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152907
Tested-by: Jenkins
Reviewed-by: خالد حسني <kha...@libreoffice.org>
diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx
index 6a395899778c..36fe89c9c151 100644
--- a/vcl/qa/cppunit/complextext.cxx
+++ b/vcl/qa/cppunit/complextext.cxx
@@ -362,4 +362,42 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testGdefCaret)
#endif
}
+CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf152048)
+{
+#if HAVE_MORE_FONTS
+ OUString aText(u"میشود");
+
+ 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
+ std::vector<sal_Int32> aRefCharWidths{ 33, 82, 82, 129, 163, 193 };
+ tools::Long nRefTextWidth(193);
+
+ KernArray aCharWidths;
+ tools::Long nTextWidth = pOutDev->GetTextArray(aText, &aCharWidths);
+
+ CPPUNIT_ASSERT_EQUAL(aRefCharWidths, aCharWidths.get_subunit_array());
+ CPPUNIT_ASSERT_EQUAL(nRefTextWidth, nTextWidth);
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(nTextWidth), aCharWidths.back());
+
+ // Simulate Kashida insertion using Kashida array and extending text array
+ // to have room for Kashida.
+ std::vector<sal_Bool> aKashidaArray{ false, false, false, true, false,
false };
+ auto nKashida = 200;
+
+ aCharWidths.set(3, aCharWidths[3] + nKashida);
+ aCharWidths.set(4, aCharWidths[4] + nKashida);
+ aCharWidths.set(5, aCharWidths[5] + nKashida);
+ auto pLayout = pOutDev->ImplLayout(aText, 0, -1, Point(0, 0), 0,
aCharWidths, aKashidaArray);
+
+ // Without the fix this fails with:
+ // - Expected: 393
+ // - Actual : 511
+ CPPUNIT_ASSERT_EQUAL(DeviceCoordinate(nRefTextWidth + nKashida),
pLayout->GetTextWidth());
+#endif
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
