Skip to content

Commit

Permalink
Revert of "Converted LayerImpl::bounds() to return SizeF."
Browse files Browse the repository at this point in the history
Seems this was more contentious than originally anticipated. Reverting
this until we have consensus on how to go forward.

TBR=aelias@chromium.org,enne@chromium.org,danakj@chromium.org
BUG=

Review URL: https://codereview.chromium.org/637913003

Cr-Commit-Position: refs/heads/master@{#298710}
  • Loading branch information
bokand authored and Commit bot committed Oct 8, 2014
1 parent 6efe7f2 commit cccfde7
Show file tree
Hide file tree
Showing 26 changed files with 116 additions and 208 deletions.
4 changes: 2 additions & 2 deletions cc/debug/debug_rect_history.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ void DebugRectHistory::SavePaintRects(LayerImpl* layer) {

if (!layer->update_rect().IsEmpty() && layer->DrawsContent()) {
float width_scale = layer->content_bounds().width() /
layer->bounds().width();
static_cast<float>(layer->bounds().width());
float height_scale = layer->content_bounds().height() /
layer->bounds().height();
static_cast<float>(layer->bounds().height());
gfx::Rect update_content_rect = gfx::ScaleToEnclosingRect(
gfx::ToEnclosingRect(layer->update_rect()), width_scale, height_scale);
debug_rects_.push_back(
Expand Down
2 changes: 1 addition & 1 deletion cc/layers/delegated_renderer_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void DelegatedRendererLayerImpl::SetFrameData(
gfx::RectF damage_in_layer = damage_in_frame;
damage_in_layer.Scale(inverse_device_scale_factor_);
SetUpdateRect(gfx::IntersectRects(
gfx::UnionRects(update_rect(), damage_in_layer), gfx::RectF(bounds())));
gfx::UnionRects(update_rect(), damage_in_layer), gfx::Rect(bounds())));

SetRenderPasses(&render_pass_list);
have_render_passes_to_push_ = true;
Expand Down
6 changes: 3 additions & 3 deletions cc/layers/heads_up_display_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ SkRect HeadsUpDisplayLayerImpl::DrawFPSDisplay(

int width = kGraphWidth + kHistogramWidth + 4 * kPadding;
int height = kFontHeight + kGraphHeight + 4 * kPadding + 2;
int left = ceil(bounds().width()) - width - right;
int left = bounds().width() - width - right;
SkRect area = SkRect::MakeXYWH(left, top, width, height);

SkPaint paint = CreatePaint();
Expand Down Expand Up @@ -492,7 +492,7 @@ SkRect HeadsUpDisplayLayerImpl::DrawMemoryDisplay(SkCanvas* canvas,
const int kFontHeight = 13;

const int height = 3 * kFontHeight + 4 * kPadding;
const int left = ceil(bounds().width()) - width - right;
const int left = bounds().width() - width - right;
const SkRect area = SkRect::MakeXYWH(left, top, width, height);

const double megabyte = 1024.0 * 1024.0;
Expand Down Expand Up @@ -547,7 +547,7 @@ SkRect HeadsUpDisplayLayerImpl::DrawPaintTimeDisplay(
const int width = kGraphWidth + 2 * kPadding;
const int height =
kFontHeight + kGraphHeight + 4 * kPadding + 2 + kFontHeight + kPadding;
const int left = ceil(bounds().width()) - width - right;
const int left = bounds().width() - width - right;

const SkRect area = SkRect::MakeXYWH(left, top, width, height);

Expand Down
9 changes: 5 additions & 4 deletions cc/layers/layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ gfx::Vector2dF LayerImpl::FixedContainerSizeDelta() const {
// (w/s',h/s') - (w/s,h/s) = (w,h)(1/s' - 1/s) = (w,h)(1 - ds)/(s ds)
//
gfx::Vector2dF delta_from_pinch =
gfx::RectF(scroll_clip_layer_->bounds()).bottom_right() - gfx::PointF();
gfx::Rect(scroll_clip_layer_->bounds()).bottom_right() - gfx::PointF();
delta_from_pinch.Scale((1.f - scale_delta) / (scale * scale_delta));

return delta_from_scroll + delta_from_pinch;
Expand All @@ -633,8 +633,8 @@ base::DictionaryValue* LayerImpl::LayerTreeAsJson() const {
result->SetString("LayerType", LayerTypeAsString());

base::ListValue* list = new base::ListValue;
list->AppendDouble(bounds().width());
list->AppendDouble(bounds().height());
list->AppendInteger(bounds().width());
list->AppendInteger(bounds().height());
result->Set("Bounds", list);

list = new base::ListValue;
Expand Down Expand Up @@ -771,7 +771,8 @@ bool LayerImpl::IsActive() const {
return layer_tree_impl_->IsActiveTree();
}

gfx::SizeF LayerImpl::bounds() const {
// TODO(aelias): Convert so that bounds returns SizeF.
gfx::Size LayerImpl::bounds() const {
return gfx::ToCeiledSize(gfx::SizeF(bounds_.width() + bounds_delta_.x(),
bounds_.height() + bounds_delta_.y()));
}
Expand Down
5 changes: 4 additions & 1 deletion cc/layers/layer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,
// them from the other values.

void SetBounds(const gfx::Size& bounds);
gfx::SizeF bounds() const;
gfx::Size bounds() const;
void SetBoundsDelta(const gfx::Vector2dF& bounds_delta);
gfx::Vector2dF bounds_delta() const { return bounds_delta_; }

Expand Down Expand Up @@ -515,6 +515,9 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,
void RemoveScrollbar(ScrollbarLayerImplBase* layer);
bool HasScrollbar(ScrollbarOrientation orientation) const;
void ScrollbarParametersDidChange(bool on_resize);
int clip_height() {
return scroll_clip_layer_ ? scroll_clip_layer_->bounds().height() : 0;
}

gfx::Rect LayerRectToContentRect(const gfx::RectF& layer_rect) const;

Expand Down
4 changes: 2 additions & 2 deletions cc/layers/layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ TEST_F(LayerImplScrollTest, SetNewScrollbarParameters) {
vertical_scrollbar->SetScrollLayerAndClipLayerByIds(
layer()->id(), tree()->root_layer()->id());

float expected_vertical_maximum =
int expected_vertical_maximum =
layer()->bounds().height() - tree()->root_layer()->bounds().height();
EXPECT_EQ(expected_vertical_maximum, vertical_scrollbar->maximum());
EXPECT_EQ(scroll_offset.y(), vertical_scrollbar->current_pos());
Expand All @@ -692,7 +692,7 @@ TEST_F(LayerImplScrollTest, SetNewScrollbarParameters) {
horizontal_scrollbar->SetScrollLayerAndClipLayerByIds(
layer()->id(), tree()->root_layer()->id());

float expected_horizontal_maximum =
int expected_horizontal_maximum =
layer()->bounds().width() - tree()->root_layer()->bounds().width();
EXPECT_EQ(expected_horizontal_maximum, horizontal_scrollbar->maximum());
EXPECT_EQ(scroll_offset.x(), horizontal_scrollbar->current_pos());
Expand Down
1 change: 1 addition & 0 deletions cc/layers/painted_scrollbar_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ void PaintedScrollbarLayerImpl::AppendQuads(
bool flipped = false;
gfx::PointF uv_top_left(0.f, 0.f);
gfx::PointF uv_bottom_right(1.f, 1.f);
gfx::Rect bounds_rect(bounds());
gfx::Rect content_bounds_rect(content_bounds());

SharedQuadState* shared_quad_state =
Expand Down
8 changes: 3 additions & 5 deletions cc/layers/picture_image_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "cc/test/test_shared_bitmap_manager.h"
#include "cc/trees/layer_tree_impl.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/size_conversions.h"

namespace cc {
namespace {
Expand Down Expand Up @@ -64,9 +63,8 @@ class PictureImageLayerImplTest : public testing::Test {
new TestablePictureImageLayerImpl(tree, id);
layer->SetBounds(gfx::Size(100, 200));
layer->SetContentBounds(gfx::Size(100, 200));
layer->tilings_.reset(
new PictureLayerTilingSet(&tiling_client_,
gfx::ToCeiledSize(layer->bounds())));
layer->tilings_.reset(new PictureLayerTilingSet(&tiling_client_,
layer->bounds()));
layer->pile_ = tiling_client_.GetPile();
return make_scoped_ptr(layer);
}
Expand Down Expand Up @@ -147,7 +145,7 @@ TEST_F(PictureImageLayerImplTest, IgnoreIdealContentScale) {

// Draw.
active_layer->draw_properties().visible_content_rect =
gfx::Rect(gfx::ToCeiledSize(active_layer->bounds()));
gfx::Rect(active_layer->bounds());
scoped_ptr<RenderPass> render_pass = RenderPass::Create();
AppendQuadsData data;
active_layer->WillDraw(DRAW_MODE_SOFTWARE, NULL);
Expand Down
4 changes: 1 addition & 3 deletions cc/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "cc/layers/picture_layer_impl.h"
#include "cc/trees/layer_tree_impl.h"
#include "third_party/skia/include/core/SkPictureRecorder.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/rect_conversions.h"

namespace cc {
Expand Down Expand Up @@ -47,8 +46,7 @@ void PictureLayer::PushPropertiesTo(LayerImpl* base_layer) {
// TODO(ernstm): This DCHECK is only valid as long as the pile's tiling_rect
// is identical to the layer_rect.
// If update called, then pile size must match bounds pushed to impl layer.
DCHECK_EQ(gfx::ToCeiledSize(layer_impl->bounds()).ToString(),
pile_->tiling_size().ToString());
DCHECK_EQ(layer_impl->bounds().ToString(), pile_->tiling_size().ToString());
}

// Unlike other properties, invalidation must always be set on layer_impl.
Expand Down
10 changes: 3 additions & 7 deletions cc/layers/picture_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -732,10 +732,7 @@ void PictureLayerImpl::SyncFromActiveLayer(const PictureLayerImpl* other) {
bool synced_high_res_tiling = false;
if (CanHaveTilings()) {
synced_high_res_tiling = tilings_->SyncTilings(
*other->tilings_,
gfx::ToCeiledSize(bounds()),
invalidation_,
MinimumContentsScale());
*other->tilings_, bounds(), invalidation_, MinimumContentsScale());
} else {
RemoveAllTilings();
}
Expand Down Expand Up @@ -953,8 +950,7 @@ void PictureLayerImpl::DoPostCommitInitialization() {
DCHECK(layer_tree_impl()->IsPendingTree());

if (!tilings_)
tilings_.reset(new PictureLayerTilingSet(this,
gfx::ToCeiledSize(bounds())));
tilings_.reset(new PictureLayerTilingSet(this, bounds()));

DCHECK(!twin_layer_);
twin_layer_ = static_cast<PictureLayerImpl*>(
Expand Down Expand Up @@ -1295,7 +1291,7 @@ float PictureLayerImpl::MinimumContentsScale() const {
// then it will end up having less than one pixel of content in that
// dimension. Bump the minimum contents scale up in this case to prevent
// this from happening.
float min_dimension = std::min(bounds().width(), bounds().height());
int min_dimension = std::min(bounds().width(), bounds().height());
if (!min_dimension)
return setting_min;

Expand Down
2 changes: 1 addition & 1 deletion cc/layers/picture_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ TEST(PictureLayerTest, NoTilesIfEmptyBounds) {

layer->PushPropertiesTo(layer_impl.get());
EXPECT_FALSE(layer_impl->CanHaveTilings());
EXPECT_TRUE(layer_impl->bounds() == gfx::SizeF(0, 0));
EXPECT_TRUE(layer_impl->bounds() == gfx::Size(0, 0));
EXPECT_EQ(gfx::Size(), layer_impl->pile()->tiling_size());
EXPECT_FALSE(layer_impl->pile()->HasRecordings());
}
Expand Down
3 changes: 1 addition & 2 deletions cc/layers/ui_resource_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "cc/quads/texture_draw_quad.h"
#include "cc/trees/layer_tree_impl.h"
#include "cc/trees/occlusion.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/rect_f.h"

namespace cc {
Expand Down Expand Up @@ -120,7 +119,7 @@ void UIResourceLayerImpl::AppendQuads(
bool opaque = layer_tree_impl()->IsUIResourceOpaque(ui_resource_id_) ||
contents_opaque();

gfx::Rect quad_rect(gfx::ToCeiledSize(bounds()));
gfx::Rect quad_rect(bounds());
gfx::Rect opaque_rect(opaque ? quad_rect : gfx::Rect());
gfx::Rect visible_quad_rect =
occlusion_in_content_space.GetUnoccludedContentRect(quad_rect);
Expand Down
5 changes: 2 additions & 3 deletions cc/layers/ui_resource_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "cc/trees/single_thread_proxy.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/transform.h"

namespace cc {
Expand Down Expand Up @@ -121,7 +120,7 @@ TEST(UIResourceLayerImplTest, VerifySetOpaqueOnSkBitmap) {
layer_size,
opaque,
uid);
expected_opaque_bounds = gfx::Rect(gfx::ToCeiledSize(layer->bounds()));
expected_opaque_bounds = gfx::Rect(layer->bounds());
OpaqueBoundsTest(layer.Pass(), expected_opaque_bounds);
}

Expand All @@ -143,7 +142,7 @@ TEST(UIResourceLayerImplTest, VerifySetOpaqueOnLayer) {
layer = GenerateUIResourceLayer(
&host_impl, bitmap_size, layer_size, skbitmap_opaque, uid);
layer->SetContentsOpaque(true);
expected_opaque_bounds = gfx::Rect(gfx::ToCeiledSize(layer->bounds()));
expected_opaque_bounds = gfx::Rect(layer->bounds());
OpaqueBoundsTest(layer.Pass(), expected_opaque_bounds);
}

Expand Down
6 changes: 3 additions & 3 deletions cc/test/layer_tree_json_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ scoped_refptr<Layer> ParseTreeFromValue(base::Value* val,
success &= dict->GetString("LayerType", &layer_type);
base::ListValue* list;
success &= dict->GetList("Bounds", &list);
double width, height;
success &= list->GetDouble(0, &width);
success &= list->GetDouble(1, &height);
int width, height;
success &= list->GetInteger(0, &width);
success &= list->GetInteger(1, &height);
success &= dict->GetList("Position", &list);
double position_x, position_y;
success &= list->GetDouble(0, &position_x);
Expand Down
7 changes: 2 additions & 5 deletions cc/trees/damage_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "cc/trees/layer_tree_host_common.h"
#include "cc/trees/layer_tree_impl.h"
#include "ui/gfx/geometry/rect_conversions.h"
#include "ui/gfx/geometry/size_conversions.h"

namespace cc {

Expand Down Expand Up @@ -210,8 +209,7 @@ gfx::Rect DamageTracker::TrackDamageFromSurfaceMask(
// expected to be a common case.
if (target_surface_mask_layer->LayerPropertyChanged() ||
!target_surface_mask_layer->update_rect().IsEmpty()) {
damage_rect = gfx::Rect(
gfx::ToCeiledSize(target_surface_mask_layer->bounds()));
damage_rect = gfx::Rect(target_surface_mask_layer->bounds());
}

return damage_rect;
Expand Down Expand Up @@ -380,8 +378,7 @@ void DamageTracker::ExtendDamageForRenderSurface(
const gfx::Transform& replica_draw_transform =
render_surface->replica_draw_transform();
gfx::Rect replica_mask_layer_rect = MathUtil::MapEnclosingClippedRect(
replica_draw_transform,
gfx::Rect(gfx::ToCeiledSize(replica_mask_layer->bounds())));
replica_draw_transform, gfx::Rect(replica_mask_layer->bounds()));
data.Update(replica_mask_layer_rect, mailboxId_);

// In the current implementation, a change in the replica mask damages the
Expand Down
17 changes: 7 additions & 10 deletions cc/trees/damage_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "third_party/skia/include/effects/SkBlurImageFilter.h"
#include "ui/gfx/geometry/quad_f.h"
#include "ui/gfx/geometry/rect_conversions.h"
#include "ui/gfx/geometry/size_conversions.h"

namespace cc {
namespace {
Expand All @@ -33,7 +32,7 @@ void ExecuteCalculateDrawProperties(LayerImpl* root,

FakeLayerTreeHostImpl::RecursiveUpdateNumChildren(root);
LayerTreeHostCommon::CalcDrawPropsImplInputsForTesting inputs(
root, gfx::ToCeiledSize(root->bounds()), render_surface_layer_list);
root, root->bounds(), render_surface_layer_list);
LayerTreeHostCommon::CalculateDrawProperties(&inputs);
}

Expand Down Expand Up @@ -1138,8 +1137,8 @@ TEST_F(DamageTrackerTest, VerifyDamageForMask) {
scoped_ptr<LayerImpl> mask_layer =
LayerImpl::Create(host_impl_.active_tree(), 3);
mask_layer->SetPosition(child->position());
mask_layer->SetBounds(gfx::ToCeiledSize(child->bounds()));
mask_layer->SetContentBounds(gfx::ToCeiledSize(child->bounds()));
mask_layer->SetBounds(child->bounds());
mask_layer->SetContentBounds(child->bounds());
child->SetMaskLayer(mask_layer.Pass());
}
LayerImpl* mask_layer = child->mask_layer();
Expand Down Expand Up @@ -1242,9 +1241,8 @@ TEST_F(DamageTrackerTest, VerifyDamageForReplicaMask) {
scoped_ptr<LayerImpl> replica_mask_layer =
LayerImpl::Create(host_impl_.active_tree(), 7);
replica_mask_layer->SetPosition(gfx::PointF());
replica_mask_layer->SetBounds(gfx::ToCeiledSize(grand_child1->bounds()));
replica_mask_layer->SetContentBounds(
gfx::ToCeiledSize(grand_child1->bounds()));
replica_mask_layer->SetBounds(grand_child1->bounds());
replica_mask_layer->SetContentBounds(grand_child1->bounds());
grand_child1_replica->SetMaskLayer(replica_mask_layer.Pass());
}
LayerImpl* replica_mask_layer = grand_child1_replica->mask_layer();
Expand Down Expand Up @@ -1321,9 +1319,8 @@ TEST_F(DamageTrackerTest, VerifyDamageForReplicaMaskWithTransformOrigin) {
LayerImpl::Create(host_impl_.active_tree(), 7);
replica_mask_layer->SetPosition(gfx::PointF());
// Note: this is not the transform origin being tested.
replica_mask_layer->SetBounds(gfx::ToCeiledSize(grand_child1->bounds()));
replica_mask_layer->SetContentBounds(
gfx::ToCeiledSize(grand_child1->bounds()));
replica_mask_layer->SetBounds(grand_child1->bounds());
replica_mask_layer->SetContentBounds(grand_child1->bounds());
grand_child1_replica->SetMaskLayer(replica_mask_layer.Pass());
}
LayerImpl* replica_mask_layer = grand_child1_replica->mask_layer();
Expand Down
Loading

0 comments on commit cccfde7

Please sign in to comment.