vcl: Better kashida insertion position validation

Use the new HarfBuzz glyph flag HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL if
available to prevent kashida insertion where is would interrupt shaping
(between contextual glyphs). Previously we only prevented it when there
is a ligature across the insertion position, with this change fonts that
use contextual alternate instead of ligatures will be handled better as
well.

Change-Id: Ibb42a310c1e7dbcb225a1ba3acac82154b4edb3a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137649
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

2 files changed, 35 insertions, 5 deletions

diff --git a/vcl/inc/impglyphitem.hxx b/vcl/inc/impglyphitem.hxx
index ea3b2f391334..f0e4e70ef21c 100644
--- a/vcl/inc/impglyphitem.hxx
+++ b/vcl/inc/impglyphitem.hxx
@@ -40,11 +40,12 @@ enum class GlyphItemFlags : sal_uInt16
     ALLOW_KASHIDA = 0x20,
     IS_DROPPED = 0x40,
     IS_CLUSTER_START = 0x80,
-    IS_UNSAFE_TO_BREAK = 0x100 // HB_GLYPH_FLAG_UNSAFE_TO_BREAK from harfbuzz
+    IS_UNSAFE_TO_BREAK = 0x100, // HB_GLYPH_FLAG_UNSAFE_TO_BREAK from harfbuzz
+    IS_SAFE_TO_INSERT_KASHIDA = 0x200 // HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL from harfbuzz
 };
 namespace o3tl
 {
-template <> struct typed_flags<GlyphItemFlags> : is_typed_flags<GlyphItemFlags, 0x1ff>
+template <> struct typed_flags<GlyphItemFlags> : is_typed_flags<GlyphItemFlags, 0x3ff>
 {
 };
 };
@@ -85,6 +86,10 @@ public:
     bool IsDropped() const { return bool(m_nFlags & GlyphItemFlags::IS_DROPPED); }
     bool IsClusterStart() const { return bool(m_nFlags & GlyphItemFlags::IS_CLUSTER_START); }
     bool IsUnsafeToBreak() const { return bool(m_nFlags & GlyphItemFlags::IS_UNSAFE_TO_BREAK); }
+    bool IsSafeToInsertKashida() const
+    {
+        return bool(m_nFlags & GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA);
+    }
 
     inline bool GetGlyphBoundRect(const LogicalFontInstance*, tools::Rectangle&) const;
     inline bool GetGlyphOutline(const LogicalFontInstance*, basegfx::B2DPolyPolygon&) const;
diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx
index 7e2bcd6290cb..98364d4ae443 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -438,6 +438,10 @@ bool GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
             const int nRunLen = nEndRunPos - nMinRunPos;
 
             int nHbFlags = HB_BUFFER_FLAGS_DEFAULT;
+#if HB_VERSION_ATLEAST(5, 1, 0)
+            // Produce HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL that we use below.
+            nHbFlags |= HB_BUFFER_FLAG_PRODUCE_SAFE_TO_INSERT_TATWEEL;
+#endif
             if (nMinRunPos == 0)
                 nHbFlags |= HB_BUFFER_FLAG_BOT; /* Beginning-of-text */
             if (nEndRunPos == nLength)
@@ -577,6 +581,14 @@ bool GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
                 nGlyphFlags |= GlyphItemFlags::IS_UNSAFE_TO_BREAK;
 #endif
 
+#if HB_VERSION_ATLEAST(5, 1, 0)
+                if (hb_glyph_info_get_glyph_flags(&pHbGlyphInfos[i]) & HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL)
+                    nGlyphFlags |= GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA;
+#else
+                // If support is not present, then allow kashida anywhere.
+                nGlyphFlags |= GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA;
+#endif
+
                 DeviceCoordinate nAdvance, nXOffset, nYOffset;
                 if (aSubRun.maDirection == HB_DIRECTION_TTB)
                 {
@@ -837,18 +849,31 @@ bool GenericSalLayout::IsKashidaPosValid(int nCharPos) const
             if (pIter->glyphId() == 0)
                 break;
 
+            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 (pPrev->charPos() != nCharPos)
+#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() == (nCharPos + 1))
-                        return true;
+                    if (pPrev->charPos() == (nClusterEndPos + 1))
+                        return pPrev->IsSafeToInsertKashida();
                     break;
                 }
             }