Skip to content

Commit

Permalink
Fix right-to-left advances from canvas measureText
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
David Quiroz Marin authored and Commit Bot committed Jul 24, 2018
1 parent 36a8495 commit 67493d0
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]) {
Expand Down Expand Up @@ -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 + ")");
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down

0 comments on commit 67493d0

Please sign in to comment.