include/vcl/vcllayout.hxx | 2
vcl/inc/sallayout.hxx | 4 -
vcl/source/gdi/CommonSalLayout.cxx | 82 +++++++++++--------------------------
vcl/source/gdi/sallayout.cxx | 9 ++--
vcl/source/outdev/font.cxx | 26 +++++++++--
5 files changed, 54 insertions(+), 69 deletions(-)
New commits:
commit 5e2b7a656024b621bdeeb6efd977331191b66d9d
Author: Khaled Hosny <kha...@aliftype.com>
AuthorDate: Fri Aug 12 02:12:04 2022 +0200
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Sun Aug 14 21:11:52 2022 +0200
Streamline Kashida validation logic
We are asked to validate the position *after* which Kashida will be
inserted, but HarfBuzz will tell us which glyph we can insert Kashida
*before*. So align both by passing down the position before and after
and make the loop iterating over glyph items a lot simpler.
As a bonus, the new code allow Kashida insertion across layout change in
both sides, old code allowed it only at the start of the layout.
Change-Id: I9f632610b92c0f4c512e50456bf7d207175f17ac
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138168
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caol...@redhat.com>
diff --git a/include/vcl/vcllayout.hxx b/include/vcl/vcllayout.hxx
index 87d9345f4b9b..cbadb47ee0ba 100644
--- a/include/vcl/vcllayout.hxx
+++ b/include/vcl/vcllayout.hxx
@@ -92,7 +92,7 @@ public:
virtual DeviceCoordinate FillDXArray( std::vector<DeviceCoordinate>*
pDXArray ) const = 0;
virtual DeviceCoordinate GetTextWidth() const { return FillDXArray(
nullptr ); }
virtual void GetCaretPositions( int nArraySize, sal_Int32* pCaretXArray
) const = 0;
- virtual bool IsKashidaPosValid ( int /*nCharPos*/ ) const { return
true; } // i60594
+ virtual bool IsKashidaPosValid ( int /*nCharPos*/, int /*nNextCharPos*/
) const = 0; // i60594
// methods using glyph indexing
virtual bool GetNextGlyph(const GlyphItem** pGlyph, DevicePoint& rPos,
int& nStart,
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index 885de2446db7..b49c1c663ee4 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -68,7 +68,7 @@ public:
const LogicalFontInstance** ppGlyphFont =
nullptr,
const vcl::font::PhysicalFontFace**
pFallbackFont = nullptr) const override;
bool GetOutline(basegfx::B2DPolyPolygonVector&) const override;
- bool IsKashidaPosValid(int nCharPos) const override;
+ bool IsKashidaPosValid(int nCharPos, int nNextCharPos) const
override;
SalLayoutGlyphs GetGlyphs() const final override;
// used only by OutputDevice::ImplLayout, TODO: make friend
@@ -115,7 +115,7 @@ public:
void DrawText(SalGraphics&) const final override;
SalLayoutGlyphs GetGlyphs() const final override;
- bool IsKashidaPosValid(int nCharPos) const final override;
+ bool IsKashidaPosValid(int nCharPos, int nNextCharPos) const
final override;
// used by upper layers
DeviceCoordinate GetTextWidth() const final override;
diff --git a/vcl/source/gdi/CommonSalLayout.cxx
b/vcl/source/gdi/CommonSalLayout.cxx
index da7334352858..b3d63dac30a0 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -719,22 +719,19 @@ void GenericSalLayout::ApplyDXArray(const DC* pDXArray,
const sal_Bool* pKashida
m_GlyphItems[i].addNewWidth(nDiff);
m_GlyphItems[i].adjustLinearPosX(nDelta + nDiff);
- // Adjust the X position of the rest of the glyphs in the cluster.
size_t j = i;
while (j > 0)
{
--j;
- if (!m_GlyphItems[j].IsInCluster())
+ if (!(m_GlyphItems[j].IsDiacritic() ||
m_GlyphItems[j].IsInCluster()))
break;
- m_GlyphItems[j].adjustLinearPosX(nDelta + nDiff);
- }
- // Move any non-spacing marks to keep attached to this cluster.
- while (j > 0)
- {
- if (!m_GlyphItems[j].IsDiacritic())
- break;
- m_GlyphItems[j--].adjustLinearPosX(nDiff);
+ if (m_GlyphItems[j].IsInCluster())
+ // Adjust X position of the remainder of the cluster.
+ m_GlyphItems[j].adjustLinearPosX(nDelta + nDiff);
+ else
+ // Move non-spacing marks to keep attached to this cluster.
+ m_GlyphItems[j].adjustLinearPosX(nDiff);
}
// This is a Kashida insertion position, mark it. Kashida glyphs
@@ -807,56 +804,27 @@ void GenericSalLayout::ApplyDXArray(const DC* pDXArray,
const sal_Bool* pKashida
}
}
-bool GenericSalLayout::IsKashidaPosValid(int nCharPos) const
+// Kashida will be inserted between nCharPos and nNextCharPos.
+bool GenericSalLayout::IsKashidaPosValid(int nCharPos, int nNextCharPos) const
{
- for (auto pIter = m_GlyphItems.begin(); pIter != m_GlyphItems.end();
++pIter)
- {
- if (pIter->charPos() == nCharPos)
- {
- // The position is the first glyph, this would happen if we
- // changed the text styling in the middle of a word. Since we don’t
- // do ligatures across layout engine instances, this can’t be a
- // ligature so it should be fine.
- if (pIter == m_GlyphItems.begin())
- return true;
-
- // If the character is not supported by this layout, return false
- // so that fallback layouts would be checked for it.
- if (pIter->glyphId() == 0)
- break;
+ // Search for glyph items corresponding to nCharPos and nNextCharPos.
+ auto const& rGlyph = std::find_if(m_GlyphItems.begin(), m_GlyphItems.end(),
+ [&](const GlyphItem& g) { return
g.charPos() == nCharPos; });
+ auto const& rNextGlyph = std::find_if(m_GlyphItems.begin(),
m_GlyphItems.end(),
+ [&](const GlyphItem& g) { return
g.charPos() == nNextCharPos; });
+
+ // If either is not found then a ligature is created at this position, we
+ // can’t insert Kashida here.
+ if (rGlyph == m_GlyphItems.end() || rNextGlyph == m_GlyphItems.end())
+ return false;
- int nClusterEndPos = nCharPos;
- // Search backwards for previous glyph belonging to a different
- // character. We are looking backwards because we are dealing with
- // RTL glyphs, which will be in visual order.
- for (auto pPrev = pIter - 1; pPrev != m_GlyphItems.begin();
--pPrev)
- {
-#if HB_VERSION_ATLEAST(5, 1, 0)
- // This is a combining mark, keep moving until we find a base,
- // since HarfBuzz will tell as we can’t insert kashida after a
- // mark, but we know know enough to move the marks when
- // inserting kashida.
- if (pPrev->IsDiacritic())
- {
- nClusterEndPos = pPrev->charPos();
- continue;
- }
-#endif
- if (pPrev->charPos() > nClusterEndPos)
- {
- // Check if the found glyph belongs to the next character,
- // and return if it is safe to insert kashida before it,
- // otherwise the current glyph will be a ligature which is
- // invalid kashida position.
- if (pPrev->charPos() == (nClusterEndPos + 1))
- return pPrev->IsSafeToInsertKashida();
- break;
- }
- }
- }
- }
+ // If the either character is not supported by this layout, return false so
+ // that fallback layouts would be checked for it.
+ if (rGlyph->glyphId() == 0 || rNextGlyph->glyphId() == 0)
+ return false;
- return false;
+ // Lastly check if this position is kashida-safe.
+ return rNextGlyph->IsSafeToInsertKashida();
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 0e582015adc8..e043e1f96c28 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -1186,10 +1186,10 @@ bool
MultiSalLayout::GetOutline(basegfx::B2DPolyPolygonVector& rPPV) const
return bRet;
}
-bool MultiSalLayout::IsKashidaPosValid(int nCharPos) const
+bool MultiSalLayout::IsKashidaPosValid(int nCharPos, int nNextCharPos) const
{
// Check the base layout
- bool bValid = mpLayouts[0]->IsKashidaPosValid(nCharPos);
+ bool bValid = mpLayouts[0]->IsKashidaPosValid(nCharPos, nNextCharPos);
// If base layout returned false, it might be because the character was not
// supported there, so we check fallback layouts.
@@ -1198,9 +1198,10 @@ bool MultiSalLayout::IsKashidaPosValid(int nCharPos)
const
for (int i = 1; i < mnLevel; ++i)
{
// - 1 because there is no fallback run for the base layout, IIUC.
- if (maFallbackRuns[i - 1].PosIsInAnyRun(nCharPos))
+ if (maFallbackRuns[i - 1].PosIsInAnyRun(nCharPos) &&
+ maFallbackRuns[i - 1].PosIsInAnyRun(nNextCharPos))
{
- bValid = mpLayouts[i]->IsKashidaPosValid(nCharPos);
+ bValid = mpLayouts[i]->IsKashidaPosValid(nCharPos,
nNextCharPos);
break;
}
}
diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx
index 071056e83690..8bae1c719e37 100644
--- a/vcl/source/outdev/font.cxx
+++ b/vcl/source/outdev/font.cxx
@@ -48,6 +48,8 @@
#include <salgdi.hxx>
#include <svdata.hxx>
+#include <unicode/uchar.h>
+
#include <strings.hrc>
void OutputDevice::SetFont( const vcl::Font& rNewFont )
@@ -1311,14 +1313,28 @@ sal_Int32 OutputDevice::ValidateKashidas ( const
OUString& rTxt,
std::unique_ptr<SalLayout> pSalLayout = ImplLayout( rTxt, nIdx, nLen );
if( !pSalLayout )
return 0;
+
+ auto nEnd = nIdx + nLen - 1;
sal_Int32 nDropped = 0;
for( int i = 0; i < nKashCount; ++i )
{
- if( !pSalLayout->IsKashidaPosValid( pKashidaPos[ i ] ))
- {
- pKashidaPosDropped[ nDropped ] = pKashidaPos [ i ];
- ++nDropped;
- }
+ auto nPos = pKashidaPos[i];
+ auto nNextPos = nPos + 1;
+
+ // Skip combining marks to find the next character after this position.
+ while (nNextPos <= nEnd &&
+ u_getIntPropertyValue(rTxt[nNextPos], UCHAR_JOINING_TYPE) ==
U_JT_TRANSPARENT)
+ nNextPos++;
+
+ // This is the start or end of the layout, it would happen if we
+ // changed the text styling in the middle of a word. Since we don’t do
+ // apply OpenType features across different layouts, this can’t be an
+ // invalid place to insert Kashida.
+ if (nPos == nIdx || nNextPos >= nEnd)
+ continue;
+
+ if (!pSalLayout->IsKashidaPosValid(nPos, nNextPos))
+ pKashidaPosDropped[nDropped++] = nPos;
}
return nDropped;
}
