vcl/inc/impglyphitem.hxx | 4 +---
vcl/source/gdi/CommonSalLayout.cxx | 20 +++++++++-----------
vcl/source/gdi/pdfwriter_impl.cxx | 7 -------
vcl/source/gdi/sallayout.cxx | 4 ++--
4 files changed, 12 insertions(+), 23 deletions(-)
New commits:
commit 29de4d6e1afcf2ad79056e4ad4403c3d4bdda4a0
Author: Khaled Hosny <kha...@aliftype.com>
AuthorDate: Mon Oct 3 21:53:38 2022 +0200
Commit: خالد حسني <kha...@aliftype.com>
CommitDate: Tue Oct 4 14:10:53 2022 +0200
tdf#150665: Fix justifying spacing marks
We shouldn’t be concerned whether a character is combining mark or not,
what we really care about is whether the font classifies it as mark
glyph, so check for that. Furthermore, for our purposes a mark glyph
forms a cluster with the preceding glyph, so we can use IN_CLUSTER flag
and drop the IS_DIACRITIC flag. This fixes the big above and a few other
issues I found myself.
Remove also overzealous asserts in pdfwriter, they are long past their
usefulness.
Change-Id: I7c87868a5c5877774bb42343079cad6e89380961
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140925
Tested-by: Jenkins
Reviewed-by: خالد حسني <kha...@aliftype.com>
diff --git a/vcl/inc/impglyphitem.hxx b/vcl/inc/impglyphitem.hxx
index 8bc8f19b9176..9c369c204500 100644
--- a/vcl/inc/impglyphitem.hxx
+++ b/vcl/inc/impglyphitem.hxx
@@ -34,7 +34,6 @@ enum class GlyphItemFlags : sal_uInt16
NONE = 0,
IS_IN_CLUSTER = 0x01,
IS_RTL_GLYPH = 0x02,
- IS_DIACRITIC = 0x04,
IS_VERTICAL = 0x08,
IS_SPACING = 0x10,
IS_DROPPED = 0x40,
@@ -44,7 +43,7 @@ enum class GlyphItemFlags : sal_uInt16
};
namespace o3tl
{
-template <> struct typed_flags<GlyphItemFlags> :
is_typed_flags<GlyphItemFlags, 0x3df>
+template <> struct typed_flags<GlyphItemFlags> :
is_typed_flags<GlyphItemFlags, 0x3db>
{
};
};
@@ -78,7 +77,6 @@ public:
bool IsInCluster() const { return bool(m_nFlags &
GlyphItemFlags::IS_IN_CLUSTER); }
bool IsRTLGlyph() const { return bool(m_nFlags &
GlyphItemFlags::IS_RTL_GLYPH); }
- bool IsDiacritic() const { return bool(m_nFlags &
GlyphItemFlags::IS_DIACRITIC); }
bool IsVertical() const { return bool(m_nFlags &
GlyphItemFlags::IS_VERTICAL); }
bool IsSpacing() const { return bool(m_nFlags &
GlyphItemFlags::IS_SPACING); }
bool IsDropped() const { return bool(m_nFlags &
GlyphItemFlags::IS_DROPPED); }
diff --git a/vcl/source/gdi/CommonSalLayout.cxx
b/vcl/source/gdi/CommonSalLayout.cxx
index 6caeca7c86a1..ca7e1cd42f55 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -527,6 +527,15 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
continue;
}
+ // For our purposes, a mark glyph is part of cluster as well.
+ hb_face_t* pHbFace = hb_font_get_face(pHbFont);
+ if (hb_ot_layout_get_glyph_class(pHbFace, nGlyphIndex) ==
HB_OT_LAYOUT_GLYPH_CLASS_MARK
+ && nCharPos != mnMinCharPos)
+ {
+ bClusterStart = false;
+ bInCluster = true;
+ }
+
GlyphItemFlags nGlyphFlags = GlyphItemFlags::NONE;
if (bRightToLeft)
nGlyphFlags |= GlyphItemFlags::IS_RTL_GLYPH;
@@ -540,9 +549,6 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
sal_UCS4 aChar
=
rArgs.mrStr.iterateCodePoints(&o3tl::temporary(sal_Int32(nCharPos)), 0);
- if (u_getIntPropertyValue(aChar, UCHAR_GENERAL_CATEGORY) ==
U_NON_SPACING_MARK)
- nGlyphFlags |= GlyphItemFlags::IS_DIACRITIC;
-
if (u_isUWhiteSpace(aChar))
nGlyphFlags |= GlyphItemFlags::IS_SPACING;
@@ -792,14 +798,6 @@ void GenericSalLayout::ApplyDXArray(const double*
pDXArray, const sal_Bool* pKas
m_GlyphItems[j].adjustLinearPosX(nDelta + nDiff);
}
- // Move any non-spacing marks to keep attached to this cluster.
- while (j > 0)
- {
- if (!m_GlyphItems[j].IsDiacritic())
- break;
- m_GlyphItems[j--].adjustLinearPosX(nDiff);
- }
-
// This is a Kashida insertion position, mark it. Kashida glyphs
// will be inserted below.
if (pKashidaArray && pKashidaArray[nCharPos])
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx
b/vcl/source/gdi/pdfwriter_impl.cxx
index cf5ee9dca0f7..8fbbea859f2b 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6616,11 +6616,6 @@ void PDFWriterImpl::drawLayout( SalLayout& rLayout,
const OUString& rText, bool
if (pGlyph->IsClusterStart())
bUseActualText = true;
- // Or part of a complex cluster, will be handled by the ActualText
- // of its cluster start.
- if (pGlyph->IsInCluster())
- assert(aCodeUnits.empty());
-
const auto nGlyphId = pGlyph->glyphId();
// A glyph can't have more than one ToUnicode entry, use ActualText
@@ -6638,8 +6633,6 @@ void PDFWriterImpl::drawLayout( SalLayout& rLayout, const
OUString& rText, bool
}
}
- assert(!aCodeUnits.empty() || bUseActualText || pGlyph->IsInCluster());
-
auto nGlyphWidth = pGlyphFont->GetGlyphWidth(nGlyphId,
pGlyph->IsVertical(), false);
sal_uInt8 nMappedGlyph;
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index b960d510ac24..cffa4cd8ecbe 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -315,7 +315,7 @@ void GenericSalLayout::Justify( DeviceCoordinate nNewWidth )
int nMaxGlyphWidth = 0;
for(pGlyphIter = m_GlyphItems.begin(); pGlyphIter != pGlyphIterRight;
++pGlyphIter)
{
- if( !pGlyphIter->IsDiacritic() )
+ if( !pGlyphIter->IsInCluster() )
++nStretchable;
if (nMaxGlyphWidth < pGlyphIter->origWidth())
nMaxGlyphWidth = pGlyphIter->origWidth();
@@ -342,7 +342,7 @@ void GenericSalLayout::Justify( DeviceCoordinate nNewWidth )
pGlyphIter->adjustLinearPosX(nDeltaSum);
// do not stretch non-stretchable glyphs
- if( pGlyphIter->IsDiacritic() || (nStretchable <= 0) )
+ if( pGlyphIter->IsInCluster() || (nStretchable <= 0) )
continue;
// distribute extra space equally to stretchable glyphs
