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>
This commit is contained in:
Luboš Luňák
2022-04-27 09:36:45 +02:00
parent e3c0a4eb7d
commit 2b418c16cd

View File

@@ -95,15 +95,33 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
{ {
std::unique_ptr<SalLayoutGlyphsImpl> copy(new SalLayoutGlyphsImpl(*GetFont())); std::unique_ptr<SalLayoutGlyphsImpl> copy(new SalLayoutGlyphsImpl(*GetFont()));
copy->SetFlags(GetFlags()); copy->SetFlags(GetFlags());
if (empty())
return copy.release();
copy->reserve(std::min<size_t>(size(), length)); copy->reserve(std::min<size_t>(size(), length));
// Skip glyphs that are in the string before the given index (glyphs are sorted by charPos()). sal_Int32 beginPos = index;
const_iterator pos = std::partition_point( sal_Int32 endPos = index + length;
begin(), end(), [index](const GlyphItem& it) { return it.charPos() < index; }); 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()) if (pos == end())
return nullptr; return nullptr;
// Require a start at the exact position given, otherwise bail out. // Require a start at the exact position given, otherwise bail out.
// TODO: This bails out also for RTL text. if (pos->charPos() != beginPos)
if (pos->charPos() != index)
return nullptr; return nullptr;
// Don't create a subset if it's not safe to break at the beginning or end of the sequence // 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). // (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(). // that's how it's computed in GenericSalLayout::LayoutText().
DevicePoint zeroPoint = pos->linearPos() - DevicePoint(pos->xOffset(), pos->yOffset()); DevicePoint zeroPoint = pos->linearPos() - DevicePoint(pos->xOffset(), pos->yOffset());
// Add and adjust all glyphs until the given length. // Add and adjust all glyphs until the given length.
// The check is written as 'charPos + charCount <= index + length' rather than // The check is written as 'charPos + charCount <= endPos' rather than 'charPos < endPos'
// 'charPos < index + length' to make sure we include complete glyphs. If a glyph is composed // (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 // 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 // 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. // the loop and the later 'pos->charPos() != endPos' check will fail and bail out.
// CppunitTest_sw_layoutwriter's testCombiningCharacterCursorPosition would fail without this. // 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. return nullptr; // Don't mix RTL and non-RTL runs.
copy->push_back(*pos); copy->push_back(*pos);
copy->back().setLinearPos(copy->back().linearPos() - zeroPoint); copy->back().setLinearPos(copy->back().linearPos() - zeroPoint);
@@ -129,7 +149,9 @@ SalLayoutGlyphsImpl* SalLayoutGlyphsImpl::cloneCharRange(sal_Int32 index, sal_In
} }
if (pos != end()) 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; return nullptr;
if (pos->IsUnsafeToBreak() || (pos->IsInCluster() && !pos->IsClusterStart())) if (pos->IsUnsafeToBreak() || (pos->IsInCluster() && !pos->IsClusterStart()))
return nullptr; return nullptr;