Improve vsync provider

Now we actually use the vsync signal to trigger work. Previously, we'd hit the
pipeline depth limit too early and fall back to swap buffers-triggered
rendering.

Also, rename Vsync to VSync on recommendation from jamesr.

sky/services/vsync/BUILD.gn

@@ -27,7 +27,7 @@ if (is_android) {
   import("//build/config/android/rules.gni")
 
   android_library("vsync_lib") {
-    java_files = [ "src/org/domokit/vsync/VsyncProviderImpl.java" ]
+    java_files = [ "src/org/domokit/vsync/VSyncProviderImpl.java" ]
 
     deps = [
       "//base:base_java",

sky/services/vsync/src/org/domokit/vsync/VsyncProviderImpl.java → sky/services/vsync/src/org/domokit/vsync/VSyncProviderImpl.java

@@ -6,16 +6,21 @@ package org.domokit.vsync;
 
 import android.view.Choreographer;
 
+import org.chromium.mojo.system.MessagePipeHandle;
 import org.chromium.mojo.system.MojoException;
-import org.chromium.mojom.vsync.VsyncProvider;
+import org.chromium.mojom.vsync.VSyncProvider;
 
 /**
- * Android implementation of VsyncProvider.
+ * Android implementation of VSyncProvider.
  */
-public class VsyncProviderImpl implements VsyncProvider {
-    private static final String TAG = "VsyncProviderImpl";
-
-    public VsyncProviderImpl() {
+public class VSyncProviderImpl implements VSyncProvider, Choreographer.FrameCallback {
+    private Choreographer mChoreographer;
+    private AwaitVSyncResponse mCallback;
+    private MessagePipeHandle mPipe;
+
+    public VSyncProviderImpl(MessagePipeHandle pipe) {
+        mPipe = pipe;
+        mChoreographer = Choreographer.getInstance();
     }
 
     @Override
@@ -25,12 +30,18 @@ public class VsyncProviderImpl implements VsyncProvider {
     public void onConnectionError(MojoException e) {}
 
     @Override
-    public void awaitVsync(final AwaitVsyncResponse callback) {
-        Choreographer.getInstance().postFrameCallback(new Choreographer.FrameCallback() {
-            @Override
-            public void doFrame(long frameTimeNanos) {
-                callback.call(frameTimeNanos);
-            }
-        });
+    public void awaitVSync(final AwaitVSyncResponse callback) {
+        if (mCallback != null) {
+            mPipe.close();
+            return;
+        }
+        mCallback = callback;
+        mChoreographer.postFrameCallback(this);
+    }
+
+    @Override
+    public void doFrame(long frameTimeNanos) {
+        mCallback.call(frameTimeNanos / 1000);
+        mCallback = null;
     }
 }

sky/services/vsync/vsync.mojom

@@ -4,6 +4,8 @@
 
 module vsync;
 
-interface VsyncProvider {
-  AwaitVsync() => (int64 time_stamp);
+interface VSyncProvider {
+  // Timebase is in MojoGetTimeTicksNow.
+  // Only one callback can be parked at a given time.
+  AwaitVSync() => (int64 time_stamp);
 };

sky/shell/android/org/domokit/sky/shell/SkyApplication.java

@@ -21,11 +21,11 @@ import org.chromium.mojom.keyboard.KeyboardService;
 import org.chromium.mojom.media.MediaService;
 import org.chromium.mojom.mojo.NetworkService;
 import org.chromium.mojom.sensors.SensorService;
-import org.chromium.mojom.vsync.VsyncProvider;
+import org.chromium.mojom.vsync.VSyncProvider;
 import org.domokit.activity.ActivityImpl;
 import org.domokit.media.MediaServiceImpl;
 import org.domokit.oknet.NetworkServiceImpl;
-import org.domokit.vsync.VsyncProviderImpl;
+import org.domokit.vsync.VSyncProviderImpl;
 
 /**
  * Sky implementation of {@link android.app.Application}, managing application-level global
@@ -100,10 +100,10 @@ public class SkyApplication extends BaseChromiumApplication {
             }
         });
 
-        registry.register(VsyncProvider.MANAGER.getName(), new ServiceFactory() {
+        registry.register(VSyncProvider.MANAGER.getName(), new ServiceFactory() {
             @Override
             public void connectToService(Context context, Core core, MessagePipeHandle pipe) {
-                VsyncProvider.MANAGER.bind(new VsyncProviderImpl(), pipe);
+                VSyncProvider.MANAGER.bind(new VSyncProviderImpl(pipe), pipe);
             }
         });
     }

sky/shell/ui/animator.cc

@@ -11,7 +11,7 @@
 namespace sky {
 namespace shell {
 
-const int kPipelineDepth = 2;
+const int kPipelineDepth = 3;
 
 Animator::Animator(const Engine::Config& config, Engine* engine)
     : config_(config),
@@ -39,10 +39,7 @@ void Animator::RequestFrame() {
     return;
   }
 
-  if (vsync_provider_) {
-    vsync_provider_->AwaitVsync(
-        base::Bind(&Animator::BeginFrame, weak_factory_.GetWeakPtr()));
-  } else {
+  if (!AwaitVSync()) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&Animator::BeginFrame, weak_factory_.GetWeakPtr(), 0));
@@ -87,9 +84,19 @@ void Animator::OnFrameComplete() {
 
   if (did_defer_frame_request_) {
     did_defer_frame_request_ = false;
-    BeginFrame(0);
+
+    if (!AwaitVSync())
+      BeginFrame(0);
   }
 }
 
+bool Animator::AwaitVSync() {
+  if (!vsync_provider_)
+    return false;
+  vsync_provider_->AwaitVSync(
+      base::Bind(&Animator::BeginFrame, weak_factory_.GetWeakPtr()));
+  return true;
+}
+
 }  // namespace shell
 }  // namespace sky

sky/shell/ui/animator.h

@@ -22,17 +22,18 @@ class Animator {
   void Start();
   void Stop();
 
-  void set_vsync_provider(vsync::VsyncProviderPtr vsync_provider) {
+  void set_vsync_provider(vsync::VSyncProviderPtr vsync_provider) {
     vsync_provider_ = vsync_provider.Pass();
   }
 
  private:
   void BeginFrame(int64_t time_stamp);
   void OnFrameComplete();
+  bool AwaitVSync();
 
   Engine::Config config_;
   Engine* engine_;
-  vsync::VsyncProviderPtr vsync_provider_;
+  vsync::VSyncProviderPtr vsync_provider_;
   int outstanding_requests_;
   bool did_defer_frame_request_;
   bool engine_requested_frame_;

sky/shell/ui/engine.cc

@@ -67,8 +67,8 @@ Engine::Engine(const Config& config)
   mojo::ConnectToService(service_provider.get(), &network_service_);
 
 #if defined(OS_ANDROID)
-  // TODO(abarth): Implement VsyncProvider on other platforms.
-  vsync::VsyncProviderPtr vsync_provider;
+  // TODO(abarth): Implement VSyncProvider on other platforms.
+  vsync::VSyncProviderPtr vsync_provider;
   mojo::ConnectToService(service_provider.get(), &vsync_provider);
   animator_->set_vsync_provider(vsync_provider.Pass());
 #endif