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: خالد حسني <khaled@aliftype.com>

3 files changed, 11 insertions, 20 deletions

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