diff options
author | Luboš Luňák <l.lunak@collabora.com> | 2022-05-25 18:29:42 +0200 |
---|---|---|
committer | Luboš Luňák <l.lunak@collabora.com> | 2022-05-26 07:08:47 +0200 |
commit | 755ed6f5f062afecb157f06e6a3132873ca7b23f (patch) | |
tree | b9fe329a59d6b282a10e2b5d866777f57254a63e | |
parent | 13762c1ec222b6235bbef39eed9bb1037e082a1e (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.cxx | 45 |
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) |