Skip to content

GitLab

T
tangled
Commit 7f73e60a authored by Stefan Zager's avatar Stefan Zager Committed by Commit Bot
Browse files
Options

Make PaintLayer::size_ a LayoutSize 

Snapping PaintLayer size to the pixel grid too early causes a couple of problems:

- It can inadvertently trigger auto scrollbars to be created for a box with
non-pixel-aligned location and non-pixel-aligned content size.  This is the root cause
of the bug linked below.

- There are a few places in the code that call LayoutBox::SetLocation without updating
the box'es PaintLayer's size.  Since PaintLayer size relies on snapping to the box'es
location, this can cause the PaintLayer size to be off by one pixel.  That's what
happened in the new test baseline in this patch: the layer size in the old expectation
is actually off by one pixel.

BUG=777095

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I58362d64eff39c5499417f038f5e6ed5eed6f1e3
Reviewed-on: https://chromium-review.googlesource.com/752030Reviewed-by: default avatarTien-Ren Chen <trchen@chromium.org>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515826}
parent 099f86d3
Hide whitespace changes
Inline Side-by-side
Showing with 65 additions and 28 deletions
third_party/WebKit/LayoutTests/platform/mac/media/controls/controls-page-zoom-in-expected.txt
View file @ 7f73e60a
...@@ -47,5 +47,5 @@ layer at (433,419) size 82x3 ...@@ -47,5 +47,5 @@ layer at (433,419) size 82x3
LayoutBlockFlow (positioned) zI: 1 {DIV} at (0,0) size 82.08x2.88 [bgcolor=#4285F4] LayoutBlockFlow (positioned) zI: 1 {DIV} at (0,0) size 82.08x2.88 [bgcolor=#4285F4]
layer at (169,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47 layer at (169,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47
LayoutBlockFlow (positioned) zI: 2 {DIV} at (27.88,-34.55) size 51.83x69.11 LayoutBlockFlow (positioned) zI: 2 {DIV} at (27.88,-34.55) size 51.83x69.11
layer at (490,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47 layer at (490,386) size 51x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47
LayoutBlockFlow (positioned) zI: 2 {DIV} at (82.52,-34.55) size 51.83x69.11 LayoutBlockFlow (positioned) zI: 2 {DIV} at (82.52,-34.55) size 51.83x69.11
third_party/WebKit/LayoutTests/platform/mac/virtual/new-remote-playback-pipeline/media/controls/controls-page-zoom-in-expected.txt
View file @ 7f73e60a
...@@ -47,5 +47,5 @@ layer at (433,419) size 82x3 ...@@ -47,5 +47,5 @@ layer at (433,419) size 82x3
LayoutBlockFlow (positioned) zI: 1 {DIV} at (0,0) size 82.08x2.88 [bgcolor=#4285F4] LayoutBlockFlow (positioned) zI: 1 {DIV} at (0,0) size 82.08x2.88 [bgcolor=#4285F4]
layer at (169,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47 layer at (169,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47
LayoutBlockFlow (positioned) zI: 2 {DIV} at (27.88,-34.55) size 51.83x69.11 LayoutBlockFlow (positioned) zI: 2 {DIV} at (27.88,-34.55) size 51.83x69.11
layer at (490,386) size 52x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47 layer at (490,386) size 51x70 backgroundClip at (12,397) size 576x47 clip at (12,397) size 576x47
LayoutBlockFlow (positioned) zI: 2 {DIV} at (82.52,-34.55) size 51.83x69.11 LayoutBlockFlow (positioned) zI: 2 {DIV} at (82.52,-34.55) size 51.83x69.11
third_party/WebKit/LayoutTests/scrollbars/sub-pixel-overflow.html 0 → 100644
View file @ 7f73e60a
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
#scroller {
border-width: 0 1px;
border-style:solid;
margin-left: 26px;
overflow: auto
}
</style>
<div id="scroller-container">
<div id="scroller">
<div id="content">Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</div>
</div>
</div>
<script>
if (window.internals) {
internals.setZoomFactor(1.1);
}
test(() => {
let container = document.getElementById('scroller-container');
let scroller = document.getElementById('scroller');
assert_equals(container.clientHeight, scroller.clientHeight);
});
</script>
third_party/WebKit/Source/core/paint/PaintLayer.cpp
View file @ 7f73e60a
...@@ -864,16 +864,16 @@ void PaintLayer::UpdateLayerPosition() { ...@@ -864,16 +864,16 @@ void PaintLayer::UpdateLayerPosition() {
} }
bool PaintLayer::UpdateSize() { bool PaintLayer::UpdateSize() {
IntSize old_size = size_; LayoutSize old_size = size_;
if (IsRootLayer() && RuntimeEnabledFeatures::RootLayerScrollingEnabled()) { if (IsRootLayer() && RuntimeEnabledFeatures::RootLayerScrollingEnabled()) {
size_ = GetLayoutObject().GetDocument().View()->Size(); size_ = LayoutSize(GetLayoutObject().GetDocument().View()->Size());
} else if (GetLayoutObject().IsInline() && } else if (GetLayoutObject().IsInline() &&
GetLayoutObject().IsLayoutInline()) { GetLayoutObject().IsLayoutInline()) {
LayoutInline& inline_flow = ToLayoutInline(GetLayoutObject()); LayoutInline& inline_flow = ToLayoutInline(GetLayoutObject());
IntRect line_box = EnclosingIntRect(inline_flow.LinesBoundingBox()); IntRect line_box = EnclosingIntRect(inline_flow.LinesBoundingBox());
size_ = line_box.Size(); size_ = LayoutSize(line_box.Size());
} else if (LayoutBox* box = GetLayoutBox()) { } else if (LayoutBox* box = GetLayoutBox()) {
size_ = PixelSnappedIntSize(box->Size(), box->Location()); size_ = box->Size();
} }
return old_size != size_; return old_size != size_;
} }
......
third_party/WebKit/Source/core/paint/PaintLayer.h
View file @ 7f73e60a
...@@ -278,15 +278,21 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient { ...@@ -278,15 +278,21 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient {
// FIXME: size() should DCHECK(!needs_position_update_) as well, but that // FIXME: size() should DCHECK(!needs_position_update_) as well, but that
// fails in some tests, for example, fast/repaint/clipped-relative.html. // fails in some tests, for example, fast/repaint/clipped-relative.html.
const IntSize& Size() const { return size_; } const LayoutSize& Size() const { return size_; }
IntSize PixelSnappedSize() const {
LayoutPoint location = layout_object_.IsBox()
? ToLayoutBox(layout_object_).Location()
: LayoutPoint();
return PixelSnappedIntSize(Size(), location);
}
void SetSizeHackForLayoutTreeAsText(const IntSize& size) { size_ = size; } void SetSizeHackForLayoutTreeAsText(const LayoutSize& size) { size_ = size; }
LayoutRect Rect() const { return LayoutRect(Location(), LayoutSize(Size())); } LayoutRect Rect() const { return LayoutRect(Location(), Size()); }
// For LayoutTreeAsText // For LayoutTreeAsText
LayoutRect RectIgnoringNeedsPositionUpdate() const { LayoutRect RectIgnoringNeedsPositionUpdate() const {
return LayoutRect(location_, LayoutSize(size_)); return LayoutRect(location_, size_);
} }
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
bool NeedsPositionUpdate() const { return needs_position_update_; } bool NeedsPositionUpdate() const { return needs_position_update_; }
...@@ -1274,7 +1280,7 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient { ...@@ -1274,7 +1280,7 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient {
// //
// If the associated LayoutBoxModelObject is a LayoutBox, it's its border // If the associated LayoutBoxModelObject is a LayoutBox, it's its border
// box. Otherwise, this is the LayoutInline's lines' bounding box. // box. Otherwise, this is the LayoutInline's lines' bounding box.
IntSize size_; LayoutSize size_;
// Cached normal flow values for absolute positioned elements with static // Cached normal flow values for absolute positioned elements with static
// left/top values. // left/top values.
......
third_party/WebKit/Source/core/paint/PaintLayerClipper.cpp
View file @ 7f73e60a
...@@ -299,7 +299,7 @@ void PaintLayerClipper::CalculateRectsWithGeometryMapper( ...@@ -299,7 +299,7 @@ void PaintLayerClipper::CalculateRectsWithGeometryMapper(
layer_.ConvertToLayerCoords(context.root_layer, offset); layer_.ConvertToLayerCoords(context.root_layer, offset);
} }
} }
layer_bounds = LayoutRect(offset, LayoutSize(layer_.Size())); layer_bounds = LayoutRect(offset, LayoutSize(layer_.PixelSnappedSize()));
CalculateBackgroundClipRectWithGeometryMapper(context, fragment_data, CalculateBackgroundClipRectWithGeometryMapper(context, fragment_data,
background_rect); background_rect);
...@@ -360,7 +360,7 @@ void PaintLayerClipper::CalculateRects( ...@@ -360,7 +360,7 @@ void PaintLayerClipper::CalculateRects(
offset = *offset_from_root; offset = *offset_from_root;
else else
layer_.ConvertToLayerCoords(context.root_layer, offset); layer_.ConvertToLayerCoords(context.root_layer, offset);
layer_bounds = LayoutRect(offset, LayoutSize(layer_.Size())); layer_bounds = LayoutRect(offset, LayoutSize(layer_.PixelSnappedSize()));
// Update the clip rects that will be passed to child layers. // Update the clip rects that will be passed to child layers.
if (ShouldClipOverflow(context)) { if (ShouldClipOverflow(context)) {
......
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
View file @ 7f73e60a
...@@ -603,7 +603,7 @@ void PaintLayerScrollableArea::VisibleSizeChanged() { ...@@ -603,7 +603,7 @@ void PaintLayerScrollableArea::VisibleSizeChanged() {
LayoutRect PaintLayerScrollableArea::LayoutContentRect( LayoutRect PaintLayerScrollableArea::LayoutContentRect(
IncludeScrollbarsInRect scrollbar_inclusion) const { IncludeScrollbarsInRect scrollbar_inclusion) const {
// LayoutContentRect is conceptually the same as the box's client rect. // LayoutContentRect is conceptually the same as the box's client rect.
LayoutSize layer_size = LayoutSize(Layer()->Size()); LayoutSize layer_size(Layer()->Size());
LayoutUnit border_width = Box().BorderWidth(); LayoutUnit border_width = Box().BorderWidth();
LayoutUnit border_height = Box().BorderHeight(); LayoutUnit border_height = Box().BorderHeight();
LayoutUnit horizontal_scrollbar_height, vertical_scrollbar_width; LayoutUnit horizontal_scrollbar_height, vertical_scrollbar_width;
...@@ -631,8 +631,9 @@ IntRect PaintLayerScrollableArea::VisibleContentRect( ...@@ -631,8 +631,9 @@ IntRect PaintLayerScrollableArea::VisibleContentRect(
LayoutRect layout_content_rect(LayoutContentRect(scrollbar_inclusion)); LayoutRect layout_content_rect(LayoutContentRect(scrollbar_inclusion));
// TODO(szager): It's not clear that Floor() is the right thing to do here; // TODO(szager): It's not clear that Floor() is the right thing to do here;
// what is the correct behavior for fractional scroll offsets? // what is the correct behavior for fractional scroll offsets?
return IntRect(FlooredIntPoint(layout_content_rect.Location()), return IntRect(
RoundedIntSize(layout_content_rect.Size())); FlooredIntPoint(layout_content_rect.Location()),
PixelSnappedIntSize(layout_content_rect.Size(), Box().Location()));
} }
IntSize PaintLayerScrollableArea::ContentsSize() const { IntSize PaintLayerScrollableArea::ContentsSize() const {
...@@ -1215,8 +1216,9 @@ int PaintLayerScrollableArea::HorizontalScrollbarStart(int min_x) const { ...@@ -1215,8 +1216,9 @@ int PaintLayerScrollableArea::HorizontalScrollbarStart(int min_x) const {
IntSize PaintLayerScrollableArea::ScrollbarOffset( IntSize PaintLayerScrollableArea::ScrollbarOffset(
const Scrollbar& scrollbar) const { const Scrollbar& scrollbar) const {
if (&scrollbar == VerticalScrollbar()) { if (&scrollbar == VerticalScrollbar()) {
return IntSize(VerticalScrollbarStart(0, Layer()->Size().Width()), return IntSize(
Box().BorderTop().ToInt()); VerticalScrollbarStart(0, Layer()->PixelSnappedSize().Width()),
Box().BorderTop().ToInt());
} }
if (&scrollbar == HorizontalScrollbar()) { if (&scrollbar == HorizontalScrollbar()) {
...@@ -1556,13 +1558,13 @@ bool PaintLayerScrollableArea::HitTestOverflowControls( ...@@ -1556,13 +1558,13 @@ bool PaintLayerScrollableArea::HitTestOverflowControls(
int resize_control_size = max(resize_control_rect.Height(), 0); int resize_control_size = max(resize_control_rect.Height(), 0);
if (HasVerticalScrollbar() && if (HasVerticalScrollbar() &&
VerticalScrollbar()->ShouldParticipateInHitTesting()) { VerticalScrollbar()->ShouldParticipateInHitTesting()) {
LayoutRect v_bar_rect(VerticalScrollbarStart(0, Layer()->Size().Width()), LayoutRect v_bar_rect(
Box().BorderTop().ToInt(), VerticalScrollbarStart(0, Layer()->PixelSnappedSize().Width()),
VerticalScrollbar()->ScrollbarThickness(), Box().BorderTop().ToInt(), VerticalScrollbar()->ScrollbarThickness(),
VisibleContentRect(kIncludeScrollbars).Height() - VisibleContentRect(kIncludeScrollbars).Height() -
(HasHorizontalScrollbar() (HasHorizontalScrollbar()
? HorizontalScrollbar()->ScrollbarThickness() ? HorizontalScrollbar()->ScrollbarThickness()
: resize_control_size)); : resize_control_size));
if (v_bar_rect.Contains(local_point)) { if (v_bar_rect.Contains(local_point)) {
result.SetScrollbar(VerticalScrollbar()); result.SetScrollbar(VerticalScrollbar());
return true; return true;
...@@ -1632,7 +1634,7 @@ bool PaintLayerScrollableArea::IsPointInResizeControl( ...@@ -1632,7 +1634,7 @@ bool PaintLayerScrollableArea::IsPointInResizeControl(
IntPoint local_point = IntPoint local_point =
RoundedIntPoint(Box().AbsoluteToLocal(absolute_point, kUseTransforms)); RoundedIntPoint(Box().AbsoluteToLocal(absolute_point, kUseTransforms));
IntRect local_bounds(IntPoint(), Layer()->Size()); IntRect local_bounds(IntPoint(), Layer()->PixelSnappedSize());
return ResizerCornerRect(local_bounds, resizer_hit_test_type) return ResizerCornerRect(local_bounds, resizer_hit_test_type)
.Contains(local_point); .Contains(local_point);
} }
...@@ -1722,7 +1724,7 @@ IntSize PaintLayerScrollableArea::OffsetFromResizeCorner( ...@@ -1722,7 +1724,7 @@ IntSize PaintLayerScrollableArea::OffsetFromResizeCorner(
// left corner. // left corner.
// FIXME: This assumes the location is 0, 0. Is this guaranteed to always be // FIXME: This assumes the location is 0, 0. Is this guaranteed to always be
// the case? // the case?
IntSize element_size = Layer()->Size(); IntSize element_size = Layer()->PixelSnappedSize();
if (Box().ShouldPlaceBlockDirectionScrollbarOnLogicalLeft()) if (Box().ShouldPlaceBlockDirectionScrollbarOnLogicalLeft())
element_size.SetWidth(0); element_size.SetWidth(0);
IntPoint resizer_point = IntPoint(element_size); IntPoint resizer_point = IntPoint(element_size);
......
third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp
View file @ 7f73e60a
...@@ -172,7 +172,8 @@ void ScrollableAreaPainter::PaintOverflowControls( ...@@ -172,7 +172,8 @@ void ScrollableAreaPainter::PaintOverflowControls(
if (painting_overlay_controls && !GetScrollableArea().HasOverlayScrollbars()) if (painting_overlay_controls && !GetScrollableArea().HasOverlayScrollbars())
return; return;
IntRect clip_rect(adjusted_paint_offset, GetScrollableArea().Layer()->Size()); IntRect clip_rect(adjusted_paint_offset,
GetScrollableArea().Layer()->PixelSnappedSize());
ClipRecorder clip_recorder(context, GetScrollableArea().Box(), ClipRecorder clip_recorder(context, GetScrollableArea().Box(),
DisplayItem::kClipLayerOverflowControls, DisplayItem::kClipLayerOverflowControls,
clip_rect); clip_rect);
......
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