Don't paint text outside display area

ui/gfx/render_text_pango.cc

Index: ui/gfx/render_text_pango.cc
diff --git a/ui/gfx/render_text_pango.cc b/ui/gfx/render_text_pango.cc
index 07cbfb93cdf5c0e71d09261b618bd009028ba17c..1df03bb2cc410429bfa214ee2d5bd7415eea5f48 100644
--- a/ui/gfx/render_text_pango.cc
+++ b/ui/gfx/render_text_pango.cc
@@ -410,9 +410,12 @@ void RenderTextPango::DrawVisualText(Canvas* canvas) {
 
   internal::StyleIterator style(colors(), styles());
   for (GSList* it = current_line_->runs; it; it = it->next) {
+    // Don't do anything if we're outside the display area

[msw, 2014/09/08 19:10:30]

nit: "Skip painting runs outside the display area."

+    if (SkScalarTruncToInt(x) >= display_rect().right())
+      break;
+
     PangoLayoutRun* run = reinterpret_cast<PangoLayoutRun*>(it->data);
     int glyph_count = run->glyphs->num_glyphs;
-    // TODO(msw): Skip painting runs outside the display rect area, like Win.
     if (glyph_count == 0)
       continue;
 
@@ -446,8 +449,11 @@ void RenderTextPango::DrawVisualText(Canvas* canvas) {
       x += pango_units_to_double(glyph.geometry.width);
 
       ++glyph_index;
-      const size_t glyph_text_index = (glyph_index == glyph_count) ?
-          style_range.end() : GetGlyphTextIndex(run, glyph_index);
+      const size_t glyph_text_index =
+          (glyph_index == glyph_count ||
+           SkScalarTruncToInt(x) >= display_rect().right())

[msw, 2014/09/05 20:15:32]

Truncation doesn't seem correct here; why do we need this change?

[Arjan van Leeuwen, 2014/09/08 09:20:11]

This is what actually stops it from rendering glyphs outside the area (the
change above only activates if the start of the line is already outside the
display area, so that we don't have to paint the current line at all).

We set the index to style_range.end() if we've either reached the end of the
line (glyph_index == glyph_count, as before) or the current and subsequent
characters will fall out of the display area (new).

The value is truncated because if for example we have a unit that starts at
(double) 1.7, and (int) display_rect.right() is 2, we still probably need to
draw the glyph (after all it's part of the interval 1-2), so rounding it up
would be wrong but truncation means there's no risk of accidentally not painting
a glyph that needed painting.

[msw, 2014/09/08 19:10:30]

Ah, your change below to the while loop on [new] line 479/480 doesn't suffice
for early termination alone, because it would leave the last range of styled
glyphs unpainted, right? I think that's what I didn't catch on my first review.

Still, modifying |glyph_text_index| seems wrong, can you instead modify the if
statement below to if (!IndexInRange(style_range, glyph_text_index) ||
SkScalarTruncToInt(x) >= display_rect().right())?

Add a comment to explain the early termination either way.

[Arjan van Leeuwen, 2014/09/09 07:03:59]

Right. Added some comments now.

+              ? style_range.end()
+              : GetGlyphTextIndex(run, glyph_index);
       if (!IndexInRange(style_range, glyph_text_index)) {
         // TODO(asvitkine): For cases like "fi", where "fi" is a single glyph
         //                  but can span multiple styles, Pango splits the
@@ -470,7 +476,8 @@ void RenderTextPango::DrawVisualText(Canvas* canvas) {
         style_start_glyph_index = glyph_index;
         style_start_x = x;
       }
-    } while (glyph_index < glyph_count);
+    } while (glyph_index < glyph_count &&
+             SkScalarTruncToInt(x) < display_rect().right());
   }
 
   renderer.EndDiagonalStrike();