[Remoting Android] Cursor & Cursor Feedback for OpenGL Renderer

remoting/client/gl_cursor_feedback.cc

+// 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.
+
+#include "remoting/client/gl_cursor_feedback.h"
+
+#include <math.h>
+#include <array>
+
+#include "base/logging.h"
+#include "remoting/client/gl_canvas.h"
+#include "remoting/client/gl_math.h"
+#include "remoting/client/gl_render_layer.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"

[Sergey Ulanov, 2016/07/19 00:42:48]

I don't think we want to pull this dependency here just for
webrtc::DesktopFrame::kBytesPerPixel. webrtc::DesktopFrame::kBytesPerPixel makes
sense when working with DesktopFrame, but this class shouldn't depend on
DesktopFrame

[Yuwei, 2016/07/19 20:34:23]

Done. Reuse the one in remoting/base/util

+
+namespace {
+
+const int kTextureId = 2;
+const int kColorRingsCount = 4;
+const int kFeedbackTexturePixelDiameter = 256;

[Sergey Ulanov, 2016/07/19 00:42:48]

Problem with the way animation is currently implemented is that it ignores
screen DPI.  It's rather small on modern devices. E.g. Galaxy S7 has screen with
DPI of 577, so 256 pixels is about 11mm on that screen, i.e. it's super tiny and
hard to notice. Can you please fix that with the new renderer? At very least add
a TODO here.

[Yuwei, 2016/07/19 04:11:52]

I think the actual problem of the current implementation is that in Java it hard
codes the radius of the feedback circle in pixels so the small feedback circle
is just 80px wide. To fix this we can instead define the radius in dp and
convert it back to px when using it.

The 256 pixels here is actually just the dimension of the texture. We are still
able to draw the low-res texture in larger size with some interpolations,
although it won't look very smooth in that case. I can simply change this value
to 512 to improve its quality in higher DPI devices...

[Yuwei, 2016/07/19 20:34:23]

Leaved comment in Java GlDesktopView.

[Sergey Ulanov, 2016/07/20 18:06:11]

SGTM

+const float kAnimationDurationMs = 220.f;
+
+// RGBA8888 colors. From inside to outside.
+const uint8_t kFeedbackRingColors[kColorRingsCount]
+                                 [webrtc::DesktopFrame::kBytesPerPixel] = {
+    {0, 0, 0, 0},  // transparent black
+    {0xff, 0xff, 0xff, 0xff},  // white
+    {0, 0, 0, 0xff},  // black
+    {0, 0, 0, 0}  // transparent black
+};
+
+const float kFeedbackRadiusStops[kColorRingsCount] = {0.0f, 0.8f, 0.9f, 1.0f};
+
+}  // namespace
+
+namespace remoting {
+
+GlCursorFeedback::GlCursorFeedback() {
+}

[Sergey Ulanov, 2016/07/19 00:42:48]

git cl format please

[Yuwei, 2016/07/19 20:34:23]

Done.

+
+GlCursorFeedback::~GlCursorFeedback() {
+}
+
+void GlCursorFeedback::SetCanvas(GlCanvas* canvas) {
+  if (!canvas) {
+    layer_.reset();
+    return;
+  }
+  layer_.reset(new GlRenderLayer(kTextureId, canvas));
+  layer_->SetTexture(GetFeedbackTexture(), kFeedbackTexturePixelDiameter,
+                     kFeedbackTexturePixelDiameter);
+}
+
+void GlCursorFeedback::StartAnimation(float normalized_x,
+                                      float normalized_y,
+                                      float normalized_width,
+                                      float normalized_height) {
+  max_width_ = normalized_width;
+  max_height_ = normalized_height;
+  cursor_x_ = normalized_x;
+  cursor_y_ = normalized_y;
+  animation_start_time_ = base::TimeTicks::Now();
+}
+
+bool GlCursorFeedback::Draw() {
+  if (!layer_ || animation_start_time_.is_null()) {
+    return false;
+  }
+  float progress = (base::TimeTicks::Now() - animation_start_time_)
+      .InMilliseconds() / kAnimationDurationMs;

[Sergey Ulanov, 2016/07/19 00:42:48]

git cl format please

[Yuwei, 2016/07/19 20:34:23]

Done.

+  if (progress > 1) {
+    animation_start_time_ = base::TimeTicks();
+    return false;
+  }
+  float width = progress * max_width_;
+  float height = progress * max_height_;
+  std::array<float, 8> positions;
+  FillRectangleVertexPositions(positions, cursor_x_ - width / 2,
+                               cursor_y_ - height / 2, width, height);
+  layer_->SetVertexPositions(positions);
+  layer_->Draw(1.f - progress);
+  return true;
+}
+
+void FillColorByRadius(float radius, uint8_t* out) {
+  for (int i = kColorRingsCount - 1; i > -1; i--) {

[Sergey Ulanov, 2016/07/19 00:42:48]

i >= 0 instead of i > -1

[Yuwei, 2016/07/19 20:34:23]

Obsolete.

+    if (radius >= kFeedbackRadiusStops[i]) {

[Sergey Ulanov, 2016/07/19 00:42:48]

It would be better to break here and then move the rendering logic out of the
for loop, but please see my next comment first.

[Yuwei, 2016/07/19 20:34:23]

Obsolete.

+      if (i == kColorRingsCount - 1) {
+        // Area outside the circle. Just use the outermost color.
+        memcpy(out, kFeedbackRingColors[i],
+               webrtc::DesktopFrame::kBytesPerPixel);
+      } else {
+        const uint8_t* first_color = kFeedbackRingColors[i];
+        const uint8_t* second_color = kFeedbackRingColors[i + 1];
+        float first_radius = kFeedbackRadiusStops[i];
+        float second_radius = kFeedbackRadiusStops[i + 1];
+        float interpolation_percentage =
+            (radius - first_radius) / (second_radius - first_radius);
+        LinearInterpolate(out, first_color, second_color,
+                          interpolation_percentage,
+                          webrtc::DesktopFrame::kBytesPerPixel);
+      }
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
+// static
+const uint8_t* GlCursorFeedback::GetFeedbackTexture() {
+  if (texture_) {
+    return texture_.get();
+  }
+  texture_.reset(new uint8_t[kFeedbackTexturePixelDiameter *
+                             kFeedbackTexturePixelDiameter *
+                             webrtc::DesktopFrame::kBytesPerPixel]);
+  int pixel_radius = kFeedbackTexturePixelDiameter / 2;
+  for (int x = 0; x < kFeedbackTexturePixelDiameter; x++) {

[Sergey Ulanov, 2016/07/19 00:42:48]

This code looks like it would be rather slow. It might be acceptable given that
it's called only once, but there are some simple optimizations you can do. E.g.
draw one quarter and then copy it to 4 other quarters mirroring as necessary.
Also avoid converting between FP and int types point when possible.

If we were to use it more than once that I think it would be better to use skia
to draw gradients here (see https://skia.org/user/api/skpaint#ShShader).

[Yuwei, 2016/07/19 20:34:23]

Done. Using Skia.

+    for (int y = 0; y < kFeedbackTexturePixelDiameter; y++) {
+      float radius = sqrt((x - pixel_radius) * (x - pixel_radius)
+                          + (y - pixel_radius) * (y - pixel_radius))
+                              / pixel_radius;
+      uint8_t* current = texture_.get() +
+          (y * kFeedbackTexturePixelDiameter + x) *
+          webrtc::DesktopFrame::kBytesPerPixel;
+      FillColorByRadius(radius, current);
+    }
+  }
+  return texture_.get();
+}
+
+// static
+std::unique_ptr<uint8_t[]> GlCursorFeedback::texture_;

[Sergey Ulanov, 2016/07/19 00:42:48]

only POD types are allowed for static variables.

[Yuwei, 2016/07/19 04:11:52]

Yeah I don't think it make sense to have a static unique_ptr... Should I just
new a uint8_t[] and just not delete it?

[Yuwei, 2016/07/19 20:34:23]

Done.

+
+}  // namespace remoting