Allow animation in Android viewer

tools/viewer/sk_app/android/surface_glue_android.cpp

 /*
 * Copyright 2016 Google Inc.
 *
 * Use of this source code is governed by a BSD-style license that can be
 * found in the LICENSE file.
 */
 
 #include "surface_glue_android.h"
 
 #include <jni.h>
 #include <pthread.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <unordered_map>
 
 #include <android/input.h>
 #include <android/keycodes.h>
 #include <android/looper.h>
 #include <android/native_window_jni.h>
 
 #include "../Application.h"
 #include "SkTypes.h"
 #include "SkUtils.h"
 #include "Window_android.h"
+#include "Timer.h"

[scroggo, 2016/06/03 19:11:06]

I think you just use SkTIme, which is defined in SkTime.h - you do not appear to
use anything from Timer.h. Why not include SkTime.h directly?

[liyuqian, 2016/06/06 13:38:27]

Done.

 
 namespace sk_app {
 
 static const int LOOPER_ID_MESSAGEPIPE = 1;
 
 static const std::unordered_map<int, Window::Key> ANDROID_TO_WINDOW_KEYMAP({
     {AKEYCODE_SOFT_LEFT, Window::Key::kLeft},
     {AKEYCODE_SOFT_RIGHT, Window::Key::kRight}
 });
 
@@ skipping 118 matching lines @@
             break;
         }
         default: {
             // do nothing
         }
     }
 
     return 1;  // continue receiving callbacks
 }
 
+static double now_ms() { return SkTime::GetNSecs() * 1e-6; }

[scroggo, 2016/06/03 19:11:06]

Why not call SkTime::GetMSecs()?

And while we're at it, why not use SkTime::GetMSecs() at the single call site?

[liyuqian, 2016/06/06 13:38:27]

Done.

+
 void* SkiaAndroidApp::pthread_main(void* arg) {
     SkDebugf("pthread_main begins");
 
     auto skiaAndroidApp = (SkiaAndroidApp*)arg;
 
     // Because JNIEnv is thread sensitive, we need AttachCurrentThread to set our fPThreadEnv
     skiaAndroidApp->fJavaVM->AttachCurrentThread(&(skiaAndroidApp->fPThreadEnv), nullptr);
 
     ALooper* looper = ALooper_prepare(ALOOPER_PREPARE_ALLOW_NON_CALLBACKS);
     pipe(skiaAndroidApp->fPipes);
     ALooper_addFd(looper, skiaAndroidApp->fPipes[0], LOOPER_ID_MESSAGEPIPE, ALOOPER_EVENT_INPUT,
                   message_callback, skiaAndroidApp);
 
-    int ident;
-    int events;
-    struct android_poll_source* source;
-
     skiaAndroidApp->fApp = Application::Create(0, nullptr, skiaAndroidApp);
 
-    while ((ident = ALooper_pollAll(-1, nullptr, &events, (void**)&source)) >= 0) {
-        SkDebugf("ALooper_pollAll ident=%d", ident);
+    double currentTime = 0.0;
+    double previousTime = 0.0;
+    while (true) {
+        int ident;

[scroggo, 2016/06/03 19:11:06]

It's easier to follow (and less error-prone) if you declare variables where
they're used. e.g.

const int ident = ALooper_pollAll(0, nullptr, &events, (void**)&source);

[liyuqian, 2016/06/06 13:38:27]

Done.

+        int events;
+        struct android_poll_source* source;

[scroggo, 2016/06/03 19:11:06]

You don't use either of these variables. Will ALooper_pollAll let you pass
nullptr instead?

[liyuqian, 2016/06/06 13:38:27]

Unfortunately, the Android developer document doesn't say that null is allowed.
But I checked their source code and they do allow null in the current
implementation. Is it safe for me to proceed with null?

[scroggo, 2016/06/06 14:17:00]

I suppose their implementation could vary based on the version of Android? I
would recommend going on the safe side and passing pointers, just in case. But I
would give them names that specify that they are unused:

    // We do not need these values, but a ALooper_pollAll does not guarantee
that they can be null:
    int events_unused;
    struct android_poll_source* source_unused;

+        ident = ALooper_pollAll(0, nullptr, &events, (void**)&source);
+
+        if (ident >= 0) {
+            SkDebugf("Unhandled ALooper_pollAll ident=%d !", ident);
+        } else {
+            previousTime = currentTime;
+            currentTime = now_ms();
+            skiaAndroidApp->fApp->onIdle(currentTime - previousTime);
+        }
     }
 
+    SkDebugf("pthread_main ends");
+
     return nullptr;
 }
 
 extern "C"  // extern "C" is needed for JNI (although the method itself is in C++)
     JNIEXPORT jlong JNICALL
     Java_org_skia_viewer_ViewerApplication_createNativeApp(JNIEnv* env, jobject application) {
     SkiaAndroidApp* skiaAndroidApp = new SkiaAndroidApp(env, application);
     return (jlong)((size_t)skiaAndroidApp);
 }
 
@@ skipping 53 matching lines @@
     const char* nameChars = env->GetStringUTFChars(stateName, nullptr);
     const char* valueChars = env->GetStringUTFChars(stateValue, nullptr);
     message.stateName = new SkString(nameChars);
     message.stateValue = new SkString(valueChars);
     skiaAndroidApp->postMessage(message);
     env->ReleaseStringUTFChars(stateName, nameChars);
     env->ReleaseStringUTFChars(stateValue, valueChars);
 }
 
 }  // namespace sk_app