[PPAPI] Compositor API implementation

ppapi/shared_impl/compositor_layer_data.h

Index: ppapi/shared_impl/compositor_layer_data.h
diff --git a/ppapi/proxy/uma_private_resource.h b/ppapi/shared_impl/compositor_layer_data.h
similarity index 12%
copy from ppapi/proxy/uma_private_resource.h
copy to ppapi/shared_impl/compositor_layer_data.h
index 93328e8ec847580eb2a3816611cd84c66a0f8c79..4fc5a3ab862087625c9d1b625b76ca3aaa0db287 100644
--- a/ppapi/proxy/uma_private_resource.h
+++ b/ppapi/shared_impl/compositor_layer_data.h
@@ -2,61 +2,93 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef PPAPI_PROXY_UMA_PRIVATE_RESOURCE_H_
-#define PPAPI_PROXY_UMA_PRIVATE_RESOURCE_H_
+#ifndef PPAPI_SHARED_IMPL_COMPOSITOR_LAYER_DATA_H_
+#define PPAPI_SHARED_IMPL_COMPOSITOR_LAYER_DATA_H_
 
-#include "ppapi/proxy/connection.h"
-#include "ppapi/proxy/plugin_resource.h"
-#include "ppapi/proxy/ppapi_proxy_export.h"
-#include "ppapi/thunk/ppb_uma_singleton_api.h"
+#include <string.h>
+
+#include "gpu/command_buffer/common/mailbox.h"
+#include "ppapi/c/ppb_compositor_layer.h"
+#include "ppapi/shared_impl/host_resource.h"
 
 namespace ppapi {
 
-namespace proxy {
-
-class PPAPI_PROXY_EXPORT UMAPrivateResource
-    : public PluginResource,
-      public thunk::PPB_UMA_Singleton_API {
- public:
-  UMAPrivateResource(Connection connection, PP_Instance instance);
-  virtual ~UMAPrivateResource();
-
-  // Resource overrides.
-  virtual thunk::PPB_UMA_Singleton_API* AsPPB_UMA_Singleton_API() OVERRIDE;
-
-  // PPB_UMA_Singleton_API implementation.
-  virtual void HistogramCustomTimes(PP_Instance instance,
-                                    struct PP_Var name,
-                                    int64_t sample,
-                                    int64_t min,
-                                    int64_t max,
-                                    uint32_t bucket_count) OVERRIDE;
-
-  virtual void HistogramCustomCounts(PP_Instance instance,
-                                     struct PP_Var name,
-                                     int32_t sample,
-                                     int32_t min,
-                                     int32_t max,
-                                     uint32_t bucket_count) OVERRIDE;
-
-  virtual void HistogramEnumeration(PP_Instance instance,
-                                    struct PP_Var name,
-                                    int32_t sample,
-                                    int32_t boundary_value) OVERRIDE;
-
-  virtual int32_t IsCrashReportingEnabled(
-      PP_Instance instance,
-      scoped_refptr<TrackedCallback> callback) OVERRIDE;
-
- private:
-  void OnPluginMsgIsCrashReportingEnabled(
-      const ResourceMessageReplyParams& params);
-  scoped_refptr<TrackedCallback> pending_callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(UMAPrivateResource);
+struct CompositorLayerData {
+  typedef int8_t Mailbox[GL_MAILBOX_SIZE_CHROMIUM];

[piman, 2014/06/04 13:19:44]

just use gpu::Mailbox. It also has existing ParamTraits.

[Peng, 2014/06/05 00:50:38]

It has a constructor. The compiler does not allow put it in a union. :(

[piman, 2014/06/09 18:27:24]

Then maybe a union is not a good choice.

Alternatives:
- put all fields inline. Overhead is small compared to 1 mailbox + 1 transform
(and layer count is expected to be small).
- put 3 scoped_ptr for each of the fields. You only allocate memory for the
fields you actually use, at the cost of an indirection.

[Peng, 2014/06/10 16:20:28]

Use 3 scoped_ptr. Done.

+
+  enum Type {
+    TYPE_UNKNOWN = 0,
+    TYPE_COLOR,
+    TYPE_TEXTURE,
+    TYPE_IMAGE,
+    TYPE_LAST = TYPE_IMAGE,
+  };
+
+  CompositorLayerData()
+      : type(TYPE_UNKNOWN),
+        size(PP_MakeSize(0, 0)),
+        clip_rect(PP_MakeRectFromXYWH(0, 0, 0, 0)),
+        blend_mode(PP_BLENDMODE_SRC_OVER),
+        opacity(255),
+        resource_id(0) {
+    transform[0] = 1.0f;
+    transform[1] = 0.0f;
+    transform[2] = 0.0f;
+    transform[3] = 0.0f;
+    transform[4] = 0.0f;
+    transform[5] = 1.0f;
+    transform[6] = 0.0f;
+    transform[7] = 0.0f;
+    transform[8] = 0.0f;
+    transform[9] = 0.0f;
+    transform[10] = 1.0f;
+    transform[11] = 0.0f;
+    transform[12] = 0.0f;
+    transform[13] = 0.0f;
+    transform[14] = 0.0f;
+    transform[15] = 1.0f;
+
+    // Becasue texture is the biggest struct in the union, so

[piman, 2014/06/04 13:19:43]

typo becasue -> because

[Peng, 2014/06/05 00:50:38]

Done.

+    // we only need set it to 0.

[piman, 2014/06/04 13:19:44]

Technically, this is undefined behavior. Also, fragile.

If you give a name to the union, you can memset it.

[Peng, 2014/06/05 00:50:38]

If I give it a name, I think I can not use union member directly anymore. For
example:

data.color.red -> data.union_name.color.red.

Do you have a better idea to achieve it?

[piman, 2014/06/09 18:27:24]

How is that a problem?

[Peng, 2014/06/10 16:20:28]

I feel it looks a little weird. Anyway, change it to use 3 scoped_ptr. Done

+    memset(&texture, 0, sizeof(texture));
+  }
+
+  Type type;
+  PP_Size size;
+  PP_Rect clip_rect;
+  float transform[16];
+  PP_BlendMode blend_mode;
+  uint8_t opacity;
+
+  // Unique resource id for texture or image.
+  int32_t resource_id;

[raymes, 2014/06/04 01:11:39]

On the same note as my previous comments, I think this could just be called
layer_id. The host could notify the plugin when it is finished with a layer_id
(instead of a resource_id) and then the CompositorResource could just drop its
references to it.

[Peng, 2014/06/05 00:50:38]

Actually it is not a layer id. One single layer can generate many resource id. 
Each SetImage or SetTexture call will generate a resource id to present the
resource (texture or image).

+
+  union {
+    // Properties for a color layer.
+    struct {
+      uint8_t red;
+      uint8_t green;
+      uint8_t blue;
+      uint8_t alpha;
+    } color;

[piman, 2014/06/04 13:19:43]

If you define those as actual structs, you can use the IPC_STRUCT_TRAITS_*
macros which are a lot more maintainable that the individual ParamTraits.

[Peng, 2014/06/05 00:50:38]

Unless using a struct includes all fields in struct color, texture, and image, I
think we can not avoid ParamTraits. But it will add some overhead for IPC
message.

Thought?

[piman, 2014/06/09 18:27:24]

If you define a struct ColorLayerData { ... }, TextureLayerData { ... },
ImageLayerData { ... }

then you can have
union LayerData {
  ColorLayerData color;
  TextureLayerData texture;
  ImageLayerData image;
} layer_data;

Then you can define the param traits for each of
ColorLayerData/TextureLayerData/ImageLayerData using the IPC_STRUCT_TRAITS_*

Then the ParamTraits for CompositorLayerData become something like

switch (p.type) {
  case TYPE_COLOR:
    WriteParam(m, p.layer_data.color);
    break;
  case TYPE_TEXTURE:
    WriteParam(m, p.layer_data.texture);
    break;
  case TYPE_IMAGE:
    WriteParam(m, p.layer_data.image);
    break;
}

And similar for the Read.

[Peng, 2014/06/10 16:20:28]

Done. Now we only has ParamTraits for the 4x4 matrix, because we can not use
gfx::Transform which uses skia.

+
+    // Properties for a texture layer.
+    struct {
+      Mailbox mailbox;
+      uint32_t sync_point;
+      PP_FloatRect source_rect;

[piman, 2014/06/04 13:19:43]

if you use gfx::RectF, ParamTraits already exist.

[Peng, 2014/06/05 00:50:38]

Seems pepper does not use any gfx structs in IPC.

https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppapi_...

+      bool premult_alpha;
+    } texture;
+
+    // Properties for an image layer
+    struct {
+      PP_Instance instance;
+      PP_Resource host_resource;
+      PP_FloatRect source_rect;

[piman, 2014/06/04 13:19:44]

same here

[Peng, 2014/06/05 00:50:38]

Seems pepper does not use any gfx structs in IPC.

https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppapi_...

+    } image;
+  };
 };
 
-}  // namespace proxy
 }  // namespace ppapi
 
-#endif  // PPAPI_PROXY_UMA_PRIVATE_RESOURCE_H_
+#endif  // PPAPI_SHARED_IMPL_COMPOSITOR_LAYER_DATA_H_