From f942757417b8472005abc6ef62dbc18d6d1bf86e Mon Sep 17 00:00:00 2001 From: danakj Date: Fri, 24 Apr 2015 15:19:02 -0700 Subject: cc: Some more cleanup and removing TODOs for validating resources. This adds a LayerImpl::ValidateQuadResources() that each layer can call on quads that it appends. This function is empty when DCHECKs are off, which means it should be cheap/free in release builds. R=piman@chromium.org BUG=475894 Review URL: https://codereview.chromium.org/1096703006 Cr-Commit-Position: refs/heads/master@{#326903} --- cc/layers/delegated_renderer_layer_impl.cc | 16 ++-------------- cc/layers/heads_up_display_layer_impl.cc | 4 +--- cc/layers/io_surface_layer_impl.cc | 4 +--- cc/layers/layer_impl.cc | 17 +++++++++++++++++ cc/layers/layer_impl.h | 9 +++++++++ cc/layers/nine_patch_layer_impl.cc | 12 +++++++++--- cc/layers/painted_scrollbar_layer_impl.cc | 6 ++---- cc/layers/picture_layer_impl.cc | 7 ++++--- cc/layers/texture_layer_impl.cc | 3 +-- cc/layers/tiled_layer_impl.cc | 4 +--- cc/layers/ui_resource_layer_impl.cc | 4 +--- cc/layers/video_layer_impl.cc | 28 +++++----------------------- cc/resources/resource_provider.cc | 7 +++---- cc/resources/resource_provider.h | 2 +- 14 files changed, 57 insertions(+), 66 deletions(-) (limited to 'cc') diff --git a/cc/layers/delegated_renderer_layer_impl.cc b/cc/layers/delegated_renderer_layer_impl.cc index aa16beb..7037038 100644 --- a/cc/layers/delegated_renderer_layer_impl.cc +++ b/cc/layers/delegated_renderer_layer_impl.cc @@ -408,14 +408,6 @@ void DelegatedRendererLayerImpl::AppendRainbowDebugBorder( } } -// TODO(danakj): crbug.com/455931 -static ResourceProvider::ResourceId ValidateResource( - ResourceProvider* provider, - ResourceProvider::ResourceId id) { - provider->ValidateResource(id); - return id; -} - void DelegatedRendererLayerImpl::AppendRenderPassQuads( RenderPass* render_pass, const RenderPass* delegated_render_pass, @@ -487,9 +479,7 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads( DrawQuad* output_quad = render_pass->CopyFromAndAppendDrawQuad( delegated_quad, output_shared_quad_state); output_quad->visible_rect = quad_visible_rect; - // TODO(danakj): crbug.com/455931 - output_quad->IterateResources(base::Bind( - &ValidateResource, layer_tree_impl()->resource_provider())); + ValidateQuadResources(output_quad); } else { RenderPassId delegated_contributing_render_pass_id = RenderPassDrawQuad::MaterialCast(delegated_quad)->render_pass_id; @@ -507,9 +497,7 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads( RenderPassDrawQuad::MaterialCast(delegated_quad), output_shared_quad_state, output_contributing_render_pass_id); output_quad->visible_rect = quad_visible_rect; - // TODO(danakj): crbug.com/455931 - output_quad->IterateResources(base::Bind( - &ValidateResource, layer_tree_impl()->resource_provider())); + ValidateQuadResources(output_quad); } } } diff --git a/cc/layers/heads_up_display_layer_impl.cc b/cc/layers/heads_up_display_layer_impl.cc index 3eacbbd..7d449b7 100644 --- a/cc/layers/heads_up_display_layer_impl.cc +++ b/cc/layers/heads_up_display_layer_impl.cc @@ -159,9 +159,6 @@ void HeadsUpDisplayLayerImpl::AppendQuads( bool nearest_neighbor = false; TextureDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - resources_.back()->id()); quad->SetNew(shared_quad_state, quad_rect, opaque_rect, @@ -174,6 +171,7 @@ void HeadsUpDisplayLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } void HeadsUpDisplayLayerImpl::UpdateHudTexture( diff --git a/cc/layers/io_surface_layer_impl.cc b/cc/layers/io_surface_layer_impl.cc index f9acc68..a7da986 100644 --- a/cc/layers/io_surface_layer_impl.cc +++ b/cc/layers/io_surface_layer_impl.cc @@ -77,9 +77,6 @@ void IOSurfaceLayerImpl::AppendQuads( if (visible_quad_rect.IsEmpty()) return; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - io_surface_resource_id_); IOSurfaceDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, @@ -89,6 +86,7 @@ void IOSurfaceLayerImpl::AppendQuads( io_surface_size_, io_surface_resource_id_, IOSurfaceDrawQuad::FLIPPED); + ValidateQuadResources(quad); } void IOSurfaceLayerImpl::ReleaseResources() { diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc index 5655596..c47e50c 100644 --- a/cc/layers/layer_impl.cc +++ b/cc/layers/layer_impl.cc @@ -746,6 +746,23 @@ void LayerImpl::NoteLayerPropertyChangedForDescendants() { SetNeedsPushProperties(); } +#if DCHECK_IS_ON() +// Verify that the resource id is valid. +static ResourceProvider::ResourceId ValidateResource( + const ResourceProvider* provider, + ResourceProvider::ResourceId id) { + provider->ValidateResource(id); + return id; +} +#endif + +void LayerImpl::ValidateQuadResourcesInternal(DrawQuad* quad) const { +#if DCHECK_IS_ON() + quad->IterateResources( + base::Bind(&ValidateResource, layer_tree_impl_->resource_provider())); +#endif +} + const char* LayerImpl::LayerTypeAsString() const { return "cc::LayerImpl"; } diff --git a/cc/layers/layer_impl.h b/cc/layers/layer_impl.h index 730b188..f3eb4ce 100644 --- a/cc/layers/layer_impl.h +++ b/cc/layers/layer_impl.h @@ -247,6 +247,13 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, AppendQuadsData* append_quads_data) {} virtual void DidDraw(ResourceProvider* resource_provider); + // Verify that the resource ids in the quad are valid. + void ValidateQuadResources(DrawQuad* quad) const { +#if DCHECK_IS_ON() + ValidateQuadResourcesInternal(quad); +#endif + } + virtual void GetContentsResourceId(ResourceProvider::ResourceId* resource_id, gfx::Size* resource_size) const; @@ -669,6 +676,8 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, gfx::Rect GetScaledEnclosingRectInTargetSpace(float scale) const; private: + void ValidateQuadResourcesInternal(DrawQuad* quad) const; + void PushScrollOffset(const gfx::ScrollOffset* scroll_offset); // If the new scroll offset is assigned from the root scroll offset delegate, // LayerImpl won't inform the root scroll offset delegate about the scroll diff --git a/cc/layers/nine_patch_layer_impl.cc b/cc/layers/nine_patch_layer_impl.cc index 898e6b5..fc2f457 100644 --- a/cc/layers/nine_patch_layer_impl.cc +++ b/cc/layers/nine_patch_layer_impl.cc @@ -100,9 +100,6 @@ void NinePatchLayerImpl::AppendQuads( if (!resource) return; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource(resource); - static const bool flipped = false; static const bool nearest_neighbor = false; static const bool premultiplied_alpha = true; @@ -234,6 +231,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -255,6 +253,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -276,6 +275,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -297,6 +297,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -318,6 +319,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -339,6 +341,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -360,6 +363,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } visible_rect = @@ -381,6 +385,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } if (fill_center_) { @@ -403,6 +408,7 @@ void NinePatchLayerImpl::AppendQuads( vertex_opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } } } diff --git a/cc/layers/painted_scrollbar_layer_impl.cc b/cc/layers/painted_scrollbar_layer_impl.cc index ad98c4f..005e087 100644 --- a/cc/layers/painted_scrollbar_layer_impl.cc +++ b/cc/layers/painted_scrollbar_layer_impl.cc @@ -103,14 +103,13 @@ void PaintedScrollbarLayerImpl::AppendQuads( if (thumb_resource_id && !visible_thumb_quad_rect.IsEmpty()) { gfx::Rect opaque_rect; const float opacity[] = {1.0f, 1.0f, 1.0f, 1.0f}; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource(thumb_resource_id); TextureDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, scaled_thumb_quad_rect, opaque_rect, scaled_visible_thumb_quad_rect, thumb_resource_id, premultipled_alpha, uv_top_left, uv_bottom_right, SK_ColorTRANSPARENT, opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } gfx::Rect track_quad_rect(bounds()); @@ -124,14 +123,13 @@ void PaintedScrollbarLayerImpl::AppendQuads( gfx::Rect opaque_rect(contents_opaque() ? scaled_track_quad_rect : gfx::Rect()); const float opacity[] = {1.0f, 1.0f, 1.0f, 1.0f}; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource(track_resource_id); TextureDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, scaled_track_quad_rect, opaque_rect, scaled_visible_track_quad_rect, track_resource_id, premultipled_alpha, uv_top_left, uv_bottom_right, SK_ColorTRANSPARENT, opacity, flipped, nearest_neighbor); + ValidateQuadResources(quad); } } diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 3fb0256..34a7599 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -208,6 +208,7 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass, visible_geometry_rect, texture_rect, texture_size, nearest_neighbor_, RGBA_8888, quad_content_rect, max_contents_scale, raster_source_); + ValidateQuadResources(quad); return; } @@ -305,15 +306,13 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass, append_quads_data->num_incomplete_tiles++; } - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - draw_info.resource_id()); TileDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, geometry_rect, opaque_rect, visible_geometry_rect, draw_info.resource_id(), texture_rect, draw_info.resource_size(), draw_info.contents_swizzled(), nearest_neighbor_); + ValidateQuadResources(quad); has_draw_quad = true; break; } @@ -322,6 +321,7 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass, render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, geometry_rect, visible_geometry_rect, draw_info.solid_color(), false); + ValidateQuadResources(quad); has_draw_quad = true; break; } @@ -346,6 +346,7 @@ void PictureLayerImpl::AppendQuads(RenderPass* render_pass, visible_geometry_rect, color, false); + ValidateQuadResources(quad); } if (geometry_rect.Intersects(scaled_viewport_for_tile_priority)) { diff --git a/cc/layers/texture_layer_impl.cc b/cc/layers/texture_layer_impl.cc index 5df00b8..a7775c9 100644 --- a/cc/layers/texture_layer_impl.cc +++ b/cc/layers/texture_layer_impl.cc @@ -167,8 +167,6 @@ void TextureLayerImpl::AppendQuads(RenderPass* render_pass, render_pass->CreateAndAppendDrawQuad(); ResourceProvider::ResourceId id = valid_texture_copy_ ? texture_copy_->id() : external_texture_resource_; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource(id); quad->SetNew(shared_quad_state, quad_rect, opaque_rect, @@ -181,6 +179,7 @@ void TextureLayerImpl::AppendQuads(RenderPass* render_pass, vertex_opacity_, flipped_, nearest_neighbor_); + ValidateQuadResources(quad); } SimpleEnclosedRegion TextureLayerImpl::VisibleContentOpaqueRegion() const { diff --git a/cc/layers/tiled_layer_impl.cc b/cc/layers/tiled_layer_impl.cc index f2b2b00..85b1115 100644 --- a/cc/layers/tiled_layer_impl.cc +++ b/cc/layers/tiled_layer_impl.cc @@ -260,9 +260,6 @@ void TiledLayerImpl::AppendQuads(RenderPass* render_pass, float tile_height = static_cast(tiler_->tile_size().height()); gfx::Size texture_size(tile_width, tile_height); - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - tile->resource_id()); TileDrawQuad* quad = render_pass->CreateAndAppendDrawQuad(); quad->SetNew(shared_quad_state, tile_rect, @@ -273,6 +270,7 @@ void TiledLayerImpl::AppendQuads(RenderPass* render_pass, texture_size, tile->contents_swizzled(), false); + ValidateQuadResources(quad); } } } diff --git a/cc/layers/ui_resource_layer_impl.cc b/cc/layers/ui_resource_layer_impl.cc index 5986bec..582ea9e 100644 --- a/cc/layers/ui_resource_layer_impl.cc +++ b/cc/layers/ui_resource_layer_impl.cc @@ -110,9 +110,6 @@ void UIResourceLayerImpl::AppendQuads( if (!resource) return; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource(resource); - static const bool flipped = false; static const bool nearest_neighbor = false; static const bool premultiplied_alpha = true; @@ -144,6 +141,7 @@ void UIResourceLayerImpl::AppendQuads( vertex_opacity_, flipped, nearest_neighbor); + ValidateQuadResources(quad); } const char* UIResourceLayerImpl::LayerTypeAsString() const { diff --git a/cc/layers/video_layer_impl.cc b/cc/layers/video_layer_impl.cc index 5c36e99..5171971 100644 --- a/cc/layers/video_layer_impl.cc +++ b/cc/layers/video_layer_impl.cc @@ -202,9 +202,6 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, float opacity[] = {1.0f, 1.0f, 1.0f, 1.0f}; bool flipped = false; bool nearest_neighbor = false; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - software_resources_[0]); TextureDrawQuad* texture_quad = render_pass->CreateAndAppendDrawQuad(); texture_quad->SetNew(shared_quad_state, @@ -219,6 +216,7 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, opacity, flipped, nearest_neighbor); + ValidateQuadResources(texture_quad); break; } case VideoFrameExternalResources::YUV_RESOURCE: { @@ -244,17 +242,6 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, frame_->format(), media::VideoFrame::kAPlane, coded_size)); } - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[0]); - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[1]); - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[2]); - if (frame_resources_.size() > 3) { - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[3]); - } gfx::RectF tex_coord_rect( tex_x_offset, tex_y_offset, tex_width_scale, tex_height_scale); YUVVideoDrawQuad* yuv_video_quad = @@ -264,6 +251,7 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, tex_coord_rect, ya_tex_size, uv_tex_size, frame_resources_[0], frame_resources_[1], frame_resources_[2], frame_resources_.size() > 3 ? frame_resources_[3] : 0, color_space); + ValidateQuadResources(yuv_video_quad); break; } case VideoFrameExternalResources::RGB_RESOURCE: { @@ -276,9 +264,6 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, float opacity[] = {1.0f, 1.0f, 1.0f, 1.0f}; bool flipped = false; bool nearest_neighbor = false; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[0]); TextureDrawQuad* texture_quad = render_pass->CreateAndAppendDrawQuad(); texture_quad->SetNew(shared_quad_state, @@ -293,15 +278,13 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, opacity, flipped, nearest_neighbor); + ValidateQuadResources(texture_quad); break; } case VideoFrameExternalResources::STREAM_TEXTURE_RESOURCE: { DCHECK_EQ(frame_resources_.size(), 1u); if (frame_resources_.size() < 1u) break; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[0]); gfx::Transform scale; scale.Scale(tex_width_scale, tex_height_scale); StreamVideoDrawQuad* stream_video_quad = @@ -310,15 +293,13 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, shared_quad_state, quad_rect, opaque_rect, visible_quad_rect, frame_resources_[0], scale * provider_client_impl_->StreamTextureMatrix()); + ValidateQuadResources(stream_video_quad); break; } case VideoFrameExternalResources::IO_SURFACE: { DCHECK_EQ(frame_resources_.size(), 1u); if (frame_resources_.size() < 1u) break; - // TODO(danakj): crbug.com/455931 - layer_tree_impl()->resource_provider()->ValidateResource( - frame_resources_[0]); IOSurfaceDrawQuad* io_surface_quad = render_pass->CreateAndAppendDrawQuad(); io_surface_quad->SetNew(shared_quad_state, @@ -328,6 +309,7 @@ void VideoLayerImpl::AppendQuads(RenderPass* render_pass, visible_rect.size(), frame_resources_[0], IOSurfaceDrawQuad::UNFLIPPED); + ValidateQuadResources(io_surface_quad); break; } #if defined(VIDEO_HOLE) diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc index a336d75..522150c 100644 --- a/cc/resources/resource_provider.cc +++ b/cc/resources/resource_provider.cc @@ -863,10 +863,9 @@ ResourceProvider::Resource* ResourceProvider::InsertResource( ResourceProvider::Resource* ResourceProvider::GetResource(ResourceId id) { DCHECK(thread_checker_.CalledOnValidThread()); - // TODO(danakj): crbug.com/455931 - CHECK(id); + DCHECK(id); ResourceMap::iterator it = resources_.find(id); - CHECK(it != resources_.end()); + DCHECK(it != resources_.end()); return &it->second; } @@ -2044,7 +2043,7 @@ GLint ResourceProvider::GetActiveTextureUnit(GLES2Interface* gl) { return active_unit; } -void ResourceProvider::ValidateResource(ResourceId id) { +void ResourceProvider::ValidateResource(ResourceId id) const { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(id); DCHECK(resources_.find(id) != resources_.end()); diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h index 70b7fc7..ea9588a 100644 --- a/cc/resources/resource_provider.h +++ b/cc/resources/resource_provider.h @@ -434,7 +434,7 @@ class CC_EXPORT ResourceProvider { OutputSurface* output_surface() { return output_surface_; } - void ValidateResource(ResourceId id); + void ValidateResource(ResourceId id) const; private: struct Resource { -- cgit v1.1