vcl/source/gdi/impglyphitem.cxx | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)
New commits: commit 091e615912e123f9d714952b61e2d9b8ae48a043 Author: Luboš Luňák <[email protected]> AuthorDate: Mon May 23 15:28:05 2022 +0200 Commit: Luboš Luňák <[email protected]> CommitDate: Mon May 23 21:05:12 2022 +0200 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 <[email protected]> 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(); }
