Commit 6faf0595 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Remove IntRect(const &FloatRect) and IntRect(const &LayoutRect)

They simply truncated x, y, width and height to integer, which may not
be what we want and may hide some errors.

The usages of them may be in the following cases:
1. The input rect is already in integral geometry, so all methods of
   IntRect conversion will get the same result;
2. The IntRect is just used for informative purpose so differences of
   1 to 2 pixels at the edges don't matter.

We should use EnclosingIntRect(), EnclosedIntRect(), etc. to explicitly
specify the way to convert.

Convert the usages of them to
- EnclosingIntRect if we are getting a visual rect or a rect that needs
  to cover the input rect;
- Floored size for page size when printing to keep the original behavior.
- PixelSnappedIntRect or RoundedIntRect for others.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ifc81334e39db105d32a046cfa868baa35340a479
Reviewed-on: https://chromium-review.googlesource.com/949571
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542534}
parent 459941db
......@@ -111,7 +111,7 @@ class DraggedNodeImageBuilder {
// If the absolute bounding box is large enough to be possibly a memory
// or IPC payload issue, clip it to the visible content rect.
if (absolute_bounding_box.Size().Area() > visible_rect.Size().Area()) {
absolute_bounding_box.Intersect(IntRect(visible_rect));
absolute_bounding_box.Intersect(EnclosingIntRect(visible_rect));
}
FloatRect bounding_box =
......@@ -366,7 +366,7 @@ void DataTransfer::SetDragImageElement(Node* node, const IntPoint& loc) {
FloatRect DataTransfer::ClipByVisualViewport(const FloatRect& absolute_rect,
const LocalFrame& frame) {
IntRect viewport_in_root_frame =
IntRect(frame.GetPage()->GetVisualViewport().VisibleRect());
EnclosingIntRect(frame.GetPage()->GetVisualViewport().VisibleRect());
FloatRect absolute_viewport =
frame.View()->RootFrameToAbsolute(viewport_in_root_frame);
return Intersection(absolute_viewport, absolute_rect);
......
......@@ -90,7 +90,7 @@ Vector<IntRect> TextMatchMarkerListImpl::LayoutRects(const Node& node) const {
UpdateMarkerLayoutRect(node, *text_match_marker);
if (!text_match_marker->IsRendered())
continue;
result.push_back(text_match_marker->GetLayoutRect());
result.push_back(PixelSnappedIntRect(text_match_marker->GetLayoutRect()));
}
return result;
......
......@@ -240,7 +240,7 @@ WebVector<WebDraggableRegion> WebDocument::DraggableRegions() const {
for (size_t i = 0; i < regions.size(); i++) {
const AnnotatedRegionValue& value = regions[i];
draggable_regions[i].draggable = value.draggable;
draggable_regions[i].bounds = IntRect(value.bounds);
draggable_regions[i].bounds = PixelSnappedIntRect(value.bounds);
}
}
return draggable_regions;
......
......@@ -84,7 +84,7 @@ void RemoteFrameView::UpdateViewportIntersectionsForSubtree(
if (remote_frame_->OwnerLayoutObject()->MapToVisualRectInAncestorSpace(
nullptr, rect)) {
IntRect root_visible_rect = local_root_view->VisibleContentRect();
IntRect intersected_rect(rect);
IntRect intersected_rect = EnclosingIntRect(rect);
intersected_rect.Intersect(root_visible_rect);
intersected_rect.Move(-local_root_view->ScrollOffsetInt());
......
......@@ -656,7 +656,7 @@ IntRect VisualViewport::VisibleContentRect(
IncludeScrollbarsInRect scrollbar_inclusion) const {
// TODO(ymalik): We're losing precision here and below. visibleRect should
// be replaced with visibleContentRect.
IntRect rect = IntRect(VisibleRect());
IntRect rect = EnclosingIntRect(VisibleRect());
if (scrollbar_inclusion == kExcludeScrollbars) {
RootFrameViewport* root_frame_viewport =
MainFrame()->View()->GetRootFrameViewport();
......
......@@ -474,7 +474,7 @@ class ChromePluginPrintContext final : public ChromePrintContext {
}
void ComputePageRects(const FloatSize& print_size) override {
IntRect rect(FloatRect(FloatPoint(0, 0), print_size));
IntRect rect(IntPoint(0, 0), FlooredIntSize(print_size));
print_params_.print_content_area = rect;
page_rects_.Fill(rect, plugin_->PrintBegin(print_params_));
}
......@@ -1595,8 +1595,8 @@ void WebLocalFrameImpl::PrintPagesForTesting(
WebRect WebLocalFrameImpl::GetSelectionBoundsRectForTesting() const {
return HasSelection()
? WebRect(
IntRect(GetFrame()->Selection().AbsoluteUnclippedBounds()))
? WebRect(PixelSnappedIntRect(
GetFrame()->Selection().AbsoluteUnclippedBounds()))
: WebRect();
}
......
......@@ -65,7 +65,7 @@ void ViewPainter::PaintBoxDecorationBackground(const PaintInfo& paint_info) {
// The background rect always includes at least the visible content size.
IntRect background_rect(
IntRect(layout_view_.OverflowClipRect(LayoutPoint())));
PixelSnappedIntRect(layout_view_.OverflowClipRect(LayoutPoint())));
// When printing with root layer scrolling, we will paint the entire
// unclipped scrolling content area.
......
......@@ -226,8 +226,8 @@ IntRect EnclosedIntRect(const FloatRect& rect) {
FloatPoint max_point(floorf(rect.MaxX()), floorf(rect.MaxY()));
FloatSize size = max_point - location;
size.ClampNegativeToZero();
FloatRect enclosed_rect(location, size);
return IntRect(enclosed_rect);
return IntRect(clampTo<int>(location.X()), clampTo<int>(location.Y()),
clampTo<int>(size.Width()), clampTo<int>(size.Height()));
}
IntRect RoundedIntRect(const FloatRect& rect) {
......
......@@ -37,14 +37,6 @@
namespace blink {
IntRect::IntRect(const FloatRect& r)
: location_(clampTo<int>(r.X()), clampTo<int>(r.Y())),
size_(clampTo<int>(r.Width()), clampTo<int>(r.Height())) {}
IntRect::IntRect(const LayoutRect& r)
: location_(r.X().ToInt(), r.Y().ToInt()),
size_(r.Width().ToInt(), r.Height().ToInt()) {}
void IntRect::ShiftXEdgeTo(int edge) {
int delta = edge - X();
SetX(edge);
......
......@@ -63,10 +63,10 @@ class PLATFORM_EXPORT IntRect {
IntRect(int x, int y, int width, int height)
: location_(IntPoint(x, y)), size_(IntSize(width, height)) {}
explicit IntRect(
const FloatRect&); // don't do this implicitly since it's lossy
explicit IntRect(
const LayoutRect&); // don't do this implicitly since it's lossy
// Use EnclosingIntRect(), EnclosedIntRect(), RoundedIntRect(),
// PixelSnappedIntRect(), etc. instead.
explicit IntRect(const FloatRect&) = delete;
explicit IntRect(const LayoutRect&) = delete;
IntPoint Location() const { return location_; }
IntSize Size() const { return size_; }
......
......@@ -64,10 +64,9 @@ class PLATFORM_EXPORT LayoutRect {
explicit LayoutRect(const IntRect& rect)
: location_(rect.Location()), size_(rect.Size()) {}
explicit LayoutRect(
const FloatRect&); // don't do this implicitly since it's lossy
explicit LayoutRect(
const DoubleRect&); // don't do this implicitly since it's lossy
// Don't do these implicitly since they are lossy.
explicit LayoutRect(const FloatRect&);
explicit LayoutRect(const DoubleRect&);
LayoutPoint Location() const { return location_; }
LayoutSize Size() const { return size_; }
......@@ -236,9 +235,10 @@ class PLATFORM_EXPORT LayoutRect {
static IntRect InfiniteIntRect() {
// Due to saturated arithemetic this value is not the same as
// LayoutRect(IntRect(INT_MIN/2, INT_MIN/2, INT_MAX, INT_MAX)).
static IntRect infinite_int_rect(
LayoutRect(LayoutUnit::NearlyMin() / 2, LayoutUnit::NearlyMin() / 2,
LayoutUnit::NearlyMax(), LayoutUnit::NearlyMax()));
static IntRect infinite_int_rect(LayoutUnit::NearlyMin().ToInt() / 2,
LayoutUnit::NearlyMin().ToInt() / 2,
LayoutUnit::NearlyMax().ToInt(),
LayoutUnit::NearlyMax().ToInt());
return infinite_int_rect;
}
......@@ -292,6 +292,12 @@ inline IntRect EnclosingIntRect(const LayoutRect& rect) {
return IntRect(location, max_point - location);
}
inline IntRect EnclosedIntRect(const LayoutRect& rect) {
IntPoint location = CeiledIntPoint(rect.MinXMinYCorner());
IntPoint max_point = FlooredIntPoint(rect.MaxXMaxYCorner());
return IntRect(location, max_point - location);
}
PLATFORM_EXPORT LayoutRect EnclosingLayoutRect(const FloatRect&);
inline IntRect PixelSnappedIntRect(LayoutUnit left,
......
......@@ -41,4 +41,103 @@ TEST(LayoutRectTest, InclusiveIntersect) {
}
#endif
TEST(LayoutRectTest, EnclosingIntRect) {
LayoutUnit small;
small.SetRawValue(1);
LayoutRect small_dimensions_rect(LayoutUnit(42.5f), LayoutUnit(84.5f), small,
small);
EXPECT_EQ(IntRect(42, 84, 1, 1), EnclosingIntRect(small_dimensions_rect));
LayoutRect integral_rect(IntRect(100, 150, 200, 350));
EXPECT_EQ(IntRect(100, 150, 200, 350), EnclosingIntRect(integral_rect));
LayoutRect fractional_pos_rect(LayoutUnit(100.6f), LayoutUnit(150.8f),
LayoutUnit(200), LayoutUnit(350));
EXPECT_EQ(IntRect(100, 150, 201, 351), EnclosingIntRect(fractional_pos_rect));
LayoutRect fractional_dimensions_rect(LayoutUnit(100), LayoutUnit(150),
LayoutUnit(200.6f), LayoutUnit(350.4f));
EXPECT_EQ(IntRect(100, 150, 201, 351),
EnclosingIntRect(fractional_dimensions_rect));
LayoutRect fractional_both_rect1(LayoutUnit(100.6f), LayoutUnit(150.8f),
LayoutUnit(200.4f), LayoutUnit(350.2f));
EXPECT_EQ(IntRect(100, 150, 201, 351),
EnclosingIntRect(fractional_both_rect1));
LayoutRect fractional_both_rect2(LayoutUnit(100.5f), LayoutUnit(150.7f),
LayoutUnit(200.3f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(100, 150, 201, 351),
EnclosingIntRect(fractional_both_rect2));
LayoutRect fractional_both_rect3(LayoutUnit(100.3f), LayoutUnit(150.2f),
LayoutUnit(200.8f), LayoutUnit(350.9f));
EXPECT_EQ(IntRect(100, 150, 202, 352),
EnclosingIntRect(fractional_both_rect3));
LayoutRect fractional_negpos_rect1(LayoutUnit(-100.4f), LayoutUnit(-150.8f),
LayoutUnit(200), LayoutUnit(350));
EXPECT_EQ(IntRect(-101, -151, 201, 351),
EnclosingIntRect(fractional_negpos_rect1));
LayoutRect fractional_negpos_rect2(LayoutUnit(-100.5f), LayoutUnit(-150.7f),
LayoutUnit(199.4f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(-101, -151, 200, 351),
EnclosingIntRect(fractional_negpos_rect2));
LayoutRect fractional_negpos_rect3(LayoutUnit(-100.3f), LayoutUnit(-150.2f),
LayoutUnit(199.6f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(-101, -151, 201, 352),
EnclosingIntRect(fractional_negpos_rect3));
}
TEST(LayoutRectTest, EnclosedIntRect) {
LayoutUnit small;
small.SetRawValue(1);
LayoutRect small_dimensions_rect(LayoutUnit(42.5f), LayoutUnit(84.5f), small,
small);
LayoutRect integral_rect(IntRect(100, 150, 200, 350));
EXPECT_EQ(IntRect(100, 150, 200, 350), EnclosedIntRect(integral_rect));
LayoutRect fractional_pos_rect(LayoutUnit(100.6f), LayoutUnit(150.8f),
LayoutUnit(200), LayoutUnit(350));
EXPECT_EQ(IntRect(101, 151, 199, 349), EnclosedIntRect(fractional_pos_rect));
LayoutRect fractional_dimensions_rect(LayoutUnit(100), LayoutUnit(150),
LayoutUnit(200.6f), LayoutUnit(350.4f));
EXPECT_EQ(IntRect(100, 150, 200, 350),
EnclosedIntRect(fractional_dimensions_rect));
LayoutRect fractional_both_rect1(LayoutUnit(100.6f), LayoutUnit(150.8f),
LayoutUnit(200.4f), LayoutUnit(350.2f));
EXPECT_EQ(IntRect(101, 151, 199, 349),
EnclosedIntRect(fractional_both_rect1));
LayoutRect fractional_both_rect2(LayoutUnit(100.5f), LayoutUnit(150.7f),
LayoutUnit(200.3f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(101, 151, 199, 349),
EnclosedIntRect(fractional_both_rect2));
LayoutRect fractional_both_rect3(LayoutUnit(100.3f), LayoutUnit(150.2f),
LayoutUnit(200.6f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(101, 151, 199, 349),
EnclosedIntRect(fractional_both_rect3));
LayoutRect fractional_negpos_rect1(LayoutUnit(-100.4f), LayoutUnit(-150.8f),
LayoutUnit(200), LayoutUnit(350));
EXPECT_EQ(IntRect(-100, -150, 199, 349),
EnclosedIntRect(fractional_negpos_rect1));
LayoutRect fractional_negpos_rect2(LayoutUnit(-100.5f), LayoutUnit(-150.7f),
LayoutUnit(199.4f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(-100, -150, 198, 349),
EnclosedIntRect(fractional_negpos_rect2));
LayoutRect fractional_negpos_rect3(LayoutUnit(-100.3f), LayoutUnit(-150.2f),
LayoutUnit(199.6f), LayoutUnit(350.3f));
EXPECT_EQ(IntRect(-100, -150, 199, 350),
EnclosedIntRect(fractional_negpos_rect3));
}
} // namespace blink
......@@ -113,8 +113,6 @@ class PLATFORM_EXPORT ScrollAnimatorMac : public ScrollAnimatorBase {
void SendContentAreaScrolledSoon(const ScrollOffset& scroll_delta);
void SetVisibleScrollerThumbRect(const IntRect&);
virtual void Trace(blink::Visitor* visitor) {
ScrollAnimatorBase::Trace(visitor);
}
......@@ -173,7 +171,6 @@ class PLATFORM_EXPORT ScrollAnimatorMac : public ScrollAnimatorBase {
bool have_scrolled_since_page_load_;
bool needs_scroller_style_update_;
IntRect visible_scroller_thumb_rect_;
};
} // namespace blink
......
......@@ -537,15 +537,6 @@ class BlinkScrollbarPartAnimationTimer {
scrollbarPartAnimation = nullptr;
}
if (part == blink::kThumbPart &&
_scrollbar->Orientation() == blink::kVerticalScrollbar) {
if (newAlpha == 1) {
blink::IntRect thumbRect([scrollerPainter rectForPart:NSScrollerKnob]);
[self scrollAnimator].SetVisibleScrollerThumbRect(thumbRect);
} else
[self scrollAnimator].SetVisibleScrollerThumbRect(blink::IntRect());
}
scrollbarPartAnimation.AdoptNS([[BlinkScrollbarPartAnimation alloc]
initWithScrollbar:_scrollbar
featureToAnimate:part == blink::kThumbPart ? ThumbAlpha : TrackAlpha
......@@ -1107,18 +1098,4 @@ void ScrollAnimatorMac::SendContentAreaScrolledTask() {
[scrollbar_painter_controller_.Get() contentAreaScrolled];
}
void ScrollAnimatorMac::SetVisibleScrollerThumbRect(
const IntRect& scroller_thumb) {
IntRect rect_in_view_coordinates = scroller_thumb;
if (Scrollbar* vertical_scrollbar = scrollable_area_->VerticalScrollbar())
rect_in_view_coordinates =
vertical_scrollbar->ConvertToContainingEmbeddedContentView(
scroller_thumb);
if (rect_in_view_coordinates == visible_scroller_thumb_rect_)
return;
visible_scroller_thumb_rect_ = rect_in_view_coordinates;
}
} // namespace blink
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