Commit fe04b007 authored by Nicolas Pena's avatar Nicolas Pena Committed by Commit Bot

Add |contains_only_whitespace_| to LayoutText and TextRun

This CL takes advantage of the loop through text in LayoutText to store
a variable indicating whether text contains only 'whitespace' or not.
The main purpose of the loop is to compute widths but this seems like a
reasonable place to also store this new information, to avoid an extra
iteration over the text.

Now, TextRun has a boolean variable indicating whether the text to be
painted contains a whitespace or not. For now, the boolean is only
changed in InlineTextBox::ConstructTextRun to prevent Font::DrawText
from returning true. The return value of this method is only used to
determine whether SetTextPainted() is called in
GraphicsContext::DrawTextInternal.

PaintController::SetTextPainted is the method that triggers First
Contentful Paint (caused by text) on PaintTiming. We do not want it to
be triggered on trivial whitespace text rendering.

Test: Check whether FCP is observed on a document with just
<div>&nbsp</div>

Also, this fix should allow landing the test in:
https://chromium-review.googlesource.com/c/chromium/src/+/769929

Bug: 785344
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Id219eeaced4679eda9c26acde7fd4fdb0cb1e891
Reviewed-on: https://chromium-review.googlesource.com/786336Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519455}
parent 28d1e1c5
...@@ -66,7 +66,7 @@ ...@@ -66,7 +66,7 @@
namespace blink { namespace blink {
struct SameSizeAsLayoutText : public LayoutObject { struct SameSizeAsLayoutText : public LayoutObject {
uint32_t bitfields : 16; uint32_t bitfields : 11;
float widths[4]; float widths[4];
String text; String text;
void* pointers[2]; void* pointers[2];
...@@ -160,6 +160,8 @@ LayoutText::LayoutText(Node* node, scoped_refptr<StringImpl> str) ...@@ -160,6 +160,8 @@ LayoutText::LayoutText(Node* node, scoped_refptr<StringImpl> str)
lines_dirty_(false), lines_dirty_(false),
contains_reversed_text_(false), contains_reversed_text_(false),
known_to_have_no_overflow_and_no_fallback_fonts_(false), known_to_have_no_overflow_and_no_fallback_fonts_(false),
contains_only_whitespace_or_nbsp_(
static_cast<unsigned>(OnlyWhitespaceOrNbsp::kUnknown)),
min_width_(-1), min_width_(-1),
max_width_(-1), max_width_(-1),
first_line_min_width_(0), first_line_min_width_(0),
...@@ -1076,6 +1078,8 @@ void LayoutText::ComputePreferredLogicalWidths( ...@@ -1076,6 +1078,8 @@ void LayoutText::ComputePreferredLogicalWidths(
has_breakable_start_ = false; has_breakable_start_ = false;
has_breakable_end_ = false; has_breakable_end_ = false;
has_end_white_space_ = false; has_end_white_space_ = false;
contains_only_whitespace_or_nbsp_ =
static_cast<unsigned>(OnlyWhitespaceOrNbsp::kYes);
const ComputedStyle& style_to_use = StyleRef(); const ComputedStyle& style_to_use = StyleRef();
const Font& f = style_to_use.GetFont(); // FIXME: This ignores first-line. const Font& f = style_to_use.GetFont(); // FIXME: This ignores first-line.
...@@ -1164,8 +1168,14 @@ void LayoutText::ComputePreferredLogicalWidths( ...@@ -1164,8 +1168,14 @@ void LayoutText::ComputePreferredLogicalWidths(
} else { } else {
is_space = true; is_space = true;
} }
} else if (c == kSpaceCharacter) {
is_space = true;
} else if (c == kNoBreakSpaceCharacter) {
is_space = false;
} else { } else {
is_space = c == kSpaceCharacter; is_space = false;
contains_only_whitespace_or_nbsp_ =
static_cast<unsigned>(OnlyWhitespaceOrNbsp::kNo);
} }
bool is_breakable_location = bool is_breakable_location =
...@@ -1540,6 +1550,12 @@ static inline bool IsInlineFlowOrEmptyText(const LayoutObject* o) { ...@@ -1540,6 +1550,12 @@ static inline bool IsInlineFlowOrEmptyText(const LayoutObject* o) {
return ToLayoutText(o)->GetText().IsEmpty(); return ToLayoutText(o)->GetText().IsEmpty();
} }
OnlyWhitespaceOrNbsp LayoutText::ContainsOnlyWhitespaceOrNbsp() const {
return PreferredLogicalWidthsDirty() ? OnlyWhitespaceOrNbsp::kUnknown
: static_cast<OnlyWhitespaceOrNbsp>(
contains_only_whitespace_or_nbsp_);
}
UChar LayoutText::PreviousCharacter() const { UChar LayoutText::PreviousCharacter() const {
// find previous text layoutObject if one exists // find previous text layoutObject if one exists
const LayoutObject* previous_text = PreviousInPreOrder(); const LayoutObject* previous_text = PreviousInPreOrder();
......
...@@ -38,6 +38,8 @@ class AbstractInlineTextBox; ...@@ -38,6 +38,8 @@ class AbstractInlineTextBox;
class InlineTextBox; class InlineTextBox;
class NGOffsetMapping; class NGOffsetMapping;
enum class OnlyWhitespaceOrNbsp : unsigned { kUnknown = 0, kNo = 1, kYes = 2 };
// LayoutText is the root class for anything that represents // LayoutText is the root class for anything that represents
// a text node (see core/dom/Text.h). // a text node (see core/dom/Text.h).
// //
...@@ -256,6 +258,8 @@ class CORE_EXPORT LayoutText : public LayoutObject { ...@@ -256,6 +258,8 @@ class CORE_EXPORT LayoutText : public LayoutObject {
known_to_have_no_overflow_and_no_fallback_fonts_ = false; known_to_have_no_overflow_and_no_fallback_fonts_ = false;
} }
OnlyWhitespaceOrNbsp ContainsOnlyWhitespaceOrNbsp() const;
virtual UChar PreviousCharacter() const; virtual UChar PreviousCharacter() const;
protected: protected:
...@@ -330,22 +334,23 @@ class CORE_EXPORT LayoutText : public LayoutObject { ...@@ -330,22 +334,23 @@ class CORE_EXPORT LayoutText : public LayoutObject {
// We put the bitfield first to minimize padding on 64-bit. // We put the bitfield first to minimize padding on 64-bit.
// Whether or not we can be broken into multiple lines. // Whether or not we can be broken into multiple lines.
bool has_breakable_char_ : 1; unsigned has_breakable_char_ : 1;
// Whether or not we have a hard break (e.g., <pre> with '\n'). // Whether or not we have a hard break (e.g., <pre> with '\n').
bool has_break_ : 1; unsigned has_break_ : 1;
// Whether or not we have a variable width tab character (e.g., <pre> with // Whether or not we have a variable width tab character (e.g., <pre> with
// '\t'). // '\t').
bool has_tab_ : 1; unsigned has_tab_ : 1;
bool has_breakable_start_ : 1; unsigned has_breakable_start_ : 1;
bool has_breakable_end_ : 1; unsigned has_breakable_end_ : 1;
bool has_end_white_space_ : 1; unsigned has_end_white_space_ : 1;
// This bit indicates that the text run has already dirtied specific line // This bit indicates that the text run has already dirtied specific line
// boxes, and this hint will enable layoutInlineChildren to avoid just // boxes, and this hint will enable layoutInlineChildren to avoid just
// dirtying everything when character data is modified (e.g., appended/ // dirtying everything when character data is modified (e.g., appended/
// inserted or removed). // inserted or removed).
bool lines_dirty_ : 1; unsigned lines_dirty_ : 1;
bool contains_reversed_text_ : 1; unsigned contains_reversed_text_ : 1;
mutable bool known_to_have_no_overflow_and_no_fallback_fonts_ : 1; mutable unsigned known_to_have_no_overflow_and_no_fallback_fonts_ : 1;
unsigned contains_only_whitespace_or_nbsp_ : 2;
float min_width_; float min_width_;
float max_width_; float max_width_;
......
...@@ -121,6 +121,33 @@ TEST_F(LayoutTextTest, WidthLengthBeyondLength) { ...@@ -121,6 +121,33 @@ TEST_F(LayoutTextTest, WidthLengthBeyondLength) {
ASSERT_LE(width, 20.f); ASSERT_LE(width, 20.f);
} }
TEST_F(LayoutTextTest, ContainsOnlyWhitespaceOrNbsp) {
// Note that '&nbsp' is needed when only including whitespace to force
// LayoutText creation from the div.
SetBasicBody("&nbsp");
// GetWidth will also compute |contains_only_whitespace_|.
GetBasicText()->Width(0u, 1u, LayoutUnit(), TextDirection::kLtr, false);
EXPECT_EQ(OnlyWhitespaceOrNbsp::kYes,
GetBasicText()->ContainsOnlyWhitespaceOrNbsp());
SetBasicBody(" \t\t\n \n\t &nbsp \n\t&nbsp\n \t");
EXPECT_EQ(OnlyWhitespaceOrNbsp::kUnknown,
GetBasicText()->ContainsOnlyWhitespaceOrNbsp());
GetBasicText()->Width(0u, 18u, LayoutUnit(), TextDirection::kLtr, false);
EXPECT_EQ(OnlyWhitespaceOrNbsp::kYes,
GetBasicText()->ContainsOnlyWhitespaceOrNbsp());
SetBasicBody("abc");
GetBasicText()->Width(0u, 3u, LayoutUnit(), TextDirection::kLtr, false);
EXPECT_EQ(OnlyWhitespaceOrNbsp::kNo,
GetBasicText()->ContainsOnlyWhitespaceOrNbsp());
SetBasicBody(" \t&nbsp\nx \n");
GetBasicText()->Width(0u, 8u, LayoutUnit(), TextDirection::kLtr, false);
EXPECT_EQ(OnlyWhitespaceOrNbsp::kNo,
GetBasicText()->ContainsOnlyWhitespaceOrNbsp());
}
TEST_P(ParameterizedLayoutTextTest, CaretMinMaxOffset) { TEST_P(ParameterizedLayoutTextTest, CaretMinMaxOffset) {
SetBasicBody("foo"); SetBasicBody("foo");
EXPECT_EQ(0, GetBasicText()->CaretMinOffset()); EXPECT_EQ(0, GetBasicText()->CaretMinOffset());
......
...@@ -53,6 +53,10 @@ class LineLayoutText : public LineLayoutItem { ...@@ -53,6 +53,10 @@ class LineLayoutText : public LineLayoutItem {
return ToText()->IsAllCollapsibleWhitespace(); return ToText()->IsAllCollapsibleWhitespace();
} }
OnlyWhitespaceOrNbsp ContainsOnlyWhitespaceOrNbsp() const {
return ToText()->ContainsOnlyWhitespaceOrNbsp();
}
UChar CharacterAt(unsigned offset) const { UChar CharacterAt(unsigned offset) const {
return ToText()->CharacterAt(offset); return ToText()->CharacterAt(offset);
} }
......
...@@ -34,8 +34,10 @@ ...@@ -34,8 +34,10 @@
#include "core/layout/line/AbstractInlineTextBox.h" #include "core/layout/line/AbstractInlineTextBox.h"
#include "core/layout/line/EllipsisBox.h" #include "core/layout/line/EllipsisBox.h"
#include "core/paint/InlineTextBoxPainter.h" #include "core/paint/InlineTextBoxPainter.h"
#include "core/paint/PaintInfo.h"
#include "platform/fonts/CharacterRange.h" #include "platform/fonts/CharacterRange.h"
#include "platform/fonts/FontCache.h" #include "platform/fonts/FontCache.h"
#include "platform/graphics/paint/PaintController.h"
#include "platform/wtf/Vector.h" #include "platform/wtf/Vector.h"
#include "platform/wtf/text/StringBuilder.h" #include "platform/wtf/text/StringBuilder.h"
...@@ -546,6 +548,10 @@ void InlineTextBox::Paint(const PaintInfo& paint_info, ...@@ -546,6 +548,10 @@ void InlineTextBox::Paint(const PaintInfo& paint_info,
LayoutUnit /*lineTop*/, LayoutUnit /*lineTop*/,
LayoutUnit /*lineBottom*/) const { LayoutUnit /*lineBottom*/) const {
InlineTextBoxPainter(*this).Paint(paint_info, paint_offset); InlineTextBoxPainter(*this).Paint(paint_info, paint_offset);
if (GetLineLayoutItem().ContainsOnlyWhitespaceOrNbsp() !=
OnlyWhitespaceOrNbsp::kYes) {
paint_info.context.GetPaintController().SetTextPainted();
}
} }
void InlineTextBox::SelectionStartEnd(int& s_pos, int& e_pos) const { void InlineTextBox::SelectionStartEnd(int& s_pos, int& e_pos) const {
...@@ -597,6 +603,10 @@ void InlineTextBox::PaintTextMatchMarkerForeground( ...@@ -597,6 +603,10 @@ void InlineTextBox::PaintTextMatchMarkerForeground(
const Font& font) const { const Font& font) const {
InlineTextBoxPainter(*this).PaintTextMatchMarkerForeground( InlineTextBoxPainter(*this).PaintTextMatchMarkerForeground(
paint_info, box_origin, marker, style, font); paint_info, box_origin, marker, style, font);
if (GetLineLayoutItem().ContainsOnlyWhitespaceOrNbsp() !=
OnlyWhitespaceOrNbsp::kYes) {
paint_info.context.GetPaintController().SetTextPainted();
}
} }
void InlineTextBox::PaintTextMatchMarkerBackground( void InlineTextBox::PaintTextMatchMarkerBackground(
......
...@@ -69,6 +69,9 @@ void EllipsisBoxPainter::PaintEllipsis(const PaintInfo& paint_info, ...@@ -69,6 +69,9 @@ void EllipsisBoxPainter::PaintEllipsis(const PaintInfo& paint_info,
ellipsis_box_.IsHorizontal()); ellipsis_box_.IsHorizontal());
text_painter.Paint(0, ellipsis_box_.EllipsisStr().length(), text_painter.Paint(0, ellipsis_box_.EllipsisStr().length(),
ellipsis_box_.EllipsisStr().length(), text_style); ellipsis_box_.EllipsisStr().length(), text_style);
// TODO(npm): Check that there are non-whitespace characters. See
// crbug.com/788444.
context.GetPaintController().SetTextPainted();
} }
} // namespace blink } // namespace blink
...@@ -178,6 +178,9 @@ void ListMarkerPainter::Paint(const PaintInfo& paint_info, ...@@ -178,6 +178,9 @@ void ListMarkerPainter::Paint(const PaintInfo& paint_info,
context.DrawText(font, text_run_paint_info, context.DrawText(font, text_run_paint_info,
text_origin + IntSize(font.Width(suffix_run), 0)); text_origin + IntSize(font.Width(suffix_run), 0));
} }
// TODO(npm): Check that there are non-whitespace characters. See
// crbug.com/788444.
context.GetPaintController().SetTextPainted();
} }
} // namespace blink } // namespace blink
...@@ -465,6 +465,9 @@ void SVGInlineTextBoxPainter::PaintText(const PaintInfo& paint_info, ...@@ -465,6 +465,9 @@ void SVGInlineTextBoxPainter::PaintText(const PaintInfo& paint_info,
text_size.Height()); text_size.Height());
context.DrawText(scaled_font, text_run_paint_info, text_origin, flags); context.DrawText(scaled_font, text_run_paint_info, text_origin, flags);
// TODO(npm): Check that there are non-whitespace characters. See
// crbug.com/788444.
context.GetPaintController().SetTextPainted();
} }
void SVGInlineTextBoxPainter::PaintText(const PaintInfo& paint_info, void SVGInlineTextBoxPainter::PaintText(const PaintInfo& paint_info,
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "platform/fonts/NGTextFragmentPaintInfo.h" #include "platform/fonts/NGTextFragmentPaintInfo.h"
#include "platform/graphics/GraphicsContext.h" #include "platform/graphics/GraphicsContext.h"
#include "platform/graphics/GraphicsContextStateSaver.h" #include "platform/graphics/GraphicsContextStateSaver.h"
#include "platform/graphics/paint/PaintController.h"
#include "platform/wtf/Assertions.h" #include "platform/wtf/Assertions.h"
#include "platform/wtf/text/CharacterNames.h" #include "platform/wtf/text/CharacterNames.h"
...@@ -56,6 +57,9 @@ void NGTextPainter::PaintInternalFragment( ...@@ -56,6 +57,9 @@ void NGTextPainter::PaintInternalFragment(
DCHECK(step == kPaintText); DCHECK(step == kPaintText);
graphics_context_.DrawText(font_, fragment_paint_info, graphics_context_.DrawText(font_, fragment_paint_info,
FloatPoint(text_origin_)); FloatPoint(text_origin_));
// TODO(npm): Check that there are non-whitespace characters. See
// crbug.com/788444.
graphics_context_.GetPaintController().SetTextPainted();
} }
} }
......
...@@ -129,7 +129,7 @@ void DrawBlobs(PaintCanvas* canvas, ...@@ -129,7 +129,7 @@ void DrawBlobs(PaintCanvas* canvas,
} // anonymous ns } // anonymous ns
bool Font::DrawText(PaintCanvas* canvas, void Font::DrawText(PaintCanvas* canvas,
const TextRunPaintInfo& run_info, const TextRunPaintInfo& run_info,
const FloatPoint& point, const FloatPoint& point,
float device_scale_factor, float device_scale_factor,
...@@ -137,7 +137,7 @@ bool Font::DrawText(PaintCanvas* canvas, ...@@ -137,7 +137,7 @@ bool Font::DrawText(PaintCanvas* canvas,
// Don't draw anything while we are using custom fonts that are in the process // Don't draw anything while we are using custom fonts that are in the process
// of loading. // of loading.
if (ShouldSkipDrawing()) if (ShouldSkipDrawing())
return false; return;
ShapeResultBloberizer bloberizer(*this, device_scale_factor); ShapeResultBloberizer bloberizer(*this, device_scale_factor);
CachingWordShaper word_shaper(*this); CachingWordShaper word_shaper(*this);
...@@ -145,10 +145,9 @@ bool Font::DrawText(PaintCanvas* canvas, ...@@ -145,10 +145,9 @@ bool Font::DrawText(PaintCanvas* canvas,
word_shaper.FillResultBuffer(run_info, &buffer); word_shaper.FillResultBuffer(run_info, &buffer);
bloberizer.FillGlyphs(run_info, buffer); bloberizer.FillGlyphs(run_info, buffer);
DrawBlobs(canvas, flags, bloberizer.Blobs(), point); DrawBlobs(canvas, flags, bloberizer.Blobs(), point);
return true;
} }
bool Font::DrawText(PaintCanvas* canvas, void Font::DrawText(PaintCanvas* canvas,
const NGTextFragmentPaintInfo& text_info, const NGTextFragmentPaintInfo& text_info,
const FloatPoint& point, const FloatPoint& point,
float device_scale_factor, float device_scale_factor,
...@@ -156,13 +155,12 @@ bool Font::DrawText(PaintCanvas* canvas, ...@@ -156,13 +155,12 @@ bool Font::DrawText(PaintCanvas* canvas,
// Don't draw anything while we are using custom fonts that are in the process // Don't draw anything while we are using custom fonts that are in the process
// of loading. // of loading.
if (ShouldSkipDrawing()) if (ShouldSkipDrawing())
return false; return;
ShapeResultBloberizer bloberizer(*this, device_scale_factor); ShapeResultBloberizer bloberizer(*this, device_scale_factor);
bloberizer.FillGlyphs(text_info.text, text_info.from, text_info.to, bloberizer.FillGlyphs(text_info.text, text_info.from, text_info.to,
text_info.shape_result); text_info.shape_result);
DrawBlobs(canvas, flags, bloberizer.Blobs(), point); DrawBlobs(canvas, flags, bloberizer.Blobs(), point);
return true;
} }
bool Font::DrawBidiText(PaintCanvas* canvas, bool Font::DrawBidiText(PaintCanvas* canvas,
......
...@@ -81,12 +81,12 @@ class PLATFORM_EXPORT Font { ...@@ -81,12 +81,12 @@ class PLATFORM_EXPORT Font {
kDoNotPaintIfFontNotReady, kDoNotPaintIfFontNotReady,
kUseFallbackIfFontNotReady kUseFallbackIfFontNotReady
}; };
bool DrawText(PaintCanvas*, void DrawText(PaintCanvas*,
const TextRunPaintInfo&, const TextRunPaintInfo&,
const FloatPoint&, const FloatPoint&,
float device_scale_factor, float device_scale_factor,
const PaintFlags&) const; const PaintFlags&) const;
bool DrawText(PaintCanvas*, void DrawText(PaintCanvas*,
const NGTextFragmentPaintInfo&, const NGTextFragmentPaintInfo&,
const FloatPoint&, const FloatPoint&,
float device_scale_factor, float device_scale_factor,
......
...@@ -743,10 +743,8 @@ void GraphicsContext::DrawTextInternal(const Font& font, ...@@ -743,10 +743,8 @@ void GraphicsContext::DrawTextInternal(const Font& font,
if (ContextDisabled()) if (ContextDisabled())
return; return;
if (font.DrawText(canvas_, text_info, point, device_scale_factor_, font.DrawText(canvas_, text_info, point, device_scale_factor_,
ApplyHighContrastFilter(&flags))) { ApplyHighContrastFilter(&flags));
paint_controller_.SetTextPainted();
}
} }
void GraphicsContext::DrawText(const Font& font, void GraphicsContext::DrawText(const Font& font,
...@@ -790,9 +788,8 @@ void GraphicsContext::DrawTextInternal(const Font& font, ...@@ -790,9 +788,8 @@ void GraphicsContext::DrawTextInternal(const Font& font,
return; return;
DrawTextPasses([&font, &text_info, &point, this](const PaintFlags& flags) { DrawTextPasses([&font, &text_info, &point, this](const PaintFlags& flags) {
if (font.DrawText(canvas_, text_info, point, device_scale_factor_, font.DrawText(canvas_, text_info, point, device_scale_factor_,
ApplyHighContrastFilter(&flags))) ApplyHighContrastFilter(&flags));
paint_controller_.SetTextPainted();
}); });
} }
......
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