cc: Make RenderPass into a struct-like class.

cc/render_pass.cc

Index: cc/render_pass.cc
diff --git a/cc/render_pass.cc b/cc/render_pass.cc
index bbb36e5a629250d83100214c67e5b77c3f19b97c..e73aa4e68fa164aa45752afb6dd9e9088b1b361d 100644
--- a/cc/render_pass.cc
+++ b/cc/render_pass.cc
@@ -16,101 +16,175 @@ using WebKit::WebTransformationMatrix;
 
 namespace cc {
 
-scoped_ptr<RenderPass> RenderPass::create(Id id, gfx::Rect outputRect, const WebKit::WebTransformationMatrix& transformToRootTarget)
-{
-    return make_scoped_ptr(new RenderPass(id, outputRect, transformToRootTarget));
+scoped_ptr<RenderPass> RenderPass::Create() {
+  return make_scoped_ptr(new RenderPass);
 }
 
-RenderPass::RenderPass(Id id, gfx::Rect outputRect, const WebKit::WebTransformationMatrix& transformToRootTarget)
-    : m_id(id)
-    , m_transformToRootTarget(transformToRootTarget)
-    , m_outputRect(outputRect)
-    , m_hasTransparentBackground(true)
-    , m_hasOcclusionFromOutsideTargetSurface(false)
-    , m_filter(0)
-{
-    DCHECK(id.layerId > 0);
-    DCHECK(id.index >= 0);
+RenderPass::RenderPass()
+    : id(Id(-1, -1)),
+      has_transparent_background(true),
+      has_occlusion_from_outside_target_surface(false),
+      filter(NULL) {
 }
 
-RenderPass::~RenderPass()
-{
-    SkSafeUnref(m_filter);
+RenderPass::~RenderPass() {
+  SkSafeUnref(filter);
 }
 
-scoped_ptr<RenderPass> RenderPass::copy(Id newId) const
-{
-    DCHECK(newId != m_id);
-
-    scoped_ptr<RenderPass> copyPass(create(newId, m_outputRect, m_transformToRootTarget));
-    copyPass->setDamageRect(m_damageRect);
-    copyPass->setHasTransparentBackground(m_hasTransparentBackground);
-    copyPass->setHasOcclusionFromOutsideTargetSurface(m_hasOcclusionFromOutsideTargetSurface);
-    copyPass->setFilters(m_filters);
-    copyPass->setBackgroundFilters(m_backgroundFilters);
-    copyPass->setFilter(m_filter);
-    return copyPass.Pass();
+scoped_ptr<RenderPass> RenderPass::Copy(Id new_id) const {
+  DCHECK(new_id != id);
+
+  scoped_ptr<RenderPass> copy_pass(Create());
+  copy_pass->SetAll(new_id,
+                    output_rect,
+                    damage_rect,
+                    transform_to_root_target,
+                    has_transparent_background,
+                    has_occlusion_from_outside_target_surface,
+                    filters,
+                    filter,
+                    background_filters);
+  return copy_pass.Pass();
 }
 
-void RenderPass::appendQuadsForLayer(LayerImpl* layer, OcclusionTrackerImpl* occlusionTracker, AppendQuadsData& appendQuadsData)
-{
-    bool forSurface = false;
-    QuadCuller quadCuller(m_quadList, m_sharedQuadStateList, layer, occlusionTracker, layer->showDebugBorders(), forSurface);
+void RenderPass::SetNew(Id id,
+                        gfx::Rect output_rect,
+                        gfx::RectF damage_rect,
+                        const WebKit::WebTransformationMatrix& transform_to_root_target) {
+  DCHECK_GT(id.layer_id, 0);
+  DCHECK_GE(id.index, 0);
 
-    layer->appendQuads(quadCuller, appendQuadsData);
+  this->id = id;
+  this->output_rect = output_rect;
+  this->damage_rect = damage_rect;
+  this->transform_to_root_target = transform_to_root_target;
+
+  DCHECK(quad_list.isEmpty());
+  DCHECK(shared_quad_state_list.isEmpty());
 }
 
-void RenderPass::appendQuadsForRenderSurfaceLayer(LayerImpl* layer, const RenderPass* contributingRenderPass, OcclusionTrackerImpl* occlusionTracker, AppendQuadsData& appendQuadsData)
-{
-    bool forSurface = true;
-    QuadCuller quadCuller(m_quadList, m_sharedQuadStateList, layer, occlusionTracker, layer->showDebugBorders(), forSurface);
+void RenderPass::SetAll(Id id,
+                        gfx::Rect output_rect,
+                        gfx::RectF damage_rect,
+                        const WebKit::WebTransformationMatrix& transform_to_root_target,
+                        bool has_transparent_background,
+                        bool has_occlusion_from_outside_target_surface,
+                        const WebKit::WebFilterOperations& filters,
+                        SkImageFilter* filter,
+                        const WebKit::WebFilterOperations& background_filters) {
+  DCHECK_GT(id.layer_id, 0);
+  DCHECK_GE(id.index, 0);
+
+  this->id = id;
+  this->output_rect = output_rect;
+  this->damage_rect = damage_rect;
+  this->transform_to_root_target = transform_to_root_target;
+  this->has_transparent_background = has_transparent_background;
+  this->has_occlusion_from_outside_target_surface =
+      has_occlusion_from_outside_target_surface;
+  this->filters = filters;
+  SkRefCnt_SafeAssign(this->filter, filter);
+  this->background_filters = background_filters;
+
+  DCHECK(quad_list.isEmpty());
+  DCHECK(shared_quad_state_list.isEmpty());
+}
 
-    bool isReplica = false;
-    layer->renderSurface()->appendQuads(quadCuller, appendQuadsData, isReplica, contributingRenderPass->id());
+void RenderPass::AppendQuadsForLayer(

[aelias_OOO_until_Jul13, 2012/11/21 01:50:10]

These methods aren't very readable without the "m_"s.  Local and member
variables are mixed together.  And they only operate on
m_quadList/m_sharedQuadStateList anyway.

I'm thinking we should probably move them out of the class.  Maybe make them
anonymous namespace functions in layer_tree_host_impl.cc?

[danakj, 2012/11/21 01:53:02]

Ya that's reasonable, they aren't used by tests.

+    LayerImpl* layer,
+    OcclusionTrackerImpl& occlusion_tracker,
+    AppendQuadsData& append_quads_data) {
 
-    // Add replica after the surface so that it appears below the surface.
-    if (layer->hasReplica()) {
-        isReplica = true;
-        layer->renderSurface()->appendQuads(quadCuller, appendQuadsData, isReplica, contributingRenderPass->id());
-    }
+  bool for_surface = false;
+  QuadCuller quad_culler(quad_list,
+                         shared_quad_state_list,
+                         layer,
+                         occlusion_tracker,
+                         layer->showDebugBorders(),
+                         for_surface);
+
+  layer->appendQuads(quad_culler, append_quads_data);
 }
 
-void RenderPass::appendQuadsToFillScreen(LayerImpl* rootLayer, SkColor screenBackgroundColor, const OcclusionTrackerImpl& occlusionTracker)
-{
-    if (!rootLayer || !screenBackgroundColor)
-        return;
+void RenderPass::AppendQuadsForRenderSurfaceLayer(
+    LayerImpl* layer,
+    const RenderPass& contributing_render_pass,
+    OcclusionTrackerImpl& occlusion_tracker,
+    AppendQuadsData& append_quads_data) {
+
+  bool for_surface = true;
+  QuadCuller quad_culler(quad_list,
+                         shared_quad_state_list,
+                         layer,
+                         occlusion_tracker,
+                         layer->showDebugBorders(),
+                         for_surface);
+
+  bool is_replica = false;
+  layer->renderSurface()->appendQuads(quad_culler,
+                                      append_quads_data,
+                                      is_replica,
+                                      contributing_render_pass.id);
+
+  // Add replica after the surface so that it appears below the surface.
+  if (layer->hasReplica()) {
+    is_replica = true;
+    layer->renderSurface()->appendQuads(quad_culler,
+                                        append_quads_data,
+                                        is_replica,
+                                        contributing_render_pass.id);
+  }
+}
 
-    Region fillRegion = occlusionTracker.computeVisibleRegionInScreen();
-    if (fillRegion.IsEmpty())
-        return;
+void RenderPass::AppendQuadsToFillScreen(
+    LayerImpl* root_layer,
+    SkColor screen_background_color,
+    const OcclusionTrackerImpl& occlusion_tracker) {
 
-    bool forSurface = false;
-    QuadCuller quadCuller(m_quadList, m_sharedQuadStateList, rootLayer, &occlusionTracker, rootLayer->showDebugBorders(), forSurface);
+  if (!root_layer || !SkColorGetA(screen_background_color))
+    return;
+
+  Region fill_region = occlusion_tracker.computeVisibleRegionInScreen();
+  if (fill_region.IsEmpty())
+    return;
+
+  bool for_surface = false;
+  QuadCuller quad_culler(quad_list,
+                         shared_quad_state_list,
+                         root_layer,
+                         occlusion_tracker,
+                         root_layer->showDebugBorders(),
+                         for_surface);
 
     // Manually create the quad state for the gutter quads, as the root layer
     // doesn't have any bounds and so can't generate this itself.
     // FIXME: Make the gutter quads generated by the solid color layer (make it smarter about generating quads to fill unoccluded areas).
 
-    DCHECK(rootLayer->screenSpaceTransform().isInvertible());
+    DCHECK(root_layer->screenSpaceTransform().isInvertible());
 
-    gfx::Rect rootTargetRect = rootLayer->renderSurface()->contentRect();
+    gfx::Rect root_target_rect = root_layer->renderSurface()->contentRect();
     float opacity = 1;
-    SharedQuadState* sharedQuadState = quadCuller.useSharedQuadState(SharedQuadState::Create());
-    sharedQuadState->SetAll(rootLayer->drawTransform(), rootTargetRect, rootTargetRect, opacity);
-
-    WebTransformationMatrix transformToLayerSpace = rootLayer->screenSpaceTransform().inverse();
-    for (Region::Iterator fillRects(fillRegion); fillRects.has_rect(); fillRects.next()) {
-        // The root layer transform is composed of translations and scales only, no perspective, so mapping is sufficient.
-        gfx::Rect layerRect = MathUtil::mapClippedRect(transformToLayerSpace, fillRects.rect());
-        // Skip the quad culler and just append the quads directly to avoid occlusion checks.
-        scoped_ptr<SolidColorDrawQuad> quad = SolidColorDrawQuad::Create();
-        quad->SetNew(sharedQuadState, layerRect, screenBackgroundColor);
-        m_quadList.append(quad.PassAs<DrawQuad>());
+    SharedQuadState* shared_quad_state = quad_culler.useSharedQuadState(
+        SharedQuadState::Create());
+    shared_quad_state->SetAll(root_layer->drawTransform(),
+                              root_target_rect,
+                              root_target_rect,
+                              opacity);
+
+    WebTransformationMatrix transform_to_layer_space =
+        root_layer->screenSpaceTransform().inverse();
+    Region::Iterator fill_rects(fill_region);

[aelias_OOO_until_Jul13, 2012/11/21 01:50:10]

nit: why not keep this inside the for()?

[danakj, 2012/11/21 01:53:02]

80 columns nonsense, and splitting the for over 3 lines looked dumb.. I'll put
it back inside and forget about 80 cols for now.

+    for (; fill_rects.has_rect(); fill_rects.next()) {
+      // The root layer transform is composed of translations and scales only,
+      // no perspective, so mapping is sufficient.
+      gfx::Rect layer_rect = MathUtil::mapClippedRect(
+          transform_to_layer_space, fill_rects.rect());
+      // Skip the quad culler and just append the quads directly to avoid
+      // occlusion checks.
+      scoped_ptr<SolidColorDrawQuad> quad = SolidColorDrawQuad::Create();
+      quad->SetNew(shared_quad_state, layer_rect, screen_background_color);
+      quad_list.append(quad.PassAs<DrawQuad>());
     }
 }
 
-void RenderPass::setFilter(SkImageFilter* filter) {
-    SkRefCnt_SafeAssign(m_filter, filter);
-}
-
 }  // namespace cc