Skip to content

Commit

Permalink
[BGPT] Stop caching position_ on GraphicsLayer and instead use the va…
Browse files Browse the repository at this point in the history
…lue on cc::Layer directly.

This improves BGPT testing to take into account layer position updates due to PAC, and also
reduces GraphicsLayer size a little bit. It exposes gfx::PointF directly to some code in core,
but that is ok because we already decided that gfx types are ok in core and this is just for
SPv1 compositing.

Bug: 883949

Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6c0423aa66e94c24e96331381e6523ba1fb574b6
Reviewed-on: https://chromium-review.googlesource.com/1227301
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591514}
  • Loading branch information
chrishtr authored and Commit Bot committed Sep 14, 2018
1 parent b39dc79 commit aaa8368
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 41 deletions.
8 changes: 4 additions & 4 deletions third_party/blink/renderer/core/frame/visual_viewport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ void VisualViewport::UpdatePaintPropertyNodesIfNeeded(
overlay_scrollbar_horizontal_->SetLayerState(
PropertyTreeState(transform_parent, context.current.clip,
horizontal_scrollbar_effect_node_.get()),
IntPoint(overlay_scrollbar_horizontal_->GetPosition().X(),
overlay_scrollbar_horizontal_->GetPosition().Y()));
IntPoint(overlay_scrollbar_horizontal_->GetPosition().x(),
overlay_scrollbar_horizontal_->GetPosition().y()));
}

if (overlay_scrollbar_vertical_) {
Expand All @@ -242,8 +242,8 @@ void VisualViewport::UpdatePaintPropertyNodesIfNeeded(
overlay_scrollbar_vertical_->SetLayerState(
PropertyTreeState(transform_parent, context.current.clip,
vertical_scrollbar_effect_node_.get()),
IntPoint(overlay_scrollbar_vertical_->GetPosition().X(),
overlay_scrollbar_vertical_->GetPosition().Y()));
IntPoint(overlay_scrollbar_vertical_->GetPosition().x(),
overlay_scrollbar_vertical_->GetPosition().y()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,9 +561,10 @@ GraphicsLayer* CompositedLayerMapping::FrameContentsGraphicsLayer() const {
void CompositedLayerMapping::UpdateAfterPartResize() {
if (GetLayoutObject().IsLayoutEmbeddedContent()) {
if (GraphicsLayer* document_layer = FrameContentsGraphicsLayer()) {
FloatPoint parent_position = child_containment_layer_
? child_containment_layer_->GetPosition()
: FloatPoint();
FloatPoint parent_position =
child_containment_layer_
? FloatPoint(child_containment_layer_->GetPosition())
: FloatPoint();
document_layer->SetPosition(FloatPoint(FlooredIntPoint(
FloatPoint(ContentsBox().Location()) - parent_position)));
}
Expand Down Expand Up @@ -1316,7 +1317,7 @@ void CompositedLayerMapping::UpdateMainGraphicsLayerGeometry(
const IntRect& relative_compositing_bounds,
const IntRect& local_compositing_bounds,
const IntPoint& graphics_layer_parent_location) {
FloatPoint old_position = graphics_layer_->GetPosition();
FloatPoint old_position(graphics_layer_->GetPosition());
IntSize old_size = graphics_layer_->Size();
FloatPoint new_position = FloatPoint(relative_compositing_bounds.Location() -
graphics_layer_parent_location);
Expand Down Expand Up @@ -1512,7 +1513,7 @@ void CompositedLayerMapping::UpdateOverflowControlsHostLayerGeometry(

FloatPoint position;
if (compositing_stacking_context == compositing_container) {
position = ancestor_clipping_layer_->GetPosition();
position = FloatPoint(ancestor_clipping_layer_->GetPosition());
} else {
// graphicsLayerParentLocation is the location of
// m_ancestorClippingLayer relative to compositingContainer (including
Expand Down Expand Up @@ -1691,7 +1692,8 @@ void CompositedLayerMapping::UpdateScrollingLayerGeometry(
ToIntSize(overflow_clip_rect.Location()));

if (child_clipping_mask_layer_ && !GetLayoutObject().StyleRef().ClipPath()) {
child_clipping_mask_layer_->SetPosition(scrolling_layer_->GetPosition());
child_clipping_mask_layer_->SetPosition(
FloatPoint(scrolling_layer_->GetPosition()));
if (child_clipping_mask_layer_->Size() != scrolling_layer_->Size()) {
child_clipping_mask_layer_->SetSize(scrolling_layer_->Size());
child_clipping_mask_layer_->SetNeedsDisplay();
Expand Down Expand Up @@ -1736,7 +1738,8 @@ void CompositedLayerMapping::UpdateChildClippingMaskLayerGeometry() {
LayoutBox& layout_box = ToLayoutBox(GetLayoutObject());
IntRect padding_box = EnclosingIntRect(layout_box.PhysicalPaddingBoxRect());

child_clipping_mask_layer_->SetPosition(graphics_layer_->GetPosition());
child_clipping_mask_layer_->SetPosition(
FloatPoint(graphics_layer_->GetPosition()));
if (child_clipping_mask_layer_->Size() != graphics_layer_->Size()) {
child_clipping_mask_layer_->SetSize(graphics_layer_->Size());
child_clipping_mask_layer_->SetNeedsDisplay();
Expand Down Expand Up @@ -1955,7 +1958,8 @@ void CompositedLayerMapping::UpdateContentsOffsetInCompositingLayer(
// accumulation of this CLM already (through its own
// graphicsLayerParentLocation it appears).
FloatPoint offset_due_to_ancestor_graphics_layers =
graphics_layer_->GetPosition() + graphics_layer_parent_location;
FloatPoint(graphics_layer_->GetPosition()) +
graphics_layer_parent_location;
content_offset_in_compositing_layer_ =
LayoutSize(snapped_offset_from_composited_ancestor -
offset_due_to_ancestor_graphics_layers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1230,12 +1230,12 @@ TEST_F(CompositedLayerMappingTest,
ASSERT_TRUE(scrolling_layer);
ASSERT_TRUE(child_transform_layer);

EXPECT_FLOAT_EQ(30, main_graphics_layer->GetPosition().X());
EXPECT_FLOAT_EQ(40, main_graphics_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(0, scrolling_layer->GetPosition().X());
EXPECT_FLOAT_EQ(0, scrolling_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(20, child_transform_layer->GetPosition().X());
EXPECT_FLOAT_EQ(10, child_transform_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(30, main_graphics_layer->GetPosition().x());
EXPECT_FLOAT_EQ(40, main_graphics_layer->GetPosition().y());
EXPECT_FLOAT_EQ(0, scrolling_layer->GetPosition().x());
EXPECT_FLOAT_EQ(0, scrolling_layer->GetPosition().y());
EXPECT_FLOAT_EQ(20, child_transform_layer->GetPosition().x());
EXPECT_FLOAT_EQ(10, child_transform_layer->GetPosition().y());

// The non-perspective scroller should have no child transform and the
// offset on the scroller layer directly.
Expand All @@ -1246,10 +1246,10 @@ TEST_F(CompositedLayerMappingTest,
ASSERT_TRUE(scrolling_layer2);
ASSERT_FALSE(mapping2->ChildTransformLayer());

EXPECT_FLOAT_EQ(30, main_graphics_layer2->GetPosition().X());
EXPECT_FLOAT_EQ(390, main_graphics_layer2->GetPosition().Y());
EXPECT_FLOAT_EQ(20, scrolling_layer2->GetPosition().X());
EXPECT_FLOAT_EQ(10, scrolling_layer2->GetPosition().Y());
EXPECT_FLOAT_EQ(30, main_graphics_layer2->GetPosition().x());
EXPECT_FLOAT_EQ(390, main_graphics_layer2->GetPosition().y());
EXPECT_FLOAT_EQ(20, scrolling_layer2->GetPosition().x());
EXPECT_FLOAT_EQ(10, scrolling_layer2->GetPosition().y());
}

TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerUpdates) {
Expand Down Expand Up @@ -2285,13 +2285,13 @@ TEST_F(CompositedLayerMappingTest,

// On the CompositedLayerMapping side however, the offset should have been
// removed so that the compositor can take care of it.
EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().X());
EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().x());
EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().y());

// The child transform layer for the perspective shifting should also not be
// moved by the sticky offset.
EXPECT_FLOAT_EQ(0, child_transform_layer->GetPosition().X());
EXPECT_FLOAT_EQ(0, child_transform_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(0, child_transform_layer->GetPosition().x());
EXPECT_FLOAT_EQ(0, child_transform_layer->GetPosition().y());
}

TEST_F(CompositedLayerMappingTest,
Expand Down Expand Up @@ -2325,8 +2325,8 @@ TEST_F(CompositedLayerMappingTest,
FloatPoint(scrollable_area->ScrollPosition().Y(), 100));
GetDocument().View()->UpdateAllLifecyclePhases();

EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().X());
EXPECT_FLOAT_EQ(100, main_graphics_layer->GetPosition().Y());
EXPECT_FLOAT_EQ(0, main_graphics_layer->GetPosition().x());
EXPECT_FLOAT_EQ(100, main_graphics_layer->GetPosition().y());
}

TEST_F(CompositedLayerMappingTest,
Expand Down Expand Up @@ -2561,10 +2561,10 @@ TEST_F(CompositedLayerMappingTest, ForegroundLayerSizing) {
ASSERT_TRUE(mapping);
EXPECT_EQ(IntSize(120, 120), mapping->MainGraphicsLayer()->Size());
ASSERT_TRUE(mapping->ClippingLayer());
EXPECT_EQ(FloatPoint(10, 10), mapping->ClippingLayer()->GetPosition());
EXPECT_EQ(gfx::PointF(10, 10), mapping->ClippingLayer()->GetPosition());
EXPECT_EQ(IntSize(100, 100), mapping->ClippingLayer()->Size());
ASSERT_TRUE(mapping->ForegroundLayer());
EXPECT_EQ(FloatPoint(0, 0), mapping->ForegroundLayer()->GetPosition());
EXPECT_EQ(gfx::PointF(0, 0), mapping->ForegroundLayer()->GetPosition());
EXPECT_EQ(IntSize(100, 100), mapping->ForegroundLayer()->Size());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ std::unique_ptr<JSONObject> GraphicsLayerAsJSON(
std::unique_ptr<JSONArray> mask_layer_json = JSONArray::Create();
mask_layer_json->PushObject(
GraphicsLayerAsJSON(layer->MaskLayer(), flags, rendering_context_map,
layer->MaskLayer()->GetPosition()));
FloatPoint(layer->MaskLayer()->GetPosition())));
json->SetArray("maskLayer", std::move(mask_layer_json));
}

Expand All @@ -208,7 +208,7 @@ std::unique_ptr<JSONObject> GraphicsLayerAsJSON(
JSONArray::Create();
contents_clipping_mask_layer_json->PushObject(GraphicsLayerAsJSON(
layer->ContentsClippingMaskLayer(), flags, rendering_context_map,
layer->ContentsClippingMaskLayer()->GetPosition()));
FloatPoint(layer->ContentsClippingMaskLayer()->GetPosition())));
json->SetArray("contentsClippingMaskLayer",
std::move(contents_clipping_mask_layer_json));
}
Expand Down Expand Up @@ -305,7 +305,7 @@ class LayersAsJSONArray {
void Walk(const GraphicsLayer& layer,
int parent_transform_id,
const FloatPoint& parent_position) {
FloatPoint position = parent_position + layer.GetPosition();
FloatPoint position = parent_position + FloatPoint(layer.GetPosition());
int transform_id = parent_transform_id;
AddLayer(layer, transform_id, position);
for (auto* const child : layer.Children())
Expand All @@ -326,7 +326,7 @@ std::unique_ptr<JSONObject> GraphicsLayerTreeAsJSON(
LayerTreeFlags flags,
RenderingContextMap& rendering_context_map) {
std::unique_ptr<JSONObject> json = GraphicsLayerAsJSON(
layer, flags, rendering_context_map, layer->GetPosition());
layer, flags, rendering_context_map, FloatPoint(layer->GetPosition()));

if (layer->Children().size()) {
std::unique_ptr<JSONArray> children_json = JSONArray::Create();
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/platform/geometry/float_point.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ FloatPoint::FloatPoint(const LayoutPoint& p)
FloatPoint::FloatPoint(const LayoutSize& size)
: x_(size.Width().ToFloat()), y_(size.Height().ToFloat()) {}

FloatPoint::FloatPoint(const gfx::PointF& point)
: x_(point.x()), y_(point.y()) {}

float FloatPoint::SlopeAngleRadians() const {
return atan2f(y_, x_);
}
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/platform/geometry/float_point.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class PLATFORM_EXPORT FloatPoint {
explicit FloatPoint(const LayoutSize&);
constexpr explicit FloatPoint(const IntSize& size)
: x_(size.Width()), y_(size.Height()) {}
explicit FloatPoint(const gfx::PointF&);

static constexpr FloatPoint Zero() { return FloatPoint(); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,9 +624,12 @@ String GraphicsLayer::DebugName(cc::Layer* layer) const {
return "";
}

void GraphicsLayer::SetPosition(const FloatPoint& point) {
position_ = point;
CcLayer()->SetPosition(position_);
void GraphicsLayer::SetPosition(const gfx::PointF& point) {
CcLayer()->SetPosition(point);
}

const gfx::PointF& GraphicsLayer::GetPosition() const {
return CcLayer()->position();
}

void GraphicsLayer::SetSize(const IntSize& size) {
Expand Down
6 changes: 2 additions & 4 deletions third_party/blink/renderer/platform/graphics/graphics_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ class PLATFORM_EXPORT GraphicsLayer : public cc::LayerClient,

// The position of the layer (the location of its top-left corner in its
// parent).
const FloatPoint& GetPosition() const { return position_; }
void SetPosition(const FloatPoint&);
const gfx::PointF& GetPosition() const;
void SetPosition(const gfx::PointF&);

const FloatPoint3D& TransformOrigin() const { return transform_origin_; }
void SetTransformOrigin(const FloatPoint3D&);
Expand Down Expand Up @@ -369,8 +369,6 @@ class PLATFORM_EXPORT GraphicsLayer : public cc::LayerClient,
// Offset from the owning layoutObject
IntSize offset_from_layout_object_;

// Position is relative to the parent GraphicsLayer
FloatPoint position_;
IntSize size_;

TransformationMatrix transform_;
Expand Down

0 comments on commit aaa8368

Please sign in to comment.