don't use glyph subsets with complicated LTR/RTL setups

This should be a more generic solution for problems of some characters
having neutral direction (spaces, commas, etc.) and ending up with
different RTL flag depending on exactly what subset of the string
is wanted (e.g. a space on its own will end up treated as LRT but
if surrounded by RTL text then it'll be flagged as RTL too).
Previous attempts 5d02daa5198d5bff9234d5db698e934a5e31c95f
and 467f2c50a935efff6ff8911e7282ecea535665a3 still left some corner
cases, and it seems that simply requiring BiDiStrong and matching LTR/RTL
for this optimization covers the usual cases.

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

1 files changed, 8 insertions, 17 deletions

diff --git a/vcl/source/gdi/impglyphitem.cxx b/vcl/source/gdi/impglyphitem.cxx
index a3ab689ff23e..ffed4d56c743 100644
--- a/vcl/source/gdi/impglyphitem.cxx
+++ b/vcl/source/gdi/impglyphitem.cxx
@@ -98,11 +98,18 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
     copy->SetFlags(GetFlags());
     if (empty())
         return copy.release();
+    bool rtl = front().IsRTLGlyph();
+    // Avoid mixing LTR/RTL or layouts that do not have it set explicitly (BiDiStrong). Otherwise
+    // the subset may not quite match what would a real layout call give (e.g. some characters with neutral
+    // direction such as space might have different LTR/RTL flag). It seems bailing out here mostly
+    // avoid relatively rare corner cases and doesn't matter for performance.
+    if (!(GetFlags() & SalLayoutFlags::BiDiStrong)
+        || rtl != bool(GetFlags() & SalLayoutFlags::BiDiRtl))
+        return nullptr;
     copy->reserve(std::min<size_t>(size(), length));
     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.
@@ -164,22 +171,6 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
         if (!isSafeToBreak(pos, rtl))
             return nullptr;
     }
-    // HACK: If mode is set to be RTL, but the last glyph is a non-RTL space,
-    // then making a subset would give a different result than the actual layout,
-    // because the weak BiDi mode code in ImplLayoutArgs ctor would interpret
-    // the string subset ending with space as the space being RTL, but it would
-    // treat it as non-RTL for the whole string if there would be more non-RTL
-    // characters after the space. So bail out.
-    if (GetFlags() & SalLayoutFlags::BiDiRtl && !rtl && !copy->empty() && copy->back().IsSpacing())
-    {
-        return nullptr;
-    }
-    // Similarly, if mode is not RTL but the last glyph is an RTL space.
-    if (!(GetFlags() & SalLayoutFlags::BiDiRtl) && rtl && !copy->empty()
-        && copy->back().IsSpacing())
-    {
-        return nullptr;
-    }
     return copy.release();
 }