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; }