Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1211)

Unified Diff: ui/gfx/render_text_harfbuzz.cc

Issue 924773002: Fix multiline behaviors for RTL text. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/gfx/render_text_harfbuzz.cc
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index e4469a6fdfa2500f117317a0b22e84bfac6deb4c..a480a7a7b57746f6d12cf84cb9da290fd90a3ab7 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -220,6 +220,7 @@ class HarfBuzzLineBreaker {
float min_height,
bool multiline,
const base::string16& text,
+ const std::vector<int32_t>& logical_to_visual,
const BreakList<size_t>* words,
const ScopedVector<internal::TextRunHarfBuzz>& runs)
: max_width_((max_width == 0) ? SK_ScalarMax : SkIntToScalar(max_width)),
@@ -227,6 +228,7 @@ class HarfBuzzLineBreaker {
min_height_(min_height),
multiline_(multiline),
text_(text),
+ logical_to_visual_(logical_to_visual),
words_(words),
runs_(runs),
text_x_(0),
@@ -373,6 +375,11 @@ class HarfBuzzLineBreaker {
void AdvanceLine() {
if (!lines_.empty()) {
internal::Line* line = &lines_.back();
+ std::sort(line->segments.begin(), line->segments.end(),
+ [this](const internal::LineSegment& s1,
+ const internal::LineSegment& s2) -> bool {
+ return logical_to_visual_[s1.run] < logical_to_visual_[s2.run];
+ });
line->size.set_height(std::max(min_height_, max_descent_ + max_ascent_));
line->baseline =
std::max(min_baseline_, SkScalarRoundToInt(max_ascent_));
@@ -434,6 +441,7 @@ class HarfBuzzLineBreaker {
const float min_height_;
const bool multiline_;
const base::string16& text_;
+ const std::vector<int32_t>& logical_to_visual_;
const BreakList<size_t>* const words_;
const ScopedVector<internal::TextRunHarfBuzz>& runs_;
@@ -923,7 +931,8 @@ void RenderTextHarfBuzz::EnsureLayout() {
HarfBuzzLineBreaker line_breaker(
display_rect().width(), font_list().GetBaseline(),
std::max(font_list().GetHeight(), min_line_height()), multiline(),
- GetLayoutText(), multiline() ? &GetLineBreaks() : nullptr, runs_);
+ GetLayoutText(), logical_to_visual_,
+ multiline() ? &GetLineBreaks() : nullptr, runs_);
// TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
tracked_objects::ScopedTracker tracking_profile3(
@@ -931,7 +940,7 @@ void RenderTextHarfBuzz::EnsureLayout() {
"431326 RenderTextHarfBuzz::EnsureLayout3"));
for (size_t i = 0; i < runs_.size(); ++i)
- line_breaker.AddRun(visual_to_logical_[i]);
+ line_breaker.AddRun(i);
std::vector<internal::Line> lines;
line_breaker.Finalize(&lines, &total_size_);
set_lines(&lines);
@@ -939,13 +948,18 @@ void RenderTextHarfBuzz::EnsureLayout() {
}
void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
+ internal::SkiaTextRenderer renderer(canvas);
+ DrawVisualTextInternal(&renderer);
+}
+
+void RenderTextHarfBuzz::DrawVisualTextInternal(
+ internal::SkiaTextRenderer* renderer) {
DCHECK(!needs_layout_);
if (lines().empty())
return;
- internal::SkiaTextRenderer renderer(canvas);
- ApplyFadeEffects(&renderer);
- ApplyTextShadows(&renderer);
+ ApplyFadeEffects(renderer);
+ ApplyTextShadows(renderer);
ApplyCompositionAndSelectionStyles();
for (size_t i = 0; i < lines().size(); ++i) {
@@ -954,10 +968,10 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
SkScalar preceding_segment_widths = 0;
for (const internal::LineSegment& segment : line.segments) {
const internal::TextRunHarfBuzz& run = *runs_[segment.run];
- renderer.SetTypeface(run.skia_face.get());
- renderer.SetTextSize(SkIntToScalar(run.font_size));
- renderer.SetFontRenderParams(run.render_params,
- background_is_transparent());
+ renderer->SetTypeface(run.skia_face.get());
+ renderer->SetTextSize(SkIntToScalar(run.font_size));
+ renderer->SetFontRenderParams(run.render_params,
+ background_is_transparent());
Range glyphs_range = run.CharRangeToGlyphRange(segment.char_range);
scoped_ptr<SkPoint[]> positions(new SkPoint[glyphs_range.length()]);
SkScalar offset_x =
@@ -984,8 +998,8 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
if (colored_glyphs.is_empty())
continue;
- renderer.SetForegroundColor(it->second);
- renderer.DrawPosText(
+ renderer->SetForegroundColor(it->second);
+ renderer->DrawPosText(
&positions[colored_glyphs.start() - glyphs_range.start()],
&run.glyphs[colored_glyphs.start()], colored_glyphs.length());
int start_x = SkScalarRoundToInt(
@@ -995,15 +1009,15 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
? (SkFloatToScalar(segment.width) + preceding_segment_widths +
SkIntToScalar(origin.x()))
: positions[colored_glyphs.end() - glyphs_range.start()].x());
- renderer.DrawDecorations(start_x, origin.y(), end_x - start_x,
- run.underline, run.strike,
- run.diagonal_strike);
+ renderer->DrawDecorations(start_x, origin.y(), end_x - start_x,
+ run.underline, run.strike,
+ run.diagonal_strike);
}
preceding_segment_widths += SkFloatToScalar(segment.width);
}
}
- renderer.EndDiagonalStrike();
+ renderer->EndDiagonalStrike();
UndoCompositionAndSelectionStyles();
}
@@ -1318,8 +1332,8 @@ bool RenderTextHarfBuzz::ShapeRunWithFont(internal::TextRunHarfBuzz* run,
const SkScalar y_offset = SkFixedToScalar(hb_positions[i].y_offset);
run->positions[i].set(run->width + x_offset, -y_offset);
run->width += (glyph_width_for_test_ > 0)
- ? SkIntToScalar(glyph_width_for_test_)
- : SkFixedToScalar(hb_positions[i].x_advance);
+ ? glyph_width_for_test_
+ : SkFixedToFloat(hb_positions[i].x_advance);
// Round run widths if subpixel positioning is off to match native behavior.
if (!run->render_params.subpixel_positioning)
run->width = std::floor(run->width + 0.5f);

Powered by Google App Engine
This is Rietveld 408576698