Harfbuzz-shaper - fix incorrect logClusters being set in HB_OpenTypePosition

After shaping in HB_OpenTypePosition, when we come to calculate the new
logClusters array we have to take into account that the glyphs passed in
are not a 1 to 1 correspondance with the original string, because some
shaping might have already been done.  So we must use the old
logClusters values (stored in tmpLogClusters) to map from the glyphs
passed in back to the original string.

This fixes visual word wrapping problems in thai

Change-Id: I384dfa98f0946e9e074728f89542acb2b6b6bc27
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

2 files changed, 47 insertions, 17 deletions

diff --git a/src/3rdparty/harfbuzz/src/harfbuzz-shaper.cpp b/src/3rdparty/harfbuzz/src/harfbuzz-shaper.cpp
index 48e9064e94..3811ba9625 100644
--- a/src/3rdparty/harfbuzz/src/harfbuzz-shaper.cpp
+++ b/src/3rdparty/harfbuzz/src/harfbuzz-shaper.cpp
@@ -1283,30 +1283,52 @@ HB_Bool HB_OpenTypePosition(HB_ShaperItem *item, int availableGlyphs, HB_Bool do
         glyphs[i] = face->buffer->in_string[i].gindex;
         attributes[i] = face->tmpAttributes[face->buffer->in_string[i].cluster];
         if (i && face->buffer->in_string[i].cluster == face->buffer->in_string[i-1].cluster)
-            attributes[i].clusterStart = false;
+            attributes[i].clusterStart = false; //FIXME - Shouldn't we otherwise set this to true, rather than leaving it?
     }
     item->num_glyphs = face->buffer->in_length;
 
     if (doLogClusters && face->glyphs_substituted) {
         // we can't do this for indic, as we pass the stuf in syllables and it's easier to do it in the shaper.
-        unsigned short *logClusters = item->log_clusters;
-        int clusterStart = 0;
-        int oldCi = 0;
         // #### the reconstruction of the logclusters currently does not work if the original string
         // contains surrogate pairs
+
+        unsigned short *logClusters = item->log_clusters;
+        int clusterStart = 0;
+        int oldIntermediateIndex = 0;
+
+        // This code makes a mapping, logClusters, between the original utf16 string (item->string) and the final
+        // set of glyphs (in_string).
+        //
+        // The code sets the value of logClusters[i] to the index of in_string containing the glyph that will render
+        // item->string[i].
+        //
+        // This is complicated slightly because in_string[i].cluster is an index to an intermediate
+        // array of glyphs - the array that we were passed as the original value of item->glyphs.
+        // To map from the original string to the intermediate array of glyphs we have tmpLogClusters.
+        //
+        // So we have three groups of indexes:
+        //
+        // i,clusterStart = index to in_length, the final set of glyphs.  Also an index to attributes
+        // intermediateIndex = index to the glyphs originally passed in.
+        // stringIndex = index to item->string, the original string.
+
+        int stringIndex = 0;
+        // Iterate over the final set of glyphs...
         for (unsigned int i = 0; i < face->buffer->in_length; ++i) {
-            int ci = face->buffer->in_string[i].cluster;
-            //         DEBUG("   ci[%d] = %d mark=%d, cmb=%d, cs=%d",
-            //                i, ci, glyphAttributes[i].mark, glyphAttributes[i].combiningClass, glyphAttributes[i].clusterStart);
-            if (!attributes[i].mark && attributes[i].clusterStart && ci != oldCi) {
-                for (int j = oldCi; j < ci; j++)
-                    logClusters[j] = clusterStart;
+            // Get the index into the intermediate string for the start of the cluster of chars
+            int intermediateIndex = face->buffer->in_string[i].cluster;
+            if (intermediateIndex != oldIntermediateIndex) {
+                // We have found the end of the cluster of chars in the intermediate string
+                while (face->tmpLogClusters[stringIndex] < intermediateIndex) {
+                    logClusters[stringIndex++] = clusterStart;
+                }
                 clusterStart = i;
-                oldCi = ci;
+                oldIntermediateIndex = intermediateIndex;
             }
         }
-        for (int j = oldCi; j < face->length; j++)
-            logClusters[j] = clusterStart;
+        while (stringIndex < face->length) {
+            logClusters[stringIndex++] = clusterStart;
+        }
     }
 
     // calulate the advances for the shaped glyphs
diff --git a/tests/auto/qtextscriptengine/tst_qtextscriptengine.cpp b/tests/auto/qtextscriptengine/tst_qtextscriptengine.cpp
index 3fec5b66b8..113f110757 100644
--- a/tests/auto/qtextscriptengine/tst_qtextscriptengine.cpp
+++ b/tests/auto/qtextscriptengine/tst_qtextscriptengine.cpp
@@ -1142,7 +1142,7 @@ void tst_QTextScriptEngine::controlInSyllable_qtbug14204()
     e->itemize();
     e->shape(0);
 
-    QVERIFY(e->layoutData->items[0].num_glyphs == 2);
+    QCOMPARE(e->layoutData->items[0].num_glyphs, (unsigned short)2);
     QVERIFY(e->layoutData->glyphLayout.advances_x[1] != 0);
 #else
     QSKIP("X11 specific test", SkipAll);
@@ -1333,8 +1333,9 @@ void tst_QTextScriptEngine::thaiLineSplitting()
 
 void tst_QTextScriptEngine::thaiSaraAM()
 {
-    //U+0E33 (SARA AM, ำ) gets counted as two characters, so make sure it does not throw off the word boundaries
-    QString s(QString::fromUtf8("ฟงคำตดสนคด"));
+    //U+0E33 (SARA AM, ำ) gets counted as two characters, so make sure it does not throw off the word boundaries by throwing off the logClusters
+    QString s(QString::fromUtf8("มาฟังคำตัดสินคดีฆ่ากำนันยูร"));
+    unsigned short clusterNumber[] = {0,1,2,2,3,4,6,7,7,9,10,10,12,13,14,14,16,16,18,19,21,22,22,24,25,25,27};
     QTextLayout layout(s);
     layout.beginLayout();
     layout.createLine();
@@ -1344,8 +1345,15 @@ void tst_QTextScriptEngine::thaiSaraAM()
     e->width(0, s.length()); //force itemize and shape
 
     QCOMPARE(e->layoutData->items.size(), 1);
-    QCOMPARE(e->layoutData->items[0].num_glyphs, ushort(11));  //Note that it's 11, not 10, because the SARA AM counts as two
+    QCOMPARE(e->layoutData->items[0].num_glyphs, ushort(28));
+    QCOMPARE(sizeof(clusterNumber) / sizeof(unsigned short), (size_t)s.size());
 
+    for (int i = 0 ; i < e->layoutData->items[0].num_glyphs; i++)
+        QCOMPARE((bool)e->layoutData->glyphLayout.attributes[i].dontPrint, 0);
+
+    for (int i = 0; i < s.length(); i++)
+        QCOMPARE(e->layoutData->logClustersPtr[i], clusterNumber[i]);
 }
+
 QTEST_MAIN(tst_QTextScriptEngine)
 #include "tst_qtextscriptengine.moc"