Implement on demand quad rasterization for PicturePiles.

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index bd4aa0f788bd7620d53802614b34b68e20602b1d..ff7941a75f7ed3956eb8494a5e303dd37193422a 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -12,6 +12,7 @@
 #include "cc/layers/quad_sink.h"
 #include "cc/quads/checkerboard_draw_quad.h"
 #include "cc/quads/debug_border_draw_quad.h"
+#include "cc/quads/picture_draw_quad.h"
 #include "cc/quads/solid_color_draw_quad.h"
 #include "cc/quads/tile_draw_quad.h"
 #include "cc/trees/layer_tree_impl.h"
@@ -120,6 +121,9 @@ void PictureLayerImpl::AppendQuads(QuadSink* quadSink,
             mode == ManagedTileState::DrawingInfo::TRANSPARENT_MODE) {
           color = DebugColors::SolidColorTileBorderColor();
           width = DebugColors::SolidColorTileBorderWidth(layer_tree_impl());
+        } else if (mode == ManagedTileState::DrawingInfo::PICTURE_PILE_MODE) {
+          color = DebugColors::PictureTileBorderColor();
+          width = DebugColors::PictureTileBorderWidth(layer_tree_impl());
         } else if (iter->priority(ACTIVE_TREE).resolution == HIGH_RESOLUTION) {
           color = DebugColors::HighResTileBorderColor();
           width = DebugColors::HighResTileBorderWidth(layer_tree_impl());
@@ -180,7 +184,8 @@ void PictureLayerImpl::AppendQuads(QuadSink* quadSink,
 
     const ManagedTileState::DrawingInfo& drawing_info = iter->drawing_info();
     switch (drawing_info.mode()) {
-      case ManagedTileState::DrawingInfo::TEXTURE_MODE: {
+      case ManagedTileState::DrawingInfo::TEXTURE_MODE:
+      case ManagedTileState::DrawingInfo::PICTURE_PILE_MODE: {
         if (iter->contents_scale() != ideal_contents_scale_)
           appendQuadsData->hadIncompleteTile = true;
 
@@ -188,15 +193,30 @@ void PictureLayerImpl::AppendQuads(QuadSink* quadSink,
         gfx::Rect opaque_rect = iter->opaque_rect();
         opaque_rect.Intersect(content_rect);
 
-        scoped_ptr<TileDrawQuad> quad = TileDrawQuad::Create();
-        quad->SetNew(sharedQuadState,
-                     geometry_rect,
-                     opaque_rect,
-                     drawing_info.get_resource_id(),
-                     texture_rect,
-                     iter.texture_size(),
-                     drawing_info.contents_swizzled());
-        quadSink->Append(quad.PassAs<DrawQuad>(), appendQuadsData);
+        if (drawing_info.mode() ==
+            ManagedTileState::DrawingInfo::TEXTURE_MODE) {

[vmpstr, 2013/03/21 16:34:11]

This still looks weird to me

switch (foo) {
  case A:
  case B:
    ...
    if (foo == A) {
      ...
    } else { // foo == B
      ...
    }
    break;
}

I would vote for having separate case A and case B, even if it results in some
code duplication. @nduca @danakj, wdyt?

[Leandro Graciá Gil, 2013/03/21 16:42:48]

More than code duplication I'd be concerned about diverging copies. Any changes
in one code should be always copied to the other or we might eventually have
subtle bugs. If maintaining this duplication is something we find acceptable
then I'll split this.

[danakj, 2013/03/21 17:20:22]

What about a block above that is "if TEXTURE MODE or PICTURE_PILE_MODE" where we
can stick common code - which would be just the hadIncompleteTile piece I think?

Then a separate case for each. The texture_rect/opaque_rect stuff is pretty
obvious/small and I have more confidence we can maintain that well. But things
like setting flags I'd worry more about, hence the above thought.

I also am ok with this how it is. But maybe a "mode" temp var would be helpful
for readability. The switch and if could both use the var, and avoid line
wrappings as well.

+          scoped_ptr<TileDrawQuad> quad = TileDrawQuad::Create();

[nduca, 2013/03/21 02:22:48]

seems like the constrcturo and append call can be outside this branch, no?

[Leandro Graciá Gil, 2013/03/21 03:35:50]

The type of the scoped_ptr changes and therefore quad->SetNew would refer to
different methods. I would move the Append call outside, but that would require
moving quad outside too and we'd have the type problem again.

The alternative would be to define quad as scoped_ptr<TileDrawQuadBase> and add
ugly static_casts on quad when calling SetNew inside the if block. That way both
Create and Append would be outside the block. If that sounds acceptable let me
know and I'll do it.

+          quad->SetNew(sharedQuadState,
+                       geometry_rect,
+                       opaque_rect,
+                       drawing_info.get_resource_id(),
+                       texture_rect,
+                       iter.texture_size(),
+                       drawing_info.contents_swizzled());
+          quadSink->Append(quad.PassAs<DrawQuad>(), appendQuadsData);
+        } else {
+          scoped_ptr<PictureDrawQuad> quad = PictureDrawQuad::Create();
+          quad->SetNew(sharedQuadState,
+                       geometry_rect,
+                       opaque_rect,
+                       texture_rect,
+                       iter.texture_size(),
+                       drawing_info.contents_swizzled(),
+                       iter->content_rect(),
+                       iter->contents_scale(),
+                       pile_);
+          quadSink->Append(quad.PassAs<DrawQuad>(), appendQuadsData);
+        }
         break;
       }
       case ManagedTileState::DrawingInfo::SOLID_COLOR_MODE: {
@@ -209,8 +229,6 @@ void PictureLayerImpl::AppendQuads(QuadSink* quadSink,
       }
       case ManagedTileState::DrawingInfo::TRANSPARENT_MODE:
         break;
-      case ManagedTileState::DrawingInfo::PICTURE_PILE_MODE:
-        // TODO: crbug.com/173011 would fill this part in.
       default:
         NOTREACHED();
     }