make SalLayoutGlyphsImpl::cloneCharRange() work for RTL runs too

Glyphs are in the reverse order in this case, so the character
positions for the wanted range must be treated that way. This improves
e.g. the second attachment from tdf#112989. Unfortunately it's
not that significant, as arabic glyphs are often unsafe to break at.

Change-Id: I836ebff6282c420462c5cd5906d30d2e9431f218
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133494
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

1 files changed, 32 insertions, 10 deletions

diff --git a/vcl/source/gdi/impglyphitem.cxx b/vcl/source/gdi/impglyphitem.cxx
index d6cd36ab1b5c..09acbae46150 100644
--- a/vcl/source/gdi/impglyphitem.cxx
+++ b/vcl/source/gdi/impglyphitem.cxx
@@ -95,15 +95,33 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
 {
     std::unique_ptr<SalLayoutGlyphsImpl> copy(new SalLayoutGlyphsImpl(*GetFont()));
     copy->SetFlags(GetFlags());
+    if (empty())
+        return copy.release();
     copy->reserve(std::min<size_t>(size(), length));
-    // Skip glyphs that are in the string before the given index (glyphs are sorted by charPos()).
-    const_iterator pos = std::partition_point(
-        begin(), end(), [index](const GlyphItem& it) { return it.charPos() < index; });
+    sal_Int32 beginPos = index;
+    sal_Int32 endPos = index + length;
+    const_iterator pos;
+    bool rtl = front().IsRTLGlyph();
+    if (rtl)
+    {
+        // Glyphs are in reverse order for RTL.
+        beginPos = index + length - 1;
+        endPos = index - 1;
+        // Skip glyphs that are in the string after the given index, i.e. are before the glyphs
+        // we want.
+        pos = std::partition_point(
+            begin(), end(), [beginPos](const GlyphItem& it) { return it.charPos() > beginPos; });
+    }
+    else
+    {
+        // Skip glyphs that are in the string before the given index (glyphs are sorted by charPos()).
+        pos = std::partition_point(
+            begin(), end(), [beginPos](const GlyphItem& it) { return it.charPos() < beginPos; });
+    }
     if (pos == end())
         return nullptr;
     // Require a start at the exact position given, otherwise bail out.
-    // TODO: This bails out also for RTL text.
-    if (pos->charPos() != index)
+    if (pos->charPos() != beginPos)
         return nullptr;
     // Don't create a subset if it's not safe to break at the beginning or end of the sequence
     // (https://harfbuzz.github.io/harfbuzz-hb-buffer.html#hb-glyph-flags-t).
@@ -113,15 +131,17 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
     // that's how it's computed in GenericSalLayout::LayoutText().
     DevicePoint zeroPoint = pos->linearPos() - DevicePoint(pos->xOffset(), pos->yOffset());
     // Add and adjust all glyphs until the given length.
-    // The check is written as 'charPos + charCount <= index + length' rather than
-    // 'charPos < index + length' to make sure we include complete glyphs. If a glyph is composed
+    // The check is written as 'charPos + charCount <= endPos' rather than 'charPos < endPos'
+    // (or similarly for RTL) to make sure we include complete glyphs. If a glyph is composed
     // from several characters, we should not cut in the middle of those characters, so this
     // checks the glyph is entirely in the given character range. If it is not, this will end
     // the loop and the later 'pos->charPos() != endPos' check will fail and bail out.
     // CppunitTest_sw_layoutwriter's testCombiningCharacterCursorPosition would fail without this.
-    while (pos != end() && pos->charPos() + pos->charCount() <= index + length)
+    while (pos != end()
+           && (rtl ? pos->charPos() - pos->charCount() >= endPos
+                   : pos->charPos() + pos->charCount() <= endPos))
     {
-        if (pos->IsRTLGlyph())
+        if (pos->IsRTLGlyph() != rtl)
             return nullptr; // Don't mix RTL and non-RTL runs.
         copy->push_back(*pos);
         copy->back().setLinearPos(copy->back().linearPos() - zeroPoint);
@@ -129,7 +149,9 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
     }
     if (pos != end())
     {
-        if (pos->charPos() != index + length)
+        // Fail if the next character is at the expected past-end position. For RTL check
+        // that we're not cutting in the middle of a multi-character glyph.
+        if (rtl ? pos->charPos() + pos->charCount() != endPos + 1 : pos->charPos() != endPos)
             return nullptr;
         if (pos->IsUnsafeToBreak() || (pos->IsInCluster() && !pos->IsClusterStart()))
             return nullptr;