From 67493d01633025e7436230039e57db20218209b9 Mon Sep 17 00:00:00 2001
From: David Quiroz Marin <davidqu@chromium.org>
Date: Tue, 24 Jul 2018 22:18:19 +0000
Subject: [PATCH] Fix right-to-left advances from canvas measureText

This fixes advances from TextMetrics for Right-to-left text. Currently
it's returning values that are correct when there is just one text_run
(or word), but the values are in the reverse order within each text_run
for when there's more than one run.

Bug: 864193
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I6459f3800a33aedf46692b3931fb5778ee3ce825
Reviewed-on: https://chromium-review.googlesource.com/1144629
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: David Quiroz Marin <davidqu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577707}
---
 .../canvas-textMetrics-advances.html          | 33 +++++++++++++++----
 .../fonts/shaping/shape_result_buffer.cc      | 25 ++++++++------
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-textMetrics-advances.html b/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-textMetrics-advances.html
index 5739540e0bea84..695969e9176695 100644
--- a/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-textMetrics-advances.html
+++ b/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-textMetrics-advances.html
@@ -19,11 +19,11 @@
   { text: "傳統是假的", rtl: false },
   { text: "フェミニズム", rtl: false },
   { text: "ليس", rtl: true },
+  { text: "ليس في اسمنا", rtl: true },
   { text: "\u202EHello", rtl: true },
+  { text: "\u202EHello World", rtl: true },
   // TODO(davidqu): Fix the following edge cases:
-  //{ text: "الله‎", rtl: true }, // Special ligatures.
-  //{ text: "ليس في اسمنا", rtl: true }, // RTL only works for single "words".
-  //{ text: "\u202EHello World", rtl: true },
+  //{ text: "الله", rtl: true }, // Special ligatures.
   //{ text: "🏁", rtl: false }, // One glyph with two "characters".
   //{ text: "एक आम भाषा", rtl: false }, // Special post-modifying characters.
   //{ text: "a\u0301", rtl: true }, // Combining diacritical marks
@@ -37,6 +37,15 @@
   }
 }
 
+function isNonIncreasing(array) {
+  for (var i = 1; i < array.length; i++) {
+    if (array[i] > array[i-1]) {
+      return false;
+    }
+  }
+  return true;
+}
+
 function isNonDecreasing(array) {
   for (var i = 1; i < array.length; i++) {
     if (array[i] < array[i-1]) {
@@ -66,11 +75,23 @@
   });
 }
 
+function testNonIncreasing(ctx) {
+  forEachExample((ex, align) => {
+      if (ex.rtl) {
+        const tm = getTextMetrics(ctx, ex.text, align, ex.rtl ? "rtl" : "ltr");
+        assert_true(isNonIncreasing(tm.advances),
+         "RTL advances must be non-increasing (" + ex.text + ")");
+      }
+  });
+}
+
 function testNonDecreasing(ctx) {
   forEachExample((ex, align) => {
-      const tm = getTextMetrics(ctx, ex.text, align, ex.rtl ? "rtl" : "ltr");
-      assert_true(isNonDecreasing(tm.advances),
-       "Advances must be non-decreasing (" + ex.text + ")");
+      if (ex.ltr) {
+        const tm = getTextMetrics(ctx, ex.text, align, ex.rtl ? "rtl" : "ltr");
+        assert_true(isNonDecreasing(tm.advances),
+         "LTR advances must be non-decreasing (" + ex.text + ")");
+      }
   });
 }
 
diff --git a/third_party/blink/renderer/platform/fonts/shaping/shape_result_buffer.cc b/third_party/blink/renderer/platform/fonts/shaping/shape_result_buffer.cc
index 1800d807bdcc7d..04bb740341b518 100644
--- a/third_party/blink/renderer/platform/fonts/shaping/shape_result_buffer.cc
+++ b/third_party/blink/renderer/platform/fonts/shaping/shape_result_buffer.cc
@@ -138,10 +138,13 @@ void ShapeResultBuffer::AddRunInfoRanges(const ShapeResult::RunInfo& run_info,
   for (const auto& glyph : run_info.glyph_data_)
     character_widths[glyph.character_index] += glyph.advance;
 
+  if (run_info.Rtl())
+    offset += run_info.width_;
+
   for (unsigned character_index = 0; character_index < run_info.num_characters_;
        character_index++) {
     float start = offset;
-    offset += character_widths[character_index];
+    offset += character_widths[character_index] * (run_info.Rtl() ? -1 : 1);
     float end = offset;
 
     // To match getCharacterRange we flip ranges to ensure start <= end.
@@ -158,17 +161,19 @@ Vector<CharacterRange> ShapeResultBuffer::IndividualCharacterRanges(
   Vector<CharacterRange> ranges;
   float current_x = direction == TextDirection::kRtl ? total_width : 0;
   for (const scoped_refptr<const ShapeResult> result : results_) {
-    if (direction == TextDirection::kRtl)
-      current_x -= result->Width();
     unsigned run_count = result->runs_.size();
-    for (unsigned index = 0; index < run_count; index++) {
-      unsigned run_index =
-          direction == TextDirection::kRtl ? run_count - 1 - index : index;
-      AddRunInfoRanges(*result->runs_[run_index], current_x, ranges);
-      current_x += result->runs_[run_index]->width_;
+
+    if (result->Rtl()) {
+      for (int index = run_count - 1; index >= 0; index--) {
+        current_x -= result->runs_[index]->width_;
+        AddRunInfoRanges(*result->runs_[index], current_x, ranges);
+      }
+    } else {
+      for (unsigned index = 0; index < run_count; index++) {
+        AddRunInfoRanges(*result->runs_[index], current_x, ranges);
+        current_x += result->runs_[index]->width_;
+      }
     }
-    if (direction == TextDirection::kRtl)
-      current_x -= result->Width();
   }
   return ranges;
 }