Adding drawable to CRD andorid and iOS gl rendering pipeline.

remoting/client/display/drawable.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_CLIENT_DISPLAY_DRAWABLE_H_
+#define REMOTING_CLIENT_DISPLAY_DRAWABLE_H_
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+
+namespace remoting {
+
+class Canvas;
+
+enum DrawableZIndex {

[Sergey Ulanov, 2017/01/10 23:24:01]

please nest it in Drawable class. Otherwise the values from this enum pollute
remoting namespace. Also do you need to set values for each enum value instead
of using defaults? Will we ever need z-index values in 100 and 200?

[nicholss, 2017/01/11 18:56:43]

The intent was that these are guide-lines and if we have something that wants to
draw between a desktop and a cursor, there will be room without needing to
update this enum. I have adjusted the comment and the name of the enum to say as
much.

+  AUTO = -1,
+  DESKTOP = 100,
+  CURSOR_FEEDBACK = 200,
+  CURSOR = 300,
+};
+
+class Drawable {

[Sergey Ulanov, 2017/01/10 23:24:01]

add a short comment to explain what this class is used for

+ public:
+  Drawable() {}
+  virtual ~Drawable(){};

[Sergey Ulanov, 2017/01/10 23:24:01]

remove ;

+
+  // Sets the canvas on which the object will be drawn.
+  // If |canvas| is nullptr, nothing will happen when calling Draw().
+  virtual void SetCanvas(base::WeakPtr<Canvas> canvas) = 0;
+
+  // Draws the object on the canvas.
+  // Returns true if there is a pending next frame.
+  virtual bool Draw() = 0;
+
+  // Used for the renderer to keep a stack of drawables.
+  virtual base::WeakPtr<Drawable> GetWeakPtr() = 0;
+
+  int GetZIndex() { return z_index_; };

[Sergey Ulanov, 2017/01/10 23:24:01]

get_z_index()?

[nicholss, 2017/01/11 18:56:43]

I am not sure what this comment is asking for. GetZIndex to get_z_index? That is
not inline with coding styles in remoting unless. Changing to just ZIndex (get
and set was used to make it settable and was attempting to not change existing
classes...)

[Yuwei, 2017/01/11 19:16:58]

Getters and setters are often named with underscores.

https://google.github.io/styleguide/cppguide.html#Function_Names

[Sergey Ulanov, 2017/01/11 22:47:31]

Normally set_var() naming is used only for methods that can be inlined. I
remember the style guide had requirement that these functions should be as cheap
to call as accessing variable, but it doesn't look like it mentions it anymore.
Any virtual functions arguably don't fall in this category.
In either case I'd prefer to reserve lower-case names for inlined
setters/getters, i.e. GetZIndex() is more appropriate here.

[Sergey Ulanov, 2017/01/11 22:47:31]

Sorry, ignore this comment please - I wrote it before I left the other one about
z_index_ and forgot to delete this one.

+
+  // Specify the Z Index for this drawable. A higher Z Index will draw after a
+  // lower z index. Elements with the same Z Index will draw in order inserted.
+  void SetZIndex(int z_index) { z_index_ = z_index; };
+
+ private:
+  int z_index_ = DrawableZIndex::AUTO;

[Sergey Ulanov, 2017/01/10 23:24:01]

I don't think we need SetZIndex or z_index_. Instead it would be cleaner for
child classes to override GetZIndex() to return z-index they want.

[nicholss, 2017/01/11 18:56:43]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(Drawable);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_CLIENT_DISPLAY_DRAWABLE_H_