Commit b5675935 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Remove use of GraphemeIterator for IsValidCursorIndex

This CL is removing the use of GraphemeIterator.
The function IsValidCursorIndex(...) is moved from
the RenderTextHarfBuzz to RenderText to be with the
other grapheme-related functions.

The main goal is to get rid of the GraphemeIterator.
There are still some uses to be removed.

Bug: 1025561

Change-Id: Ie8483dbc550aee95eb1cd64ca5d3f3e85855ea09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1951445
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722607}
parent a9ab4e2c
...@@ -827,7 +827,7 @@ bool RenderText::SelectRange(const Range& range) { ...@@ -827,7 +827,7 @@ bool RenderText::SelectRange(const Range& range) {
uint32_t text_length = static_cast<uint32_t>(text().length()); uint32_t text_length = static_cast<uint32_t>(text().length());
Range sel(std::min(range.start(), text_length), Range sel(std::min(range.start(), text_length),
std::min(range.end(), text_length)); std::min(range.end(), text_length));
// Allow selection bounds at valid indicies amid multi-character graphemes. // Allow selection bounds at valid indices amid multi-character graphemes.
if (!IsValidLogicalIndex(sel.start()) || !IsValidLogicalIndex(sel.end())) if (!IsValidLogicalIndex(sel.start()) || !IsValidLogicalIndex(sel.end()))
return false; return false;
LogicalCursorDirection affinity = LogicalCursorDirection affinity =
...@@ -1021,6 +1021,11 @@ bool RenderText::IsValidLogicalIndex(size_t index) const { ...@@ -1021,6 +1021,11 @@ bool RenderText::IsValidLogicalIndex(size_t index) const {
IsValidCodePointIndex(text(), index)); IsValidCodePointIndex(text(), index));
} }
bool RenderText::IsValidCursorIndex(size_t index) {
return index == 0 || index == text().length() ||
(IsValidLogicalIndex(index) && IsGraphemeBoundary(index));
}
Rect RenderText::GetCursorBounds(const SelectionModel& caret, Rect RenderText::GetCursorBounds(const SelectionModel& caret,
bool insert_mode) { bool insert_mode) {
EnsureLayout(); EnsureLayout();
...@@ -1071,28 +1076,59 @@ const Rect& RenderText::GetUpdatedCursorBounds() { ...@@ -1071,28 +1076,59 @@ const Rect& RenderText::GetUpdatedCursorBounds() {
return cursor_bounds_; return cursor_bounds_;
} }
bool RenderText::IsGraphemeBoundary(size_t index) {
if (index >= text().length())
return true;
EnsureLayout();
DCHECK(!text_to_display_indices_.empty());
// The function std::lower_bound(...) finds the first not less than |index|.
auto iter = std::lower_bound(text_to_display_indices_.begin(),
text_to_display_indices_.end(), index,
[](const TextToDisplayIndex& lhs, size_t rhs) {
return lhs.text_index < rhs;
});
return (iter != text_to_display_indices_.end() && iter->text_index == index);
}
size_t RenderText::IndexOfAdjacentGrapheme(size_t index, size_t RenderText::IndexOfAdjacentGrapheme(size_t index,
LogicalCursorDirection direction) { LogicalCursorDirection direction) {
if (index > text().length()) if (text_.empty())
return text().length(); return 0;
if (index > text_.length())
return text_.length();
EnsureLayout(); EnsureLayout();
DCHECK(!text_to_display_indices_.empty());
// The function std::lower_bound(...) finds the first not less than |index|.
auto iter = std::lower_bound(text_to_display_indices_.begin(),
text_to_display_indices_.end(), index,
[](const TextToDisplayIndex& lhs, size_t rhs) {
return lhs.text_index < rhs;
});
if (direction == CURSOR_FORWARD) { if (direction == CURSOR_FORWARD) {
while (index < text().length()) { // |index| is already a grapheme boundary. Move to the next grapheme.
index++; if (iter != text_to_display_indices_.end() && iter->text_index == index)
if (IsValidCursorIndex(index)) ++iter;
return index; if (iter == text_to_display_indices_.end())
} return text_.length();
return text().length(); DCHECK_LT(index, iter->text_index);
} else {
DCHECK_EQ(direction, CURSOR_BACKWARD);
if (iter != text_to_display_indices_.begin())
--iter;
if (iter == text_to_display_indices_.begin())
return 0;
DCHECK_GT(index, iter->text_index);
} }
while (index > 0) { return iter->text_index;
index--;
if (IsValidCursorIndex(index))
return index;
}
return 0;
} }
SelectionModel RenderText::GetSelectionModelForSelectionStart() const { SelectionModel RenderText::GetSelectionModelForSelectionStart() const {
......
...@@ -465,7 +465,7 @@ class GFX_EXPORT RenderText { ...@@ -465,7 +465,7 @@ class GFX_EXPORT RenderText {
// Returns true if the position is a valid logical index into text(), and is // Returns true if the position is a valid logical index into text(), and is
// also a valid grapheme boundary, which may be used as a cursor position. // also a valid grapheme boundary, which may be used as a cursor position.
virtual bool IsValidCursorIndex(size_t index) = 0; bool IsValidCursorIndex(size_t index);
// Returns true if the position is a valid logical index into text(). Indices // Returns true if the position is a valid logical index into text(). Indices
// amid multi-character graphemes are allowed here, unlike IsValidCursorIndex. // amid multi-character graphemes are allowed here, unlike IsValidCursorIndex.
...@@ -489,6 +489,9 @@ class GFX_EXPORT RenderText { ...@@ -489,6 +489,9 @@ class GFX_EXPORT RenderText {
// Subsequent text, cursor, or bounds changes may invalidate returned values. // Subsequent text, cursor, or bounds changes may invalidate returned values.
const Rect& GetUpdatedCursorBounds(); const Rect& GetUpdatedCursorBounds();
// Returns true of the current index is at the start of a grapheme.
bool IsGraphemeBoundary(size_t index);
// Given an |index| in text(), return the next or previous grapheme boundary // Given an |index| in text(), return the next or previous grapheme boundary
// in logical order (i.e. the nearest cursorable index). The return value is // in logical order (i.e. the nearest cursorable index). The return value is
// in the range 0 to text().length() inclusive (the input is clamped if it is // in the range 0 to text().length() inclusive (the input is clamped if it is
......
...@@ -1762,15 +1762,6 @@ SelectionModel RenderTextHarfBuzz::AdjacentLineSelectionModel( ...@@ -1762,15 +1762,6 @@ SelectionModel RenderTextHarfBuzz::AdjacentLineSelectionModel(
return next; return next;
} }
bool RenderTextHarfBuzz::IsValidCursorIndex(size_t index) {
if (index == 0 || index == text().length())
return true;
if (!IsValidLogicalIndex(index))
return false;
base::i18n::BreakIterator* grapheme_iterator = GetGraphemeIterator();
return !grapheme_iterator || grapheme_iterator->IsGraphemeBoundary(index);
}
void RenderTextHarfBuzz::OnLayoutTextAttributeChanged(bool text_changed) { void RenderTextHarfBuzz::OnLayoutTextAttributeChanged(bool text_changed) {
RenderText::OnLayoutTextAttributeChanged(text_changed); RenderText::OnLayoutTextAttributeChanged(text_changed);
......
...@@ -245,7 +245,6 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText { ...@@ -245,7 +245,6 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
SelectionModel AdjacentLineSelectionModel( SelectionModel AdjacentLineSelectionModel(
const SelectionModel& selection, const SelectionModel& selection,
VisualCursorDirection direction) override; VisualCursorDirection direction) override;
bool IsValidCursorIndex(size_t index) override;
void OnLayoutTextAttributeChanged(bool text_changed) override; void OnLayoutTextAttributeChanged(bool text_changed) override;
void OnDisplayTextAttributeChanged() override; void OnDisplayTextAttributeChanged() override;
void EnsureLayout() override; void EnsureLayout() override;
......
...@@ -2877,6 +2877,52 @@ TEST_F(RenderTextTest, MoveCursorLeftRight_MeiryoUILigatures) { ...@@ -2877,6 +2877,52 @@ TEST_F(RenderTextTest, MoveCursorLeftRight_MeiryoUILigatures) {
EXPECT_EQ(6U, render_text->cursor_position()); EXPECT_EQ(6U, render_text->cursor_position());
} }
TEST_F(RenderTextTest, GraphemeBoundaries) {
static const wchar_t text[] =
L"\u0065\u0301" // Letter 'e' U+0065 and acute accent U+0301
L"\u0036\uFE0F\u20E3" // Emoji 'keycap letter 6'
L"\U0001F468\u200D\u2708\uFE0F"; // Emoji 'pilot'.
RenderText* render_text = GetRenderText();
render_text->SetText(WideToUTF16(text));
EXPECT_TRUE(render_text->IsGraphemeBoundary(0));
EXPECT_FALSE(render_text->IsGraphemeBoundary(1));
EXPECT_TRUE(render_text->IsGraphemeBoundary(2));
EXPECT_FALSE(render_text->IsGraphemeBoundary(3));
EXPECT_FALSE(render_text->IsGraphemeBoundary(4));
EXPECT_TRUE(render_text->IsGraphemeBoundary(5));
EXPECT_FALSE(render_text->IsGraphemeBoundary(6));
EXPECT_FALSE(render_text->IsGraphemeBoundary(7));
EXPECT_FALSE(render_text->IsGraphemeBoundary(8));
EXPECT_FALSE(render_text->IsGraphemeBoundary(9));
EXPECT_TRUE(render_text->IsGraphemeBoundary(10));
EXPECT_EQ(2U, render_text->IndexOfAdjacentGrapheme(0, CURSOR_FORWARD));
EXPECT_EQ(2U, render_text->IndexOfAdjacentGrapheme(1, CURSOR_FORWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(2, CURSOR_FORWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(3, CURSOR_FORWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(4, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(5, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(6, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(7, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(8, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(9, CURSOR_FORWARD));
EXPECT_EQ(10U, render_text->IndexOfAdjacentGrapheme(10, CURSOR_FORWARD));
EXPECT_EQ(0U, render_text->IndexOfAdjacentGrapheme(0, CURSOR_BACKWARD));
EXPECT_EQ(0U, render_text->IndexOfAdjacentGrapheme(1, CURSOR_BACKWARD));
EXPECT_EQ(0U, render_text->IndexOfAdjacentGrapheme(2, CURSOR_BACKWARD));
EXPECT_EQ(2U, render_text->IndexOfAdjacentGrapheme(3, CURSOR_BACKWARD));
EXPECT_EQ(2U, render_text->IndexOfAdjacentGrapheme(4, CURSOR_BACKWARD));
EXPECT_EQ(2U, render_text->IndexOfAdjacentGrapheme(5, CURSOR_BACKWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(6, CURSOR_BACKWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(7, CURSOR_BACKWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(8, CURSOR_BACKWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(9, CURSOR_BACKWARD));
EXPECT_EQ(5U, render_text->IndexOfAdjacentGrapheme(10, CURSOR_BACKWARD));
}
TEST_F(RenderTextTest, GraphemePositions) { TEST_F(RenderTextTest, GraphemePositions) {
// LTR कि (DEVANAGARI KA with VOWEL I) (2-char grapheme), LTR abc, and LTR कि. // LTR कि (DEVANAGARI KA with VOWEL I) (2-char grapheme), LTR abc, and LTR कि.
const base::string16 kText1 = WideToUTF16(L"\u0915\u093fabc\u0915\u093f"); const base::string16 kText1 = WideToUTF16(L"\u0915\u093fabc\u0915\u093f");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment