cc mojo: Use ArrayDataViews in RenderPasses

cc/ipc/quads_struct_traits.h

Index: cc/ipc/quads_struct_traits.h
diff --git a/cc/ipc/quads_struct_traits.h b/cc/ipc/quads_struct_traits.h
index ed602beb8f1aed81776fde2c93b702e2fca5fa19..a35645704fb2e2d173bd7ccf22432ec46e4c8cfa 100644
--- a/cc/ipc/quads_struct_traits.h
+++ b/cc/ipc/quads_struct_traits.h
@@ -9,6 +9,7 @@
 #include "cc/ipc/filter_operations_struct_traits.h"
 #include "cc/ipc/quads.mojom.h"
 #include "cc/ipc/render_pass_id_struct_traits.h"
+#include "cc/ipc/shared_quad_state_struct_traits.h"
 #include "cc/ipc/surface_id_struct_traits.h"
 #include "cc/quads/debug_border_draw_quad.h"
 #include "cc/quads/picture_draw_quad.h"
@@ -23,6 +24,8 @@
 
 namespace mojo {
 
+cc::DrawQuad* AllocateAndConstruct(cc::mojom::Material material,
+                                   cc::QuadList* list);
 template <>
 struct EnumTraits<cc::mojom::Material, cc::DrawQuad::Material> {
   static cc::mojom::Material ToMojom(cc::DrawQuad::Material material);
@@ -390,6 +393,83 @@ struct StructTraits<cc::mojom::YUVVideoQuadState, cc::DrawQuad> {
                    cc::DrawQuad* out);
 };
 
+struct DrawQuadWithSharedQuadState {
+  cc::DrawQuad* quad;
+  bool include_sqs;
+};
+
+template <>
+struct StructTraits<cc::mojom::DrawQuad, DrawQuadWithSharedQuadState> {
+  static cc::DrawQuad::Material material(
+      const DrawQuadWithSharedQuadState& input) {
+    return input.quad->material;
+  }
+
+  static const gfx::Rect& rect(const DrawQuadWithSharedQuadState& input) {
+    return input.quad->rect;
+  }
+
+  static const gfx::Rect& opaque_rect(
+      const DrawQuadWithSharedQuadState& input) {
+    return input.quad->opaque_rect;
+  }
+
+  static const gfx::Rect& visible_rect(
+      const DrawQuadWithSharedQuadState& input) {
+    return input.quad->visible_rect;
+  }
+
+  static bool needs_blending(const DrawQuadWithSharedQuadState& input) {
+    return input.quad->needs_blending;
+  }
+
+  static const cc::DrawQuad& debug_border_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& render_pass_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& solid_color_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& surface_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& texture_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& tile_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& stream_video_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static const cc::DrawQuad& yuv_video_quad_state(
+      const DrawQuadWithSharedQuadState& input) {
+    return *input.quad;
+  }
+
+  static OptSharedQuadState sqs(const DrawQuadWithSharedQuadState& input) {
+    OptSharedQuadState sqs;
+    sqs.sqs = input.include_sqs ? input.quad->shared_quad_state : nullptr;
+    return sqs;
+  }
+};
+
 template <>
 struct StructTraits<cc::mojom::DrawQuad, cc::DrawQuad> {

[yzshen1, 2016/07/26 17:56:03]

Is this StructTraits still needed after we introduce the one above?

[Fady Samuel, 2016/07/27 23:52:22]

No, I added a TODO to move over tests.

   static cc::DrawQuad::Material material(const cc::DrawQuad& quad) {
@@ -410,6 +490,10 @@ struct StructTraits<cc::mojom::DrawQuad, cc::DrawQuad> {
     return quad.needs_blending;
   }
 
+  static cc::mojom::SharedQuadStatePtr sqs(const cc::DrawQuad& quad) {
+    return nullptr;
+  }
+
   static const cc::DrawQuad& debug_border_quad_state(const cc::DrawQuad& quad) {
     return quad;
   }
@@ -449,11 +533,12 @@ struct QuadListArray {
   // This is the expected size of the array.
   size_t size;
   cc::QuadList* list;
+  const cc::DrawQuad* last_quad;
 };
 
 template <>
 struct ArrayTraits<QuadListArray> {
-  using Element = cc::DrawQuad;
+  using Element = DrawQuadWithSharedQuadState;
   using Iterator = cc::QuadList::Iterator;
   using ConstIterator = cc::QuadList::ConstIterator;
 
@@ -461,10 +546,31 @@ struct ArrayTraits<QuadListArray> {
     return input.list->begin();
   }
   static Iterator GetBegin(QuadListArray& input) { return input.list->begin(); }
-  static void AdvanceIterator(ConstIterator& iterator) { ++iterator; }
-  static void AdvanceIterator(Iterator& iterator) { ++iterator; }
-  static const Element& GetValue(ConstIterator& iterator) { return **iterator; }
-  static Element& GetValue(Iterator& iterator) { return **iterator; }
+  static void AdvanceIterator(QuadListArray& input, ConstIterator& iterator) {

[yzshen1, 2016/07/26 17:56:03]

This API change seems pretty weird. |input| is non-const. :/

I have an idea: define a new iterator:

CustomConstIterator {
  const cc::DrawQuad* last_quad;
  cc::QuadList::ConstIterator;
};

(Because you only use this ArrayTraits for reading, those non-const
GetBegin/AdvanceIterator/... are not needed, I believe)


That way we don't need to introduce |input|.

[Fady Samuel, 2016/07/27 23:52:23]

Done.

+    input.last_quad = *iterator;
+    ++iterator;
+  }
+  static void AdvanceIterator(QuadListArray& input, Iterator& iterator) {
+    input.last_quad = *iterator;
+    ++iterator;
+  }
+  static const Element& GetValue(QuadListArray& input,
+                                 ConstIterator& iterator) {
+    static DrawQuadWithSharedQuadState dq;

[yzshen1, 2016/07/26 17:56:03]

This is pretty hacky. It won't work, for example, in multi-threaded environment.
Comparing with this, I would rather we support GetValue() returning a value (in
addition to returning a ref).

Have you considered the following alternatives:
(1) In the SetUpContext of RenderPass, const_cast the input so that we can
modify the contents, and then set |shared_quad_state| to null for quads as
needed. And in TearDownContext we restore those values. That simplifies the code
quite a bit.

(2) Or actually setup a real DrawQuadWithSharedQuadState array (and cache it as
context of the owning struct).

I wonder how the performance of those alternatives is. 

[Fady Samuel, 2016/07/27 23:52:22]

I added support for returning Element by value.

+    dq.quad = const_cast<cc::DrawQuad*>(*iterator);
+    dq.include_sqs =
+        !input.last_quad ||
+        input.last_quad->shared_quad_state != (*iterator)->shared_quad_state;
+    return dq;
+  }
+  static Element& GetValue(QuadListArray& input, Iterator& iterator) {
+    static DrawQuadWithSharedQuadState dq;
+    dq.quad = *iterator;
+    dq.include_sqs =
+        !input.last_quad ||
+        input.last_quad->shared_quad_state != (*iterator)->shared_quad_state;
+    return dq;
+  }
   static size_t GetSize(const QuadListArray& input) {
     return input.list->size();
   }
@@ -475,13 +581,8 @@ struct ArrayTraits<QuadListArray> {
 
 template <>
 struct StructTraits<cc::mojom::QuadList, cc::QuadList> {

[yzshen1, 2016/07/26 17:56:03]

Is it possible that we remove cc::mojom::QuadList? (That means we change the
cc::mojom::RenderPass::quad_list's type to array<DrawQuad>.)

[Fady Samuel, 2016/07/27 23:52:23]

Done.

-  static void* SetUpContext(const cc::QuadList& quad_list);
-  static void TearDownContext(const cc::QuadList& quad_list, void* context);
-  static const mojo::Array<cc::DrawQuad::Material>& quad_types(
-      const cc::QuadList& quad_list,
-      void* context);
   static QuadListArray quads(const cc::QuadList& quad_list) {
-    return {quad_list.size(), const_cast<cc::QuadList*>(&quad_list)};
+    return {quad_list.size(), const_cast<cc::QuadList*>(&quad_list), nullptr};
   }
   static bool Read(cc::mojom::QuadListDataView data, cc::QuadList* out);
 };