[Remoting Android] Create Interfaces for GlDisplay

remoting/android/java/src/org/chromium/chromoting/RenderStub.java

Index: remoting/android/java/src/org/chromium/chromoting/RenderStub.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/RenderStub.java b/remoting/android/java/src/org/chromium/chromoting/RenderStub.java
new file mode 100644
index 0000000000000000000000000000000000000000..6710c964a13e6b396fef37a2d8cc94f9d774c8af
--- /dev/null
+++ b/remoting/android/java/src/org/chromium/chromoting/RenderStub.java
@@ -0,0 +1,89 @@
+// 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.
+
+package org.chromium.chromoting;
+
+import android.graphics.Matrix;
+import android.graphics.PointF;
+
+/**
+ * Interface with a set of functions to control the behavior of the remote host renderer.
+ */
+public interface RenderStub {
+    /** Used to define the animation feedback shown when a user touches the screen. */
+    enum InputFeedbackType {
+        NONE,
+        SHORT_TOUCH_ANIMATION,
+        LONG_TOUCH_ANIMATION,
+        LONG_TRACKPAD_ANIMATION
+    }
+
+    void setDesktopView(DesktopView view);
+
+    /** Triggers a brief animation to indicate the existence and location of an input event. */
+    void showInputFeedback(InputFeedbackType feedbackToShow, PointF pos);
+
+    /**
+     * Informs the view that its transformation matrix (for rendering the remote desktop bitmap)

[Hzj_jie, 2016/08/29 22:52:51]

Maybe view -> stub, ditto.

[Yuwei, 2016/08/30 22:39:12]

Done.

+     * has been changed by the TouchInputHandler, which requires repainting.
+     */
+    void setTransformation(Matrix matrix);
+
+    /**
+     * Informs the view that the cursor has been moved by the TouchInputHandler, which requires

[joedow, 2016/08/29 22:35:41]

I'd remove 'TouchInputHandler' as we could introduce a new class tomorrow that
calls into the RenderStub and this comment would be outdated.  Same with all of
the other comments.

[Yuwei, 2016/08/30 22:39:12]

Done.

+     * repainting.
+     */
+    void moveCursor(PointF pos);
+
+    /**
+     * Informs the view that the cursor visibility has been changed (for different input mode) by
+     * the TouchInputHandler, which requires repainting.
+     */
+    void setCursorVisibility(boolean visible);
+
+    /**
+     * Helper class for mapping a feedback type to the max radius of the feedback animation.
+     */
+    final class InputFeedbackRadiusMapper {

[Yuwei, 2016/08/29 22:09:42]

Not sure whether it is good to make this a subclass of RenderStub. Maybe make it
a standalone class?

[joedow, 2016/08/29 22:35:41]

I think it is ok here but I'm wondering if it even needs to be exposed by the
RenderStub.  Could this interface expose a method that receives an
InputFeedbackType and a position and internally this class figures out what to
draw?

[Hzj_jie, 2016/08/29 22:52:51]

The original design is to use a float diameter to indicate the size of the
feedback (GlDisplay.showCursorInputFeedback). Is there any reason to change it
the InputFeedbackType? From my opinion, a void showFeedback(float size, PointF
pos); may be more useful.

[Yuwei, 2016/08/29 23:38:21]

I think there are two issues:
* Default function in interface is not allowed until N SDK is deployed.
* As Lambros pointed out in previous CL, how the feedback type is rendered is
the renderer's implementation detail (say if a renderer implements vibration
feedback) so we probably don't want to handle the conversion inside RenderStub.

[Yuwei, 2016/08/30 22:08:00]

I think the question here is whether we want all implementations to show a
circle for touch feedback and disallow any other forms of feedback in the
future.

[joedow, 2016/08/30 23:08:33]

My understanding of Lambros' point was that if you had an enum which had 'small'
and 'large' types then it's possible that a future implementation could
interpret these values differently.  I think that exposing a method which
receives a float param called diameter and a position is just as inflexible
(i.e. if we decided to use an audio tone (for example) as feedback, you'd need
another method anyway).

My point is why should any of the classes other than the renderer care how big
the radius is.  From an input strategy standpoint, they are defining the type of
animation which should be rendered.  The renderer knows how big to draw it.

Wouldn't it be simpler to say something like: drawAnimation(center_point,
SMALL_TOUCH_ANIMATION) and have the renderer figure out size.  Later we could do
some thing like drawAnimation(center_point, BLUE_ALPAH_SQUARE) and the renderer
would take care of it.

+        private final int mTinyFeedbackPixelRadius;
+        private final int mSmallFeedbackPixelRadius;
+        private final int mLargeFeedbackPixelRadius;
+
+        public InputFeedbackRadiusMapper(DesktopView view) {
+            mTinyFeedbackPixelRadius = view.getResources()
+                    .getDimensionPixelSize(R.dimen.feedback_animation_radius_tiny);
+
+            mSmallFeedbackPixelRadius = view.getResources()
+                    .getDimensionPixelSize(R.dimen.feedback_animation_radius_small);
+
+            mLargeFeedbackPixelRadius = view.getResources()
+                    .getDimensionPixelSize(R.dimen.feedback_animation_radius_large);
+        }
+
+        /**
+         * Returns the radius of the given feedback type.
+         * 0.0f will be returned if no feedback should be shown.
+         */
+        public float getFeedbackRadius(InputFeedbackType feedbackToShow,
+                                       float scaleFactor) {
+            switch (feedbackToShow) {
+                case NONE:
+                    return 0.0f;
+                case SHORT_TOUCH_ANIMATION:
+                    return mSmallFeedbackPixelRadius / scaleFactor;
+                case LONG_TOUCH_ANIMATION:
+                    return mLargeFeedbackPixelRadius / scaleFactor;
+                case LONG_TRACKPAD_ANIMATION:
+                    // The size of the longpress trackpad animation is supposed to be close to the
+                    // size of the cursor so it doesn't need to be normalized and should be scaled
+                    // with the canvas.
+                    return mTinyFeedbackPixelRadius;
+                default:
+                    // Unreachable, but required by Google Java style and findbugs.
+                    assert false : "Unreached";
+                    return 0.0f;
+            }
+        }
+    }
+}