summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuboš Luňák <l.lunak@collabora.com>2022-05-25 18:29:42 +0200
committerLuboš Luňák <l.lunak@collabora.com>2022-05-26 07:08:47 +0200
commit755ed6f5f062afecb157f06e6a3132873ca7b23f (patch)
treeb9fe329a59d6b282a10e2b5d866777f57254a63e
parent13762c1ec222b6235bbef39eed9bb1037e082a1e (diff)
do not use glyph subsets if LTR/RTL does not match (tdf#149264)
Writer apparently sometimes asks to lay out RTL text as LTR, for whatever reason, and Harfbuzz in that case does not give consistent results usable for creating subsets of glyphs (or maybe HB_GLYPH_FLAG_UNSAFE_TO_BREAK is not reliable in that case). I don't quite understand what the problem here is, but avoid it by checking whether the text is LTR or RTL and simply do not try to create a subset from the glyphs if the text does not match the way it was asked to be laid out. I don't even know if this is correct handling or just a workaround for something. Change-Id: Ic77db04f529c9ca2c194893a2127e85953accf32 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134950 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
-rw-r--r--vcl/source/gdi/impglyphitem.cxx45
1 files changed, 44 insertions, 1 deletions
diff --git a/vcl/source/gdi/impglyphitem.cxx b/vcl/source/gdi/impglyphitem.cxx
index bdedb263778e..39c01699ea4e 100644
--- a/vcl/source/gdi/impglyphitem.cxx
+++ b/vcl/source/gdi/impglyphitem.cxx
@@ -26,6 +26,9 @@
#include <TextLayoutCache.hxx>
#include <officecfg/Office/Common.hxx>
+#include <unicode/ubidi.h>
+#include <unicode/uchar.h>
+
// These need being explicit because of SalLayoutGlyphsImpl being private in vcl.
SalLayoutGlyphs::SalLayoutGlyphs() {}
@@ -236,16 +239,56 @@ SalLayoutGlyphsCache* SalLayoutGlyphsCache::self()
return cache.get();
}
+static UBiDiDirection getBiDiDirection(const OUString& text, sal_Int32 index, sal_Int32 len)
+{
+ // Return whether all character are LTR, RTL, neutral or whether it's mixed.
+ // This is sort of ubidi_getBaseDirection() and ubidi_getDirection(),
+ // but it's meant to be fast but also check all characters.
+ sal_Int32 end = index + len;
+ UBiDiDirection direction = UBIDI_NEUTRAL;
+ while (index < end)
+ {
+ switch (u_charDirection(text.iterateCodePoints(&index)))
+ {
+ // Only characters with strong direction.
+ case U_LEFT_TO_RIGHT:
+ if (direction == UBIDI_RTL)
+ return UBIDI_MIXED;
+ direction = UBIDI_LTR;
+ break;
+ case U_RIGHT_TO_LEFT:
+ case U_RIGHT_TO_LEFT_ARABIC:
+ if (direction == UBIDI_LTR)
+ return UBIDI_MIXED;
+ direction = UBIDI_RTL;
+ break;
+ default:
+ break;
+ }
+ }
+ return direction;
+}
+
static SalLayoutGlyphs makeGlyphsSubset(const SalLayoutGlyphs& source,
- const OutputDevice* outputDevice, std::u16string_view text,
+ const OutputDevice* outputDevice, const OUString& text,
sal_Int32 index, sal_Int32 len)
{
+ // tdf#149264: We need to check if the text is LTR, RTL or mixed. Apparently
+ // harfbuzz doesn't give reproducible results (or possibly HB_GLYPH_FLAG_UNSAFE_TO_BREAK
+ // is not reliable?) when asked to lay out RTL text as LTR. So require that the whole
+ // subset ir either LTR or RTL.
+ UBiDiDirection direction = getBiDiDirection(text, index, len);
+ if (direction == UBIDI_MIXED)
+ return SalLayoutGlyphs();
SalLayoutGlyphs ret;
for (int level = 0;; ++level)
{
const SalLayoutGlyphsImpl* sourceLevel = source.Impl(level);
if (sourceLevel == nullptr)
break;
+ bool sourceRtl = bool(sourceLevel->GetFlags() & SalLayoutFlags::BiDiRtl);
+ if ((direction == UBIDI_LTR && sourceRtl) || (direction == UBIDI_RTL && !sourceRtl))
+ return SalLayoutGlyphs();
SalLayoutGlyphsImpl* cloned = sourceLevel->cloneCharRange(index, len);
// If the glyphs range cannot be cloned, bail out.
if (cloned == nullptr)