Merge pull request #7517 from hodbauer/fix-rtl-label

fixed reversing non alphabetic characters on RTL labels
CesiumGS · Feb 19, 2019 · 0695104 · 0695104
2 parents c648159 + eedb7b1
commit 0695104
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 4 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -11,6 +11,7 @@ Change Log
 
 ##### Fixes :wrench:
 * Fixed an issue where models would cause a crash on load if some primitives were Draco encoded and others were not. [#7383](https://github.com/AnalyticalGraphicsInc/cesium/issues/7383)
+* Fixed an issue where RTL labels not reversing correctly non alphabetic characters [#7501](https://github.com/AnalyticalGraphicsInc/cesium/pull/7501)
 * Fixed Node.js support for the `Resource` class and any functionality using it internally.
 * Fixed an issue where some ground polygons crossing the Prime Meridian would have incorrect bounding rectangles. [#7533](https://github.com/AnalyticalGraphicsInc/cesium/pull/7533)
 * Fixed an issue where polygons on terrain using rhumb lines where being rendered incorrectly. [#7538](https://github.com/AnalyticalGraphicsInc/cesium/pulls/7538)
@@ -50,6 +51,7 @@ Change Log
 * Added support for glTF models with WebP textures using the `EXT_texture_webp` extension. [#7486](https://github.com/AnalyticalGraphicsInc/cesium/pull/7486)
 
 ##### Fixes :wrench:
+* Fixed an issue where RTL labels not reversing correctly non alphabetic characters [#7501](https://github.com/AnalyticalGraphicsInc/cesium/pull/7501)
 * Fixed 3D Tiles performance regression. [#7482](https://github.com/AnalyticalGraphicsInc/cesium/pull/7482)
 * Fixed an issue where classification primitives with the `CESIUM_3D_TILE` classification type would render on terrain. [#7422](https://github.com/AnalyticalGraphicsInc/cesium/pull/7422)
 * Fixed an issue where 3D Tiles would show through the globe. [#7422](https://github.com/AnalyticalGraphicsInc/cesium/pull/7422)

diff --git a/Source/Scene/Label.js b/Source/Scene/Label.js
@@ -1340,32 +1340,37 @@ define([
         var result = '';
         for (var i = 0; i < texts.length; i++) {
             var text = texts[i];
+            // first character of the line is a RTL character, so need to manage different cases
             var rtlDir = rtlChars.test(text.charAt(0));
             var parsedText = convertTextToTypes(text, rtlChars);
 
             var splicePointer = 0;
             var line = '';
             for (var wordIndex = 0; wordIndex < parsedText.length; ++wordIndex) {
                 var subText = parsedText[wordIndex];
-                var reverse = subText.Type === textTypes.BRACKETS ? reverseBrackets(subText.Word) : subText.Word;
+                var reverse = subText.Type === textTypes.BRACKETS ? reverseBrackets(subText.Word) : reverseWord(subText.Word);
                 if (rtlDir) {
                     if (subText.Type === textTypes.RTL) {
-                        line = reverseWord(subText.Word) + line;
+                        line = reverse + line;
                         splicePointer = 0;
                     }
                     else if (subText.Type === textTypes.LTR) {
                         line = spliceWord(line, splicePointer, subText.Word);
                         splicePointer += subText.Word.length;
                     }
                     else if (subText.Type === textTypes.WEAK || subText.Type === textTypes.BRACKETS) {
+                        // current word is weak, last one was bracket
                         if (subText.Type === textTypes.WEAK && parsedText[wordIndex - 1].Type === textTypes.BRACKETS) {
-                            line = reverseWord(subText.Word) + line;
+                            line = reverse + line;
                         }
+                        // current word is weak or bracket, last one was rtl
                         else if (parsedText[wordIndex - 1].Type === textTypes.RTL) {
                             line = reverse + line;
                             splicePointer = 0;
                         }
+                        // current word is weak or bracket, there is at least one more word
                         else if (parsedText.length > wordIndex + 1) {
+                            // next word is rtl
                             if (parsedText[wordIndex + 1].Type === textTypes.RTL) {
                                 line = reverse + line;
                                 splicePointer = 0;
@@ -1375,22 +1380,30 @@ define([
                                 splicePointer += subText.Word.length;
                             }
                         }
+                        // current word is weak or bracket, and it the last in this line
                         else {
                             line = spliceWord(line, 0, reverse);
                         }
                     }
                 }
+                // ltr line, rtl word
                 else if (subText.Type === textTypes.RTL) {
-                    line = spliceWord(line, splicePointer, reverseWord(subText.Word));
+                    line = spliceWord(line, splicePointer, reverse);
                 }
+                // ltr line, ltr word
                 else if (subText.Type === textTypes.LTR) {
                     line += subText.Word;
                     splicePointer = line.length;
                 }
+                // ltr line, weak or bracket word
                 else if (subText.Type === textTypes.WEAK || subText.Type === textTypes.BRACKETS) {
+                    // not first word in line
                     if (wordIndex > 0) {
+                        // last word was rtl
                         if (parsedText[wordIndex - 1].Type === textTypes.RTL) {
+                            // there is at least one more word
                             if (parsedText.length > wordIndex + 1) {
+                                // next word is rtl
                                 if (parsedText[wordIndex + 1].Type === textTypes.RTL) {
                                     line = spliceWord(line, splicePointer, reverse);
                                 }

diff --git a/Specs/Scene/LabelCollectionSpec.js b/Specs/Scene/LabelCollectionSpec.js
@@ -1983,6 +1983,17 @@ defineSuite([
             expect(label.text).toEqual(text);
             expect(label._renderedText).toEqual(expectedText);
         });
+
+        it('should reversing correctly non alphabetic characters', function() {
+            var text = 'A אב: ג\nאב: ג';
+            var expectedText = 'A ג :בא\nג :בא';
+            var label = labels.add({
+                text : text
+            });
+
+            expect(label.text).toEqual(text);
+            expect(label._renderedText).toEqual(expectedText);
+        });
     });
 
     it('computes bounding sphere in 3D', function() {