Skip to content

Commit

Permalink
[Merge M80] Revert "Add BreakLists for layout text in RenderText"
Browse files Browse the repository at this point in the history 
This reverts commit ee7c76a.

Bug: 1033194
Change-Id: Ie0dd12fd86a5266ca8d4671c39000a114431ea67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989063
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3987@{#459}
Cr-Branched-From: c4e8da9-refs/heads/master@{#722274}
  • Loading branch information
@bergeret
bergeret authored and Commit Bot committed Jan 10, 2020
1 parent 714bcd1 commit 33ec597
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 254 deletions.
116 changes: 31 additions & 85 deletions ui/gfx/render_text.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -897,35 +897,34 @@ void RenderText::SetCompositionRange(const Range& composition_range) {
Range(0, text_.length()).Contains(composition_range));
composition_range_.set_end(composition_range.end());
composition_range_.set_start(composition_range.start());
// TODO(oshima|msw): Altering composition underlines shouldn't
// require layout changes. It's currently necessary because
// RenderTextHarfBuzz paints text decorations by run, and
// RenderTextMac applies all styles during layout.
OnLayoutTextAttributeChanged(false);
}

void RenderText::SetColor(SkColor value) {
colors_.SetValue(value);
OnTextColorChanged();
OnLayoutTextAttributeChanged(false);
}

void RenderText::ApplyColor(SkColor value, const Range& range) {
colors_.ApplyValue(value, range);
OnTextColorChanged();
OnLayoutTextAttributeChanged(false);
}

void RenderText::SetBaselineStyle(BaselineStyle value) {
baselines_.SetValue(value);
OnLayoutTextAttributeChanged(false);
}

void RenderText::ApplyBaselineStyle(BaselineStyle value, const Range& range) {
baselines_.ApplyValue(value, range);
OnLayoutTextAttributeChanged(false);
}

void RenderText::ApplyFontSizeOverride(int font_size_override,
const Range& range) {
font_size_overrides_.ApplyValue(font_size_override, range);
OnLayoutTextAttributeChanged(false);
}

void RenderText::SetStyle(TextStyle style, bool value) {
Expand Down Expand Up @@ -1292,6 +1291,7 @@ RenderText::RenderText()
font_size_overrides_(0),
weights_(Font::Weight::NORMAL),
styles_(TEXT_STYLE_COUNT),
composition_and_selection_styles_applied_(false),
obscured_(false),
obscured_reveal_index_(-1),
truncate_length_(0),
Expand All @@ -1312,13 +1312,6 @@ internal::StyleIterator RenderText::GetTextStyleIterator() const {
&weights_, &styles_);
}

internal::StyleIterator RenderText::GetLayoutTextStyleIterator() {
EnsuresLayoutTextAttributeUpdated();
return internal::StyleIterator(&layout_colors_, &layout_baselines_,
&layout_font_size_overrides_, &layout_weights_,
&layout_styles_);
}

bool RenderText::IsHomogeneous() const {
if (colors().breaks().size() > 1 || baselines().breaks().size() > 1 ||
font_size_overrides().breaks().size() > 1 ||
Expand Down Expand Up @@ -1392,10 +1385,6 @@ void RenderText::SetSelectionModel(const SelectionModel& model) {
void RenderText::OnTextColorChanged() {
}

void RenderText::OnLayoutTextAttributeChanged(bool text_changed) {
layout_text_attributes_up_to_date_ = false;
}

void RenderText::EnsureLayoutTextUpdated() {
if (layout_text_up_to_date_)
return;
Expand Down Expand Up @@ -1440,75 +1429,6 @@ void RenderText::EnsureLayoutTextUpdated() {
layout_text_up_to_date_ = true;
}

void RenderText::EnsuresLayoutTextAttributeUpdated() {
if (layout_text_attributes_up_to_date_)
return;

// Reset the previous layout text attributes.
size_t max_length = layout_text_.length();
layout_colors_.SetMax(max_length);
layout_baselines_.SetMax(max_length);
layout_font_size_overrides_.SetMax(max_length);
layout_weights_.SetMax(max_length);
layout_styles_.resize(TEXT_STYLE_COUNT);
for (auto& layout_style : layout_styles_)
layout_style.SetMax(max_length);

// Create an iterator to ensure layout BreakLists don't break graphemes.
base::i18n::BreakIterator grapheme_iter(
text_, base::i18n::BreakIterator::BREAK_CHARACTER);
bool success = grapheme_iter.Init();
DCHECK(success);

// Iterates through codepoints in both strings. Both string have the same
// amount of codepoints but some codepoints may differ (e.g. obscured or
// replaced) and may differ in length (e.g. surrogate pair).
base::i18n::UTF16CharIterator text_iter(&text_);
base::i18n::UTF16CharIterator layout_text_iter(&layout_text_);
internal::StyleIterator styles = GetTextStyleIterator();
size_t current_grapheme_start_position = 0;
while (!text_iter.end() && !layout_text_iter.end()) {
size_t current_text_position = text_iter.array_pos();
size_t current_layout_text_position = layout_text_iter.array_pos();
if (grapheme_iter.IsGraphemeBoundary(current_text_position))
current_grapheme_start_position = current_text_position;

// Move text iterators to their next character.
text_iter.Advance();
layout_text_iter.Advance();

// Apply the style at current grapheme position to the layout text.
styles.IncrementToPosition(current_grapheme_start_position);

Range range(current_layout_text_position, layout_text_iter.array_pos());
layout_colors_.ApplyValue(styles.color(), range);
layout_baselines_.ApplyValue(styles.baseline(), range);
layout_font_size_overrides_.ApplyValue(styles.font_size_override(), range);
layout_weights_.ApplyValue(styles.weight(), range);
for (size_t i = 0; i < TEXT_STYLE_COUNT; ++i) {
layout_styles_[i].ApplyValue(styles.style(static_cast<TextStyle>(i)),
range);
}

// Apply an underline to the composition range in |underlines|.
if (composition_range_.Contains(
gfx::Range(current_grapheme_start_position))) {
layout_styles_[TEXT_STYLE_HEAVY_UNDERLINE].ApplyValue(true, range);
}

// Apply the selected text color to the selection range.
if (!selection().is_empty() &&
selection().Contains(gfx::Range(current_grapheme_start_position))) {
layout_colors_.ApplyValue(selection_color_, range);
}
}

// Wait to reset |layout_text_attributes_up_to_date_| until the end, to ensure
// this function's implementation doesn't indirectly rely on it being up to
// date anywhere.
layout_text_attributes_up_to_date_ = true;
}

const base::string16& RenderText::GetLayoutText() {
EnsureLayoutTextUpdated();
return layout_text_;
Expand Down Expand Up @@ -1594,6 +1514,32 @@ const BreakList<size_t>& RenderText::GetLineBreaks() {
return line_breaks_;
}

void RenderText::ApplyCompositionAndSelectionStyles(const Range& selection) {
// Save the underline and color breaks to undo the temporary styles later.
DCHECK(!composition_and_selection_styles_applied_);
saved_colors_ = colors_;
saved_underlines_ = styles_[TEXT_STYLE_HEAVY_UNDERLINE];

// Apply an underline to the composition range in |underlines|.
if (composition_range_.IsValid() && !composition_range_.is_empty())
styles_[TEXT_STYLE_HEAVY_UNDERLINE].ApplyValue(true, composition_range_);

// Apply the selected text color to the [un-reversed] selection range.
if (!selection.is_empty()) {
const Range range(selection.GetMin(), selection.GetMax());
colors_.ApplyValue(selection_color_, range);
}
composition_and_selection_styles_applied_ = true;
}

void RenderText::UndoCompositionAndSelectionStyles() {
// Restore the underline and color breaks to undo the temporary styles.
DCHECK(composition_and_selection_styles_applied_);
colors_ = saved_colors_;
styles_[TEXT_STYLE_HEAVY_UNDERLINE] = saved_underlines_;
composition_and_selection_styles_applied_ = false;
}

Point RenderText::ToViewPoint(const PointF& point,
LogicalCursorDirection caret_affinity) {
const auto float_eq = [](float a, float b) {
Expand Down
25 changes: 9 additions & 16 deletions ui/gfx/render_text.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,8 +585,6 @@ class GFX_EXPORT RenderText {

// Returns an iterator over the |text_| attributes.
internal::StyleIterator GetTextStyleIterator() const;
// Returns an iterator over the |layout_text_| attributes.
internal::StyleIterator GetLayoutTextStyleIterator();

const BreakList<SkColor>& colors() const { return colors_; }
const BreakList<BaselineStyle>& baselines() const { return baselines_; }
Expand All @@ -597,8 +595,6 @@ class GFX_EXPORT RenderText {
const std::vector<BreakList<bool> >& styles() const { return styles_; }
SkScalar strike_thickness_factor() const { return strike_thickness_factor_; }

const BreakList<SkColor>& layout_colors() const { return layout_colors_; }

// Whether all the BreakLists have only one break.
bool IsHomogeneous() const;

Expand Down Expand Up @@ -679,7 +675,7 @@ class GFX_EXPORT RenderText {
// Notifies that layout text, or attributes that affect the layout text
// shape have changed. |text_changed| is true if the content of the
// |layout_text_| has changed, not just attributes.
virtual void OnLayoutTextAttributeChanged(bool text_changed);
virtual void OnLayoutTextAttributeChanged(bool text_changed) = 0;

// Notifies that attributes that affect the display text shape have changed.
virtual void OnDisplayTextAttributeChanged() = 0;
Expand All @@ -700,6 +696,10 @@ class GFX_EXPORT RenderText {
// Returns display text positions that are suitable for breaking lines.
const BreakList<size_t>& GetLineBreaks();

// Apply (and undo) temporary composition underlines and selection colors.
void ApplyCompositionAndSelectionStyles(const Range& selection);
void UndoCompositionAndSelectionStyles();

// Convert points from the text space to the view space. Handles the display
// area, display offset, application LTR/RTL mode and multiline.
Point ToViewPoint(const PointF& point, LogicalCursorDirection caret_affinity);
Expand Down Expand Up @@ -775,9 +775,6 @@ class GFX_EXPORT RenderText {
// Computes the |layout_text_| by rewriting it from |text_|, if needed.
void EnsureLayoutTextUpdated();

// Computes the layout break lists, if needed.
void EnsuresLayoutTextAttributeUpdated();

// Elides |text| as needed to fit in the |available_width| using |behavior|.
// |text_width| is the pre-calculated width of the text shaped by this render
// text, or pass 0 if the width is unknown.
Expand Down Expand Up @@ -882,11 +879,10 @@ class GFX_EXPORT RenderText {
BreakList<Font::Weight> weights_;
std::vector<BreakList<bool> > styles_;

BreakList<SkColor> layout_colors_;
BreakList<BaselineStyle> layout_baselines_;
BreakList<int> layout_font_size_overrides_;
BreakList<Font::Weight> layout_weights_;
std::vector<BreakList<bool>> layout_styles_;
// Breaks saved without temporary composition and selection styling.
BreakList<SkColor> saved_colors_;
BreakList<bool> saved_underlines_;
bool composition_and_selection_styles_applied_;

// A flag to obscure actual text with asterisks for password fields.
bool obscured_;
Expand Down Expand Up @@ -977,9 +973,6 @@ class GFX_EXPORT RenderText {
// Tell whether or not the |layout_text_| needs an update or is up to date.
bool layout_text_up_to_date_ = false;

// Tell whether or not the layout break lists need an update.
bool layout_text_attributes_up_to_date_ = false;

DISALLOW_COPY_AND_ASSIGN(RenderText);
};

Expand Down
39 changes: 22 additions & 17 deletions ui/gfx/render_text_harfbuzz.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1766,8 +1766,6 @@ bool RenderTextHarfBuzz::IsValidCursorIndex(size_t index) {
}

void RenderTextHarfBuzz::OnLayoutTextAttributeChanged(bool text_changed) {
RenderText::OnLayoutTextAttributeChanged(text_changed);

update_layout_run_list_ = true;
OnDisplayTextAttributeChanged();
}
Expand Down Expand Up @@ -1829,13 +1827,7 @@ void RenderTextHarfBuzz::DrawVisualText(internal::SkiaTextRenderer* renderer,

ApplyFadeEffects(renderer);
ApplyTextShadows(renderer);

// Apply the selected text color to the [un-reversed] selection range.
BreakList<SkColor> colors = layout_colors();
if (!selection.is_empty()) {
colors.ApplyValue(selection_color(),
Range(selection.GetMin(), selection.GetMax()));
}
ApplyCompositionAndSelectionStyles(selection);

internal::TextRunList* run_list = GetRunList();
const base::string16& display_text = GetDisplayText();
Expand Down Expand Up @@ -1867,11 +1859,12 @@ void RenderTextHarfBuzz::DrawVisualText(internal::SkiaTextRenderer* renderer,
SkIntToScalar(origin.x()) + offset_x,
SkIntToScalar(origin.y() + run.font_params.baseline_offset));
}
for (auto it = colors.GetBreak(segment.char_range.start());
it != colors.breaks().end() && it->first < segment.char_range.end();
for (auto it = colors().GetBreak(segment.char_range.start());
it != colors().breaks().end() &&
it->first < segment.char_range.end();
++it) {
const Range intersection =
colors.GetRange(it).Intersect(segment.char_range);
colors().GetRange(it).Intersect(segment.char_range);
const Range colored_glyphs = run.CharRangeToGlyphRange(intersection);
// The range may be empty if a portion of a multi-character grapheme is
// selected, yielding two colors for a single glyph. For now, this just
Expand Down Expand Up @@ -1902,6 +1895,8 @@ void RenderTextHarfBuzz::DrawVisualText(internal::SkiaTextRenderer* renderer,
preceding_segment_widths += SkFloatToScalar(segment.width());
}
}

UndoCompositionAndSelectionStyles();
}

size_t RenderTextHarfBuzz::GetRunContainingCaret(
Expand Down Expand Up @@ -1972,9 +1967,14 @@ void RenderTextHarfBuzz::ItemizeTextToRuns(
return;
}

// Iterator to split ranged styles and baselines. The color attributes don't
// break text runs to keep ligature between graphemes (e.g. Arabic word).
internal::StyleIterator style = GetLayoutTextStyleIterator();
// Temporarily apply composition underlines and selection colors.
ApplyCompositionAndSelectionStyles(focused() ? selection() : Range{});

// Build the run list from the script items and ranged styles and baselines.
DCHECK_LE(text.size(), baselines().max());
for (const BreakList<bool>& style : styles())
DCHECK_LE(text.size(), style.max());
internal::StyleIterator style = GetTextStyleIterator();

// Split the original text by logical runs, then each logical run by common
// script and each sequence at special characters and style boundaries. This
Expand Down Expand Up @@ -2007,8 +2007,10 @@ void RenderTextHarfBuzz::ItemizeTextToRuns(
// Find the break boundary for style. The style won't break a grapheme
// since the style of the first character is applied to the whole
// grapheme.
style.IncrementToPosition(breaking_run_start);
size_t text_style_end = style.GetTextBreakingRange().end();
style.IncrementToPosition(
GivenTextIndexToTextIndex(text, breaking_run_start));
size_t text_style_end =
TextIndexToGivenTextIndex(text, style.GetTextBreakingRange().end());

// Break runs at certain characters that need to be rendered separately
// to prevent an unusual character from forcing a fallback font on the
Expand Down Expand Up @@ -2067,6 +2069,9 @@ void RenderTextHarfBuzz::ItemizeTextToRuns(
TRACE_EVENT_INSTANT1("fonts", "RenderTextHarfBuzz::ItemizeTextToRuns::Runs",
TRACE_EVENT_SCOPE_THREAD, "runs", logging_str);
}

// Undo the temporarily applied composition underlines and selection colors.
UndoCompositionAndSelectionStyles();
}

void RenderTextHarfBuzz::ShapeRuns(
Expand Down
4 changes: 0 additions & 4 deletions ui/gfx/render_text_test_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ class RenderTextTestApi {
render_text_->DrawVisualText(renderer, selection);
}

const base::string16& GetLayoutText() {
return render_text_->GetLayoutText();
}

const BreakList<SkColor>& colors() const { return render_text_->colors(); }

const BreakList<BaselineStyle>& baselines() const {
Expand Down
Loading

0 comments on commit 33ec597

Please sign in to comment.