tdf#105913: Fix applying DX array to ligatures

Accumulate the width difference for all characters belonging to the
current glyph, not just the first character.

A regression introduced in:

commit 15f6a97d9f23124c19471b9d8dd38f14f53829b3
Author: Khaled Hosny <khaledhosny@eglug.org>
Date:   Sun Sep 11 10:25:46 2016 +0200

Fix applying DX adjustments in CommonSalLayout

By overriding GetCharWidths() and ApplyDXArray() with a simpler and
saner implementation.

This fixes rendering of Awami Nastaliq, as well as subtending marks in
Amiri and potentially other bugs.

Breaks Kashida justification, will need to rewrite that one as well.

Change-Id: I843679e937f2881e77df61f5cbd9516b6df1b3b6

Change-Id: I419a620f94095238cb87d628032c9fd4be678c1a
Reviewed-on: https://gerrit.libreoffice.org/53550
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Khaled Hosny <khaledhosny@eglug.org>

1 files changed, 55 insertions, 31 deletions

diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx
index c430bda597fb..515cef284182 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -907,52 +907,76 @@ void CommonSalLayout::ApplyDXArray(ImplLayoutArgs& rArgs)
     size_t i = 0;
     while (i < m_GlyphItems.size())
     {
+        // Accumulate the width difference for all characters corresponding to
+        // this glyph.
         int nCharPos = m_GlyphItems[i].mnCharPos - mnMinCharPos;
-        DeviceCoordinate nDiff = pNewCharWidths[nCharPos] - pOldCharWidths[nCharPos];
+        DeviceCoordinate nDiff = 0;
+        for (int j = 0; j < m_GlyphItems[i].mnCharCount; j++)
+            nDiff += pNewCharWidths[nCharPos + j] - pOldCharWidths[nCharPos + j];
 
-        // Adjust the width of the first glyph in the cluster.
-        m_GlyphItems[i].mnNewWidth += nDiff;
+        if (!m_GlyphItems[i].IsRTLGlyph())
+        {
+            // Adjust the width and position of the first (leftmost) glyph in
+            // the cluster.
+            m_GlyphItems[i].mnNewWidth += nDiff;
+            m_GlyphItems[i].maLinearPos.AdjustX(nDelta);
 
-        // Apply the X position of all glyphs in the cluster.
-        size_t j = i;
-        while (j < m_GlyphItems.size())
+            // Adjust the position of the rest of the glyphs in the cluster.
+            while (++i < m_GlyphItems.size())
+            {
+                if (!m_GlyphItems[i].IsInCluster())
+                    break;
+                m_GlyphItems[i].maLinearPos.AdjustX(nDelta);
+            }
+        }
+        else if (m_GlyphItems[i].IsInCluster())
         {
-            if (m_GlyphItems[j].mnCharPos != m_GlyphItems[i].mnCharPos)
-                break;
-            m_GlyphItems[j].maLinearPos.AdjustX(nDelta );
-            // For RTL, put all DX adjustment space to the left of the glyph.
-            if (m_GlyphItems[i].IsRTLGlyph())
-                m_GlyphItems[j].maLinearPos.AdjustX(nDiff );
-            ++j;
+            // RTL glyph in the middle of the cluster, will be handled in the
+            // loop below.
+            i++;
         }
-
-        // Id this glyph is Kashida-justifiable, then mark this as a Kashida
-        // position. Since this must be a RTL glyph, we mark the last glyph in
-        // the cluster not the first as this would be the base glyph.
-        // nDiff > 1 to ignore rounding errors.
-        if (bKashidaJustify && m_GlyphItems[i].AllowKashida() && nDiff > 1)
+        else
         {
-            pKashidas[j - 1] = nDiff;
-            // Move any non-spacing marks attached to this cluster as well.
-            // Looping backward because this is RTL glyph.
-            if (i > 0)
+            // Adjust the width and position of the first (rightmost) glyph in
+            // the cluster.
+            // For RTL, we put all the adjustment to the left of the glyph.
+            m_GlyphItems[i].mnNewWidth += nDiff;
+            m_GlyphItems[i].maLinearPos.AdjustX(nDelta + nDiff);
+
+            // Adjust the X position of all glyphs in the cluster.
+            size_t j = i;
+            while (j-- > 0)
+            {
+                if (!m_GlyphItems[j].IsInCluster())
+                    break;
+                m_GlyphItems[j].maLinearPos.AdjustX(nDelta + nDiff);
+            }
+
+            // If this glyph is Kashida-justifiable, then mark this as a
+            // Kashida position. Since this must be a RTL glyph, we mark the
+            // last glyph in the cluster not the first as this would be the
+            // base glyph.
+            if (bKashidaJustify && m_GlyphItems[i].AllowKashida() &&
+                nDiff > m_GlyphItems[i].mnCharCount) // Rounding errors, 1 pixel per character!
             {
-                auto pGlyph = m_GlyphItems.begin() + i - 1;
-                while (pGlyph != m_GlyphItems.begin() && pGlyph->IsDiacritic())
+                pKashidas[i] = nDiff;
+                // Move any non-spacing marks attached to this cluster as well.
+                // Looping backward because this is RTL glyph.
+                while (j > 0)
                 {
-                    pGlyph->maLinearPos.AdjustX(nDiff );
-                    --pGlyph;
+                    if (!m_GlyphItems[j].IsDiacritic())
+                        break;
+                    m_GlyphItems[j--].maLinearPos.AdjustX(nDiff);
                 }
             }
+            i++;
         }
 
-
         // Increment the delta, the loop above makes sure we do so only once
         // for every character (cluster) not for every glyph (otherwise we
         // would apply it multiple times for each glyphs belonging to the same
         // character which is wrong since DX adjustments are character based).
         nDelta += nDiff;
-        i = j;
     }
 
     // Insert Kashida glyphs.
@@ -1005,14 +1029,14 @@ bool CommonSalLayout::IsKashidaPosValid(int nCharPos) const
     {
         if (pIter->mnCharPos == nCharPos)
         {
-            // The position is the first glyphs, this would happen if we
+            // The position is the first glyph, this would happen if we
             // changed the text styling in the middle of a word. Since we don’t
             // do ligatures across layout engine instances, this can’t be a
             // ligature so it should be fine.
             if (pIter == m_GlyphItems.begin())
                 return true;
 
-            // If the character was not supported by this layout, return false
+            // If the character is not supported by this layout, return false
             // so that fallback layouts would be checked for it.
             if (pIter->maGlyphId == 0)
                 break;