Commit a7b561cb authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[FragmentItem] Fix text snap-to-pixel behavior

This patch matches text snap-to-pixel to the current behavior.

Legacy and |NGPaintFragment| snaps the top of the padding box
of the parent box. Before this patch, |NGFragmentItem| snaps
the top of the content box. This makes different snapping
behavior for fractional paddings.

This patch adds |parent_offset| to compute the same snapping
behavior, because |NGPaintFragment| has both the offset to
parent and the offset to container block but |NGFragmentItem|
only has the latter.

Fixes ~12 tests.

Note, the snapping behavior will need to be revisited.

Bug: 982194
Change-Id: If2b52c3ae986d5ff06dc993016ad2eb0ff66de39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024133Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736251}
parent 75fc7599
...@@ -416,14 +416,15 @@ void NGBoxFragmentPainter::PaintObject( ...@@ -416,14 +416,15 @@ void NGBoxFragmentPainter::PaintObject(
paint_offset - box_item_->Offset(); paint_offset - box_item_->Offset();
PaintInlineItems(paint_info.ForDescendants(), PaintInlineItems(paint_info.ForDescendants(),
paint_offset_to_inline_formatting_context, paint_offset_to_inline_formatting_context,
descendants_); box_item_->Offset(), descendants_);
} else if (items_) { } else if (items_) {
if (physical_box_fragment.IsBlockFlow()) { if (physical_box_fragment.IsBlockFlow()) {
PaintBlockFlowContents(paint_info, paint_offset); PaintBlockFlowContents(paint_info, paint_offset);
} else { } else {
DCHECK(physical_box_fragment.IsInlineBox()); DCHECK(physical_box_fragment.IsInlineBox());
NGInlineCursor cursor(*items_); NGInlineCursor cursor(*items_);
PaintInlineItems(paint_info.ForDescendants(), paint_offset, &cursor); PaintInlineItems(paint_info.ForDescendants(), paint_offset,
PhysicalOffset(), &cursor);
} }
} else if (physical_box_fragment.ChildrenInline()) { } else if (physical_box_fragment.ChildrenInline()) {
DCHECK(!RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()); DCHECK(!RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
...@@ -1046,6 +1047,7 @@ void NGBoxFragmentPainter::PaintAllPhasesAtomically( ...@@ -1046,6 +1047,7 @@ void NGBoxFragmentPainter::PaintAllPhasesAtomically(
void NGBoxFragmentPainter::PaintInlineItems(const PaintInfo& paint_info, void NGBoxFragmentPainter::PaintInlineItems(const PaintInfo& paint_info,
const PhysicalOffset& paint_offset, const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset,
NGInlineCursor* cursor) { NGInlineCursor* cursor) {
while (*cursor) { while (*cursor) {
const NGFragmentItem* item = cursor->CurrentItem(); const NGFragmentItem* item = cursor->CurrentItem();
...@@ -1053,19 +1055,18 @@ void NGBoxFragmentPainter::PaintInlineItems(const PaintInfo& paint_info, ...@@ -1053,19 +1055,18 @@ void NGBoxFragmentPainter::PaintInlineItems(const PaintInfo& paint_info,
switch (item->Type()) { switch (item->Type()) {
case NGFragmentItem::kText: case NGFragmentItem::kText:
case NGFragmentItem::kGeneratedText: case NGFragmentItem::kGeneratedText:
PaintTextItem(*cursor, paint_info, paint_offset); PaintTextItem(*cursor, paint_info, paint_offset, parent_offset);
cursor->MoveToNext();
break; break;
case NGFragmentItem::kBox: case NGFragmentItem::kBox:
if (PaintBoxItem(*item, paint_info, paint_offset) == kSkipChildren) { PaintBoxItem(*item, *cursor, paint_info, paint_offset);
cursor->MoveToNextSkippingChildren(); cursor->MoveToNextSkippingChildren();
continue;
}
break; break;
case NGFragmentItem::kLine: case NGFragmentItem::kLine:
NOTREACHED(); NOTREACHED();
cursor->MoveToNext();
break; break;
} }
cursor->MoveToNext();
} }
} }
...@@ -1212,14 +1213,15 @@ void NGBoxFragmentPainter::PaintLineBoxChildItems( ...@@ -1212,14 +1213,15 @@ void NGBoxFragmentPainter::PaintLineBoxChildItems(
/* line_box_paint_fragment */ nullptr, child_item, /* line_box_paint_fragment */ nullptr, child_item,
paint_info, child_offset); paint_info, child_offset);
NGInlineCursor line_box_cursor = children->CursorForDescendants(); NGInlineCursor line_box_cursor = children->CursorForDescendants();
PaintInlineItems(paint_info, paint_offset, &line_box_cursor); PaintInlineItems(paint_info, paint_offset, child_item->Offset(),
&line_box_cursor);
continue; continue;
} }
if (const NGPhysicalBoxFragment* child_fragment = if (const NGPhysicalBoxFragment* child_fragment =
child_item->BoxFragment()) { child_item->BoxFragment()) {
if (child_fragment->IsListMarker()) { if (child_fragment->IsListMarker()) {
PaintBoxItem(*child_item, paint_info, paint_offset); PaintBoxItem(*child_item, *children, paint_info, paint_offset);
continue; continue;
} }
} }
...@@ -1329,7 +1331,8 @@ void NGBoxFragmentPainter::PaintTextChild(const NGPaintFragment& paint_fragment, ...@@ -1329,7 +1331,8 @@ void NGBoxFragmentPainter::PaintTextChild(const NGPaintFragment& paint_fragment,
void NGBoxFragmentPainter::PaintTextItem(const NGInlineCursor& cursor, void NGBoxFragmentPainter::PaintTextItem(const NGInlineCursor& cursor,
const PaintInfo& paint_info, const PaintInfo& paint_info,
const PhysicalOffset& paint_offset) { const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset) {
DCHECK(cursor.CurrentItem()); DCHECK(cursor.CurrentItem());
const NGFragmentItem& item = *cursor.CurrentItem(); const NGFragmentItem& item = *cursor.CurrentItem();
DCHECK(item.IsText()) << item; DCHECK(item.IsText()) << item;
...@@ -1346,7 +1349,7 @@ void NGBoxFragmentPainter::PaintTextItem(const NGInlineCursor& cursor, ...@@ -1346,7 +1349,7 @@ void NGBoxFragmentPainter::PaintTextItem(const NGInlineCursor& cursor,
if (UNLIKELY(!IsVisibleToPaint(item, item.Style()))) if (UNLIKELY(!IsVisibleToPaint(item, item.Style())))
return; return;
NGTextFragmentPainter<NGInlineCursor> text_painter(cursor); NGTextFragmentPainter<NGInlineCursor> text_painter(cursor, parent_offset);
text_painter.Paint(paint_info, paint_offset); text_painter.Paint(paint_info, paint_offset);
} }
...@@ -1365,40 +1368,40 @@ NGBoxFragmentPainter::MoveTo NGBoxFragmentPainter::PaintLineBoxItem( ...@@ -1365,40 +1368,40 @@ NGBoxFragmentPainter::MoveTo NGBoxFragmentPainter::PaintLineBoxItem(
return kDontSkipChildren; return kDontSkipChildren;
} }
NGBoxFragmentPainter::MoveTo NGBoxFragmentPainter::PaintBoxItem( void NGBoxFragmentPainter::PaintBoxItem(const NGFragmentItem& item,
const NGFragmentItem& item, const NGInlineCursor& cursor,
const PaintInfo& paint_info, const PaintInfo& paint_info,
const PhysicalOffset& paint_offset) { const PhysicalOffset& paint_offset) {
DCHECK_EQ(item.Type(), NGFragmentItem::kBox); DCHECK_EQ(item.Type(), NGFragmentItem::kBox);
DCHECK_EQ(&item, cursor.Current().Item());
const ComputedStyle& style = item.Style(); const ComputedStyle& style = item.Style();
if (UNLIKELY(!IsVisibleToPaint(item, style))) if (UNLIKELY(!IsVisibleToPaint(item, style)))
return kSkipChildren; return;
// Nothing to paint if this is a culled inline box. Proceed to its
// descendants.
const NGPhysicalBoxFragment* child_fragment = item.BoxFragment();
if (!child_fragment)
return kDontSkipChildren;
DCHECK(!child_fragment->IsHiddenForPaint()); if (const NGPhysicalBoxFragment* child_fragment = item.BoxFragment()) {
if (child_fragment->HasSelfPaintingLayer() || child_fragment->IsFloating()) DCHECK(!child_fragment->IsHiddenForPaint());
return kSkipChildren; if (child_fragment->HasSelfPaintingLayer() || child_fragment->IsFloating())
return;
// TODO(kojii): Check CullRect. // TODO(kojii): Check CullRect.
if (child_fragment->IsAtomicInline() || child_fragment->IsListMarker()) { if (child_fragment->IsAtomicInline() || child_fragment->IsListMarker()) {
if (FragmentRequiresLegacyFallback(*child_fragment)) { if (FragmentRequiresLegacyFallback(*child_fragment)) {
PaintInlineChildBoxUsingLegacyFallback(*child_fragment, paint_info); PaintInlineChildBoxUsingLegacyFallback(*child_fragment, paint_info);
return kDontSkipChildren; return;
}
NGBoxFragmentPainter(*child_fragment)
.PaintAllPhasesAtomically(paint_info);
return;
} }
NGBoxFragmentPainter(*child_fragment).PaintAllPhasesAtomically(paint_info);
return kDontSkipChildren; NGInlineBoxFragmentPainter(item, *child_fragment)
.Paint(paint_info, paint_offset);
} }
NGInlineBoxFragmentPainter(item, *child_fragment) NGInlineCursor children = cursor.CursorForDescendants();
.Paint(paint_info, paint_offset); PaintInlineItems(paint_info, paint_offset, item.Offset(), &children);
return kDontSkipChildren;
} }
bool NGBoxFragmentPainter::IsPaintingScrollingBackground( bool NGBoxFragmentPainter::IsPaintingScrollingBackground(
......
...@@ -101,6 +101,7 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -101,6 +101,7 @@ class NGBoxFragmentPainter : public BoxPainterBase {
void PaintBlockChildren(const PaintInfo&); void PaintBlockChildren(const PaintInfo&);
void PaintInlineItems(const PaintInfo&, void PaintInlineItems(const PaintInfo&,
const PhysicalOffset& paint_offset, const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset,
NGInlineCursor* cursor); NGInlineCursor* cursor);
void PaintLineBoxChildren(NGInlineCursor* children, void PaintLineBoxChildren(NGInlineCursor* children,
const PaintInfo&, const PaintInfo&,
...@@ -130,13 +131,15 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -130,13 +131,15 @@ class NGBoxFragmentPainter : public BoxPainterBase {
const PhysicalOffset& paint_offset); const PhysicalOffset& paint_offset);
void PaintTextItem(const NGInlineCursor& cursor, void PaintTextItem(const NGInlineCursor& cursor,
const PaintInfo&, const PaintInfo&,
const PhysicalOffset& paint_offset); const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset);
MoveTo PaintLineBoxItem(const NGFragmentItem& item, MoveTo PaintLineBoxItem(const NGFragmentItem& item,
const PaintInfo& paint_info, const PaintInfo& paint_info,
const PhysicalOffset& paint_offset); const PhysicalOffset& paint_offset);
MoveTo PaintBoxItem(const NGFragmentItem& item, void PaintBoxItem(const NGFragmentItem& item,
const PaintInfo& paint_info, const NGInlineCursor& cursor,
const PhysicalOffset& paint_offset); const PaintInfo& paint_info,
const PhysicalOffset& paint_offset);
void PaintFloatingChildren(const NGPhysicalContainerFragment&, void PaintFloatingChildren(const NGPhysicalContainerFragment&,
const PaintInfo& paint_info, const PaintInfo& paint_info,
const PaintInfo& float_paint_info); const PaintInfo& float_paint_info);
......
...@@ -64,6 +64,29 @@ inline const NGPaintFragment& AsDisplayItemClient( ...@@ -64,6 +64,29 @@ inline const NGPaintFragment& AsDisplayItemClient(
return cursor.PaintFragment(); return cursor.PaintFragment();
} }
inline PhysicalRect ComputeBoxRect(const NGInlineCursor& cursor,
const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset) {
PhysicalRect box_rect = cursor.CurrentItem()->Rect();
box_rect.offset.left += paint_offset.left;
// We round the y-axis to ensure consistent line heights.
box_rect.offset.top =
LayoutUnit((paint_offset.top + parent_offset.top).Round()) +
(box_rect.offset.top - parent_offset.top);
return box_rect;
}
inline PhysicalRect ComputeBoxRect(const NGTextPainterCursor& cursor,
const PhysicalOffset& paint_offset,
const PhysicalOffset& parent_offset) {
PhysicalRect box_rect = cursor.PaintFragment().Rect();
// We round the y-axis to ensure consistent line heights.
PhysicalOffset adjusted_paint_offset(paint_offset.left,
LayoutUnit(paint_offset.top.Round()));
box_rect.offset += adjusted_paint_offset;
return box_rect;
}
inline const NGInlineCursor& InlineCursorForBlockFlow( inline const NGInlineCursor& InlineCursorForBlockFlow(
const NGInlineCursor& cursor, const NGInlineCursor& cursor,
base::Optional<NGInlineCursor>* storage) { base::Optional<NGInlineCursor>* storage) {
...@@ -327,10 +350,6 @@ const NGPaintFragment& NGTextPainterCursor::RootPaintFragment() const { ...@@ -327,10 +350,6 @@ const NGPaintFragment& NGTextPainterCursor::RootPaintFragment() const {
return *root_paint_fragment_; return *root_paint_fragment_;
} }
template <typename Cursor>
NGTextFragmentPainter<Cursor>::NGTextFragmentPainter(const Cursor& cursor)
: cursor_(cursor) {}
// Logic is copied from InlineTextBoxPainter::PaintSelection. // Logic is copied from InlineTextBoxPainter::PaintSelection.
// |selection_start| and |selection_end| should be between // |selection_start| and |selection_end| should be between
// [text_fragment.StartOffset(), text_fragment.EndOffset()]. // [text_fragment.StartOffset(), text_fragment.EndOffset()].
...@@ -423,11 +442,7 @@ void NGTextFragmentPainter<Cursor>::Paint(const PaintInfo& paint_info, ...@@ -423,11 +442,7 @@ void NGTextFragmentPainter<Cursor>::Paint(const PaintInfo& paint_info,
paint_info.phase); paint_info.phase);
} }
// We round the y-axis to ensure consistent line heights. PhysicalRect box_rect = ComputeBoxRect(cursor_, paint_offset, parent_offset_);
PhysicalRect box_rect = AsDisplayItemClient(cursor_).Rect();
PhysicalOffset adjusted_paint_offset(paint_offset.left,
LayoutUnit(paint_offset.top.Round()));
box_rect.offset += adjusted_paint_offset;
if (UNLIKELY(text_item.IsSymbolMarker())) { if (UNLIKELY(text_item.IsSymbolMarker())) {
// The NGInlineItem of marker might be Split(). To avoid calling PaintSymbol // The NGInlineItem of marker might be Split(). To avoid calling PaintSymbol
......
...@@ -58,7 +58,10 @@ class NGTextFragmentPainter { ...@@ -58,7 +58,10 @@ class NGTextFragmentPainter {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
explicit NGTextFragmentPainter(const Cursor&); explicit NGTextFragmentPainter(const Cursor& cursor) : cursor_(cursor) {}
NGTextFragmentPainter(const Cursor& cursor,
const PhysicalOffset& parent_offset)
: cursor_(cursor), parent_offset_(parent_offset) {}
void Paint(const PaintInfo&, const PhysicalOffset& paint_offset); void Paint(const PaintInfo&, const PhysicalOffset& paint_offset);
...@@ -81,6 +84,7 @@ class NGTextFragmentPainter { ...@@ -81,6 +84,7 @@ class NGTextFragmentPainter {
const PhysicalOffset& paint_offset); const PhysicalOffset& paint_offset);
const Cursor& cursor_; const Cursor& cursor_;
PhysicalOffset parent_offset_;
base::Optional<NGInlineCursor> inline_cursor_for_block_flow_; base::Optional<NGInlineCursor> inline_cursor_for_block_flow_;
}; };
......
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