Skip to content

Commit

Permalink
Make ListContainer Consistently Act as Container of Pointers
Browse files Browse the repository at this point in the history
Usage of ListContainer is replacing scoped_ptr_vector. SharedQuadState
is stored in DrawQuad as pointer form. ListContainer should act as
container of pointers.

This CL changes the return value from dereference iterator from
reference to pointers.

BUG=344962

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

Cr-Commit-Position: refs/heads/master@{#301916}
  • Loading branch information
weiliangc authored and Commit bot committed Oct 29, 2014
1 parent e60561d commit 48805fc
Show file tree
Hide file tree
Showing 23 changed files with 181 additions and 185 deletions.
21 changes: 10 additions & 11 deletions cc/layers/delegated_renderer_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,9 @@ void DelegatedRendererLayerImpl::SetFrameData(
&invalid_frame,
resource_map,
&resources_in_frame);
for (size_t i = 0; i < render_pass_list.size(); ++i) {
RenderPass* pass = render_pass_list[i];
for (auto& quad : pass->quad_list)
quad.IterateResources(remap_resources_to_parent_callback);
for (const auto& pass : render_pass_list) {
for (const auto& quad : pass->quad_list)
quad->IterateResources(remap_resources_to_parent_callback);
}

if (invalid_frame) {
Expand Down Expand Up @@ -396,8 +395,8 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads(
bool is_root_delegated_render_pass =
delegated_render_pass == render_passes_in_draw_order_.back();

if (delegated_quad.shared_quad_state != delegated_shared_quad_state) {
delegated_shared_quad_state = delegated_quad.shared_quad_state;
if (delegated_quad->shared_quad_state != delegated_shared_quad_state) {
delegated_shared_quad_state = delegated_quad->shared_quad_state;
output_shared_quad_state = render_pass->CreateAndAppendSharedQuadState();
output_shared_quad_state->CopyFrom(delegated_shared_quad_state);

Expand Down Expand Up @@ -447,18 +446,18 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads(

gfx::Rect quad_visible_rect =
occlusion_in_quad_space.GetUnoccludedContentRect(
delegated_quad.visible_rect);
delegated_quad->visible_rect);

if (quad_visible_rect.IsEmpty())
continue;

if (delegated_quad.material != DrawQuad::RENDER_PASS) {
if (delegated_quad->material != DrawQuad::RENDER_PASS) {
DrawQuad* output_quad = render_pass->CopyFromAndAppendDrawQuad(
&delegated_quad, output_shared_quad_state);
delegated_quad, output_shared_quad_state);
output_quad->visible_rect = quad_visible_rect;
} else {
RenderPassId delegated_contributing_render_pass_id =
RenderPassDrawQuad::MaterialCast(&delegated_quad)->render_pass_id;
RenderPassDrawQuad::MaterialCast(delegated_quad)->render_pass_id;
RenderPassId output_contributing_render_pass_id(-1, -1);

bool present =
Expand All @@ -473,7 +472,7 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads(

RenderPassDrawQuad* output_quad =
render_pass->CopyFromAndAppendRenderPassDrawQuad(
RenderPassDrawQuad::MaterialCast(&delegated_quad),
RenderPassDrawQuad::MaterialCast(delegated_quad),
output_shared_quad_state,
output_contributing_render_pass_id);
output_quad->visible_rect = quad_visible_rect;
Expand Down
2 changes: 1 addition & 1 deletion cc/layers/nine_patch_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void NinePatchLayerLayoutTest(const gfx::Size& bitmap_size,
gfx::Rect bitmap_rect(bitmap_size);
Region tex_remaining(bitmap_rect);
for (const auto& quad : quads) {
const TextureDrawQuad* tex_quad = TextureDrawQuad::MaterialCast(&quad);
const TextureDrawQuad* tex_quad = TextureDrawQuad::MaterialCast(quad);
gfx::RectF tex_rect =
gfx::BoundingRect(tex_quad->uv_top_left, tex_quad->uv_bottom_right);
tex_rect.Scale(bitmap_size.width(), bitmap_size.height());
Expand Down
8 changes: 4 additions & 4 deletions cc/layers/picture_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1467,10 +1467,10 @@ TEST_F(PictureLayerImplTest, SolidColorLayerHasVisibleFullCoverage) {
active_layer_->DidDraw(nullptr);

Region remaining = visible_rect;
for (auto& quad : render_pass->quad_list) {
EXPECT_TRUE(visible_rect.Contains(quad.rect));
EXPECT_TRUE(remaining.Contains(quad.rect));
remaining.Subtract(quad.rect);
for (const auto& quad : render_pass->quad_list) {
EXPECT_TRUE(visible_rect.Contains(quad->rect));
EXPECT_TRUE(remaining.Contains(quad->rect));
remaining.Subtract(quad->rect);
}

EXPECT_TRUE(remaining.IsEmpty());
Expand Down
6 changes: 3 additions & 3 deletions cc/layers/tiled_layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ TEST_F(TiledLayerImplTest, Checkerboarding) {
EXPECT_EQ(0u, data.num_missing_tiles);

for (const auto& quad : render_pass->quad_list)
EXPECT_EQ(quad.material, DrawQuad::TILED_CONTENT);
EXPECT_EQ(quad->material, DrawQuad::TILED_CONTENT);
}

for (int i = 0; i < num_tiles_x; ++i)
Expand All @@ -179,7 +179,7 @@ TEST_F(TiledLayerImplTest, Checkerboarding) {
EXPECT_LT(0u, data.num_missing_tiles);
EXPECT_EQ(render_pass->quad_list.size(), 4u);
for (const auto& quad : render_pass->quad_list)
EXPECT_NE(quad.material, DrawQuad::TILED_CONTENT);
EXPECT_NE(quad->material, DrawQuad::TILED_CONTENT);
}
}

Expand Down Expand Up @@ -258,7 +258,7 @@ TEST_F(TiledLayerImplTest, TextureInfoForLayerNoBorders) {
for (auto iter = render_pass->quad_list.cbegin();
iter != render_pass->quad_list.cend();
++iter) {
const TileDrawQuad* quad = TileDrawQuad::MaterialCast(&*iter);
const TileDrawQuad* quad = TileDrawQuad::MaterialCast(*iter);

EXPECT_NE(0u, quad->resource_id) << LayerTestCommon::quad_string
<< iter.index();
Expand Down
7 changes: 3 additions & 4 deletions cc/output/delegating_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ void DelegatingRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
ResourceProvider::ResourceIdArray resources;
DrawQuad::ResourceIteratorCallback append_to_array =
base::Bind(&AppendToArray, &resources);
for (size_t i = 0; i < out_data.render_pass_list.size(); ++i) {
RenderPass* render_pass = out_data.render_pass_list.at(i);
for (auto& quad : render_pass->quad_list)
quad.IterateResources(append_to_array);
for (const auto& render_pass : out_data.render_pass_list) {
for (const auto& quad : render_pass->quad_list)
quad->IterateResources(append_to_array);
}
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
}
Expand Down
5 changes: 2 additions & 3 deletions cc/output/direct_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,9 @@ void DirectRenderer::DrawRenderPass(DrawingFrame* frame,
}

const QuadList& quad_list = render_pass->quad_list;
for (QuadList::ConstBackToFrontIterator it = quad_list.BackToFrontBegin();
it != quad_list.BackToFrontEnd();
for (auto it = quad_list.BackToFrontBegin(); it != quad_list.BackToFrontEnd();
++it) {
const DrawQuad& quad = *it;
const DrawQuad& quad = **it;
bool should_skip_quad = false;

if (using_scissor_as_optimization) {
Expand Down
7 changes: 3 additions & 4 deletions cc/output/gl_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,9 @@ void GLRenderer::BeginDrawingFrame(DrawingFrame* frame) {
DrawQuad::ResourceIteratorCallback wait_on_resource_syncpoints_callback =
base::Bind(&WaitOnResourceSyncPoints, resource_provider_);

for (size_t i = 0; i < frame->render_passes_in_draw_order->size(); ++i) {
RenderPass* pass = frame->render_passes_in_draw_order->at(i);
for (auto& quad : pass->quad_list)
quad.IterateResources(wait_on_resource_syncpoints_callback);
for (const auto& pass : *frame->render_passes_in_draw_order) {
for (const auto& quad : pass->quad_list)
quad->IterateResources(wait_on_resource_syncpoints_callback);
}

// TODO(enne): Do we need to reinitialize all of this state per frame?
Expand Down
4 changes: 2 additions & 2 deletions cc/output/overlay_strategy_single_on_top.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ bool OverlayStrategySingleOnTop::Attempt(
QuadList& quad_list = root_render_pass->quad_list;
auto candidate_iterator = quad_list.end();
for (auto it = quad_list.begin(); it != quad_list.end(); ++it) {
const DrawQuad* draw_quad = &*it;
const DrawQuad* draw_quad = *it;
if (draw_quad->material == DrawQuad::TEXTURE_CONTENT) {
const TextureDrawQuad& quad = *TextureDrawQuad::MaterialCast(draw_quad);
if (!resource_provider_->AllowOverlay(quad.resource_id)) {
Expand Down Expand Up @@ -58,7 +58,7 @@ bool OverlayStrategySingleOnTop::Attempt(
if (candidate_iterator == quad_list.end())
return false;
const TextureDrawQuad& quad =
*TextureDrawQuad::MaterialCast(&*candidate_iterator);
*TextureDrawQuad::MaterialCast(*candidate_iterator);

// Simple quads only.
gfx::OverlayTransform overlay_transform =
Expand Down
66 changes: 33 additions & 33 deletions cc/quads/list_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ ListContainer<BaseElementType>::~ListContainer() {
template <typename BaseElementType>
void ListContainer<BaseElementType>::EraseAndInvalidateAllPointers(
typename ListContainer<BaseElementType>::Iterator position) {
BaseElementType* item = &*position;
BaseElementType* item = *position;
item->~BaseElementType();
data_->Erase(position);
}
Expand Down Expand Up @@ -376,25 +376,25 @@ ListContainer<BaseElementType>::end() {
template <typename BaseElementType>
BaseElementType* ListContainer<BaseElementType>::front() {
Iterator iter = begin();
return &*iter;
return *iter;
}

template <typename BaseElementType>
BaseElementType* ListContainer<BaseElementType>::back() {
ReverseIterator iter = rbegin();
return &*iter;
return *iter;
}

template <typename BaseElementType>
const BaseElementType* ListContainer<BaseElementType>::front() const {
ConstIterator iter = begin();
return &*iter;
return *iter;
}

template <typename BaseElementType>
const BaseElementType* ListContainer<BaseElementType>::back() const {
ConstReverseIterator iter = rbegin();
return &*iter;
return *iter;
}

template <typename BaseElementType>
Expand All @@ -409,10 +409,10 @@ const BaseElementType* ListContainer<BaseElementType>::ElementAt(
break;
index -= current_size;
}
return &*ConstIterator(data_.get(),
list_index,
data_->InnerListById(list_index)->ElementAt(index),
original_index);
return *ConstIterator(data_.get(),
list_index,
data_->InnerListById(list_index)->ElementAt(index),
original_index);
}

template <typename BaseElementType>
Expand All @@ -426,10 +426,10 @@ BaseElementType* ListContainer<BaseElementType>::ElementAt(size_t index) {
break;
index -= current_size;
}
return &*Iterator(data_.get(),
list_index,
data_->InnerListById(list_index)->ElementAt(index),
original_index);
return *Iterator(data_.get(),
list_index,
data_->InnerListById(list_index)->ElementAt(index),
original_index);
}

template <typename BaseElementType>
Expand Down Expand Up @@ -486,8 +486,8 @@ BaseElementType* ListContainer<BaseElementType>::Iterator::operator->() const {
}

template <typename BaseElementType>
BaseElementType& ListContainer<BaseElementType>::Iterator::operator*() const {
return *(reinterpret_cast<BaseElementType*>(this->item_iterator));
BaseElementType* ListContainer<BaseElementType>::Iterator::operator*() const {
return reinterpret_cast<BaseElementType*>(this->item_iterator);
}

template <typename BaseElementType>
Expand All @@ -500,9 +500,9 @@ operator++(int unused_post_increment) {
}

template <typename BaseElementType>
typename ListContainer<BaseElementType>::Iterator
ListContainer<BaseElementType>::Iterator::
operator++() {
typename ListContainer<BaseElementType>::Iterator&
ListContainer<BaseElementType>::Iterator::
operator++() {
this->Increment();
++index_;
return *this;
Expand Down Expand Up @@ -542,9 +542,9 @@ operator->() const {
}

template <typename BaseElementType>
const BaseElementType& ListContainer<BaseElementType>::ConstIterator::
const BaseElementType* ListContainer<BaseElementType>::ConstIterator::
operator*() const {
return *(reinterpret_cast<const BaseElementType*>(this->item_iterator));
return reinterpret_cast<const BaseElementType*>(this->item_iterator);
}

template <typename BaseElementType>
Expand All @@ -557,9 +557,9 @@ operator++(int unused_post_increment) {
}

template <typename BaseElementType>
typename ListContainer<BaseElementType>::ConstIterator
ListContainer<BaseElementType>::ConstIterator::
operator++() {
typename ListContainer<BaseElementType>::ConstIterator&
ListContainer<BaseElementType>::ConstIterator::
operator++() {
this->Increment();
++index_;
return *this;
Expand Down Expand Up @@ -593,9 +593,9 @@ BaseElementType* ListContainer<BaseElementType>::ReverseIterator::operator->()
}

template <typename BaseElementType>
BaseElementType& ListContainer<BaseElementType>::ReverseIterator::operator*()
BaseElementType* ListContainer<BaseElementType>::ReverseIterator::operator*()
const {
return *(reinterpret_cast<BaseElementType*>(this->item_iterator));
return reinterpret_cast<BaseElementType*>(this->item_iterator);
}

template <typename BaseElementType>
Expand All @@ -608,9 +608,9 @@ operator++(int unused_post_increment) {
}

template <typename BaseElementType>
typename ListContainer<BaseElementType>::ReverseIterator
ListContainer<BaseElementType>::ReverseIterator::
operator++() {
typename ListContainer<BaseElementType>::ReverseIterator&
ListContainer<BaseElementType>::ReverseIterator::
operator++() {
this->ReverseIncrement();
++index_;
return *this;
Expand Down Expand Up @@ -650,9 +650,9 @@ operator->() const {
}

template <typename BaseElementType>
const BaseElementType& ListContainer<BaseElementType>::ConstReverseIterator::
const BaseElementType* ListContainer<BaseElementType>::ConstReverseIterator::
operator*() const {
return *(reinterpret_cast<const BaseElementType*>(this->item_iterator));
return reinterpret_cast<const BaseElementType*>(this->item_iterator);
}

template <typename BaseElementType>
Expand All @@ -665,9 +665,9 @@ operator++(int unused_post_increment) {
}

template <typename BaseElementType>
typename ListContainer<BaseElementType>::ConstReverseIterator
ListContainer<BaseElementType>::ConstReverseIterator::
operator++() {
typename ListContainer<BaseElementType>::ConstReverseIterator&
ListContainer<BaseElementType>::ConstReverseIterator::
operator++() {
this->ReverseIncrement();
++index_;
return *this;
Expand Down
16 changes: 8 additions & 8 deletions cc/quads/list_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ class CC_EXPORT ListContainer {
size_t index);
~Iterator();
BaseElementType* operator->() const;
BaseElementType& operator*() const;
BaseElementType* operator*() const;
Iterator operator++(int unused_post_increment);
Iterator operator++();
Iterator& operator++();

size_t index() const;

Expand All @@ -103,9 +103,9 @@ class CC_EXPORT ListContainer {
ConstIterator(const Iterator& other); // NOLINT
~ConstIterator();
const BaseElementType* operator->() const;
const BaseElementType& operator*() const;
const BaseElementType* operator*() const;
ConstIterator operator++(int unused_post_increment);
ConstIterator operator++();
ConstIterator& operator++();

size_t index() const;

Expand All @@ -128,9 +128,9 @@ class CC_EXPORT ListContainer {
size_t index);
~ReverseIterator();
BaseElementType* operator->() const;
BaseElementType& operator*() const;
BaseElementType* operator*() const;
ReverseIterator operator++(int unused_post_increment);
ReverseIterator operator++();
ReverseIterator& operator++();

size_t index() const;

Expand All @@ -154,9 +154,9 @@ class CC_EXPORT ListContainer {
ConstReverseIterator(const ReverseIterator& other); // NOLINT
~ConstReverseIterator();
const BaseElementType* operator->() const;
const BaseElementType& operator*() const;
const BaseElementType* operator*() const;
ConstReverseIterator operator++(int unused_post_increment);
ConstReverseIterator operator++();
ConstReverseIterator& operator++();

size_t index() const;

Expand Down
Loading

0 comments on commit 48805fc

Please sign in to comment.