Use GLX_SGI_video_sync to get a vblank signal.

ui/gl/gl_surface_glx.cc

Index: ui/gl/gl_surface_glx.cc
diff --git a/ui/gl/gl_surface_glx.cc b/ui/gl/gl_surface_glx.cc
index a164f3f56c1793cf84c36821d12874659f372c59..4f65a2ae1c3d19df801bced889b5c13950781167 100644
--- a/ui/gl/gl_surface_glx.cc
+++ b/ui/gl/gl_surface_glx.cc
@@ -12,8 +12,11 @@ extern "C" {
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/threading/thread.h"
 #include "base/time.h"
 #include "third_party/mesa/MesaLib/include/GL/osmesa.h"
 #include "ui/base/x/x11_util.h"
@@ -45,6 +48,274 @@ bool g_glx_oml_sync_control_supported = false;
 // always fails even though GLX_OML_sync_control is reported as being supported.
 bool g_glx_get_msc_rate_oml_supported = false;
 
+bool g_glx_sgi_video_sync_supported = false;
+
+class OMLSyncControlVSyncProvider
+    : public gfx::NativeViewGLSurfaceGLX::VSyncProvider {
+ public:
+  explicit OMLSyncControlVSyncProvider(gfx::AcceleratedWidget window)
+      : window_(window) {
+  }
+
+  virtual ~OMLSyncControlVSyncProvider() { }
+
+  virtual void GetVSyncParameters(
+      const GLSurface::UpdateVSyncCallback& callback) OVERRIDE {
+    base::TimeTicks timebase;
+    base::TimeDelta interval;
+
+    // The actual clock used for the system time returned by glXGetSyncValuesOML
+    // is unspecified. In practice, the clock used is likely to be either
+    // CLOCK_REALTIME or CLOCK_MONOTONIC, so we compare the returned time to the
+    // current time according to both clocks, and assume that the returned time
+    // was produced by the clock whose current time is closest to it, subject
+    // to the restriction that the returned time must not be in the future
+    // (since it is the time of a vblank that has already occurred).
+    int64 system_time;
+    int64 media_stream_counter;
+    int64 swap_buffer_counter;
+    if (!glXGetSyncValuesOML(g_display, window_, &system_time,
+                             &media_stream_counter, &swap_buffer_counter))
+      return;
+
+    struct timespec real_time;
+    struct timespec monotonic_time;
+    clock_gettime(CLOCK_REALTIME, &real_time);
+    clock_gettime(CLOCK_MONOTONIC, &monotonic_time);
+
+    int64 real_time_in_microseconds =
+        real_time.tv_sec * base::Time::kMicrosecondsPerSecond +
+        real_time.tv_nsec / base::Time::kNanosecondsPerMicrosecond;
+    int64 monotonic_time_in_microseconds =
+        monotonic_time.tv_sec * base::Time::kMicrosecondsPerSecond +
+        monotonic_time.tv_nsec / base::Time::kNanosecondsPerMicrosecond;
+
+    if ((system_time > real_time_in_microseconds) &&
+        (system_time > monotonic_time_in_microseconds))
+      return;
+
+    // We need the time according to CLOCK_MONOTONIC, so if we've been given
+    // a time from CLOCK_REALTIME, we need to convert.
+    bool time_conversion_needed =
+      (system_time > monotonic_time_in_microseconds) ||
+      (real_time_in_microseconds - system_time <
+       monotonic_time_in_microseconds - system_time);
+
+    if (time_conversion_needed) {
+      int64 time_difference =
+        real_time_in_microseconds - monotonic_time_in_microseconds;
+      timebase = base::TimeTicks::FromInternalValue(
+          system_time - time_difference);
+    } else {
+      timebase = base::TimeTicks::FromInternalValue(system_time);
+    }
+
+    // On platforms where glXGetMscRateOML doesn't work, we fall back to the
+    // assumption that we're displaying 60 frames per second.
+    const int64 kDefaultIntervalTime =
+        base::Time::kMicrosecondsPerSecond / 60;
+    int64 interval_time = kDefaultIntervalTime;
+    int32 numerator;
+    int32 denominator;
+    if (g_glx_get_msc_rate_oml_supported) {
+      if (glXGetMscRateOML(g_display, window_, &numerator, &denominator)) {
+        interval_time =
+            (base::Time::kMicrosecondsPerSecond * denominator) / numerator;
+      } else {
+        // Once glXGetMscRateOML has been found to fail, don't try again,
+        // since each failing call may spew an error message.
+        g_glx_get_msc_rate_oml_supported = false;
+      }
+    }
+    interval = base::TimeDelta::FromMicroseconds(interval_time);
+    callback.Run(timebase, interval);
+  }
+
+ private:
+  XID window_;
+
+  DISALLOW_COPY_AND_ASSIGN(OMLSyncControlVSyncProvider);
+};
+
+class SGIVideoSyncThread
+     : public base::Thread,
+       public base::NonThreadSafe,
+       public base::RefCounted<SGIVideoSyncThread> {
+ public:
+  static scoped_refptr<SGIVideoSyncThread> Create() {
+    if (!g_video_sync_thread) {
+      g_video_sync_thread = new SGIVideoSyncThread();
+      g_video_sync_thread->Start();
+    }
+    return g_video_sync_thread;
+  }
+
+  ~SGIVideoSyncThread() {
+    DCHECK(CalledOnValidThread());
+    g_video_sync_thread = NULL;
+    Stop();
+  }
+
+ private:
+  friend class base::RefCounted<SGIVideoSyncThread>;
+
+  SGIVideoSyncThread() : base::Thread("SGI_video_sync") {
+    DCHECK(CalledOnValidThread());
+  }
+
+  static SGIVideoSyncThread* g_video_sync_thread;
+
+  DISALLOW_COPY_AND_ASSIGN(SGIVideoSyncThread);
+};
+
+class SGIVideoSyncProviderThreadShim
+    : public base::SupportsWeakPtr<SGIVideoSyncProviderThreadShim> {
+ public:
+  explicit SGIVideoSyncProviderThreadShim(XID window)
+      : window_(window),
+        context_(NULL),
+        message_loop_(base::MessageLoopProxy::current()) {
+  }
+
+  void Initialize() {
+    DCHECK(SGIVideoSyncProviderThreadShim::g_display);
+
+    XWindowAttributes attributes;
+    if (!XGetWindowAttributes(SGIVideoSyncProviderThreadShim::g_display,
+                              window_, &attributes)) {

[piman, 2012/11/08 23:51:16]

It makes me somewhat uncomfortable to use the window from another display
without synchronization. Do we know that X has received the command to create
the window already?

[jonathan.backer, 2012/11/09 17:02:35]

FWIW, this is not much worse than the current situation.
- the browser process calls XCreateWindow
- GLSurfaceGLX::Initialize accesses this in the GPU process on it's own Display

I will defer the creation of VideoProvider until GLSurfaceGLX::Initialize. Then
this will be no worse.

As for your first question: all we know is that XCreateWindow has been called in
the browser. On Aura linux, we know that there has also been an XFlush. I don't
think that we know any more than that.

On GTK linux, I think that we don't even have a XFlush.

If you want, we can file a separate bug for this. The low road is to XSync after
XCreateWindow. The high road is to block (via
http://www.x.org/releases/X11R7.6/doc/xextproto/sync.html ?) in the GPU process.

[piman, 2012/11/09 18:08:05]

On Aura we have a round trip to X right after we create the window, (see
XGetWindowAttributes in RootWindowHostLinux::RootWindowHostLinux)
I was almost positive we had a XSync on this path (in the gtk_preserve_window
code), but I can't find it - you may be right (and maybe we've been lucky).

I guess I'm thinking in terms of layers, this is in ui/ that doesn't know
anything about content/ and whatever synchronization we may do there. From the
GLSurfaceGLX client's point of view, it would be legal to create a window,
create the GLSurfaceGLX around it and call GetVSyncParameters - and it'd work in
the GLX_OML_sync_control case. Maybe for now we can just call XSync in the
SGIVideoSyncVSyncProvider constructor, and optimize later with X-SYNCHRONIZATION
if it proves to be too slow.

+      LOG(ERROR) << "XGetWindowAttributes failed for window " <<
+        window_ << ".";
+      return;
+    }
+
+    XVisualInfo visual_info_template;
+    visual_info_template.visualid = XVisualIDFromVisual(attributes.visual);
+
+    int visual_info_count = 0;
+    scoped_ptr_malloc<XVisualInfo, ScopedPtrXFree> visual_info_list(
+        XGetVisualInfo(SGIVideoSyncProviderThreadShim::g_display, VisualIDMask,
+                       &visual_info_template, &visual_info_count));
+
+    DCHECK(visual_info_list.get());
+    if (visual_info_count == 0) {
+      LOG(ERROR) << "No visual info for visual ID.";
+      return;
+    }
+
+    context_ = glXCreateContext(SGIVideoSyncProviderThreadShim::g_display,
+                                visual_info_list.get(),
+                                NULL,
+                                True);
+
+    DCHECK(NULL != context_);
+  }
+
+  void Destroy() {
+    if (context_) {
+      glXDestroyContext(SGIVideoSyncProviderThreadShim::g_display, context_);
+      context_ = NULL;
+    }
+    delete this;
+  }
+
+  void GetVSyncParameters(const GLSurface::UpdateVSyncCallback& callback) {
+    if (!context_)
+      return;
+
+    glXMakeCurrent(SGIVideoSyncProviderThreadShim::g_display,
+                   window_, context_);

[piman, 2012/11/08 23:51:16]

How do we know that the window hasn't been destroyed yet?

Tab closes -> GLSurface gets destroyed -> signals the browser it can destroy the
window.
If there is a pending GetVSyncParameters callback, isn't it possible that this
runs after the window is destroyed?

[jonathan.backer, 2012/11/09 17:02:35]

Good point. I've added a cancellation flag and a lock. This will guarantee that
this section won't get hit once the SGIVideoSyncVSyncProvider is destroyed.

+
+    unsigned int retrace_count = 0;
+    if (glXWaitVideoSyncSGI(1, 0, &retrace_count) != 0)
+      return;
+
+    TRACE_EVENT_INSTANT0("gpu", "vblank");
+    base::TimeTicks now = base::TimeTicks::HighResNow();
+
+    const int64 kDefaultIntervalTime =
+        base::Time::kMicrosecondsPerSecond / 60;
+    base::TimeDelta interval =
+        base::TimeDelta::FromMicroseconds(kDefaultIntervalTime);
+
+    message_loop_->PostTask(FROM_HERE, base::Bind(callback, now, interval));
+  }
+
+ private:
+  // For initialization of g_display in GLSurface::InitializeOneOff before
+  // the sandbox goes up.
+  friend class gfx::GLSurfaceGLX;
+
+  virtual ~SGIVideoSyncProviderThreadShim() {
+  }
+
+  static Display* g_display;
+
+  XID window_;
+  GLXContext context_;
+
+  scoped_refptr<base::MessageLoopProxy> message_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(SGIVideoSyncProviderThreadShim);
+};
+
+class SGIVideoSyncVSyncProvider
+    : public gfx::NativeViewGLSurfaceGLX::VSyncProvider,
+      public base::SupportsWeakPtr<SGIVideoSyncVSyncProvider> {
+ public:
+  explicit SGIVideoSyncVSyncProvider(gfx::AcceleratedWidget window)
+      : vsync_thread_(SGIVideoSyncThread::Create()),
+        shim_((new SGIVideoSyncProviderThreadShim(window))->AsWeakPtr()) {
+    // The WeakPtr is bound to the SGIVideoSyncThread. We only use it for
+    // PostTask.
+    shim_->DetachFromThread();
+    vsync_thread_->message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&SGIVideoSyncProviderThreadShim::Initialize, shim_));
+  }
+
+  virtual ~SGIVideoSyncVSyncProvider() {
+    vsync_thread_->message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&SGIVideoSyncProviderThreadShim::Destroy, shim_));
+  }
+
+  virtual void GetVSyncParameters(
+      const GLSurface::UpdateVSyncCallback& callback) OVERRIDE {
+    // Only one outstanding request per surface.
+    if (!pending_callback_) {
+      pending_callback_.reset(new GLSurface::UpdateVSyncCallback(callback));
+      vsync_thread_->message_loop()->PostTask(
+          FROM_HERE,
+          base::Bind(&SGIVideoSyncProviderThreadShim::GetVSyncParameters,
+                     shim_, base::Bind(
+                         &SGIVideoSyncVSyncProvider::PendingCallbackRunner,
+                         AsWeakPtr())));
+    }
+  }
+
+ private:
+  void PendingCallbackRunner(const base::TimeTicks timebase,
+                             const base::TimeDelta interval) {
+    DCHECK(pending_callback_);
+    pending_callback_->Run(timebase, interval);
+    pending_callback_.reset();
+  }
+
+  scoped_refptr<SGIVideoSyncThread> vsync_thread_;
+  base::WeakPtr<SGIVideoSyncProviderThreadShim> shim_;
+
+  scoped_ptr<GLSurface::UpdateVSyncCallback> pending_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(SGIVideoSyncVSyncProvider);
+};
+
+SGIVideoSyncThread* SGIVideoSyncThread::g_video_sync_thread = NULL;
+
+// In order to take advantage of GLX_SGI_video_sync, we need a display
+// for use on a separate thread. We must allocate this before the sandbox
+// goes up (rather than on-demand when we start the thread).
+Display* SGIVideoSyncProviderThreadShim::g_display = NULL;
+
 }  // namespace
 
 GLSurfaceGLX::GLSurfaceGLX() {}
@@ -79,7 +350,11 @@ bool GLSurfaceGLX::InitializeOneOff() {
   g_glx_oml_sync_control_supported =
       HasGLXExtension("GLX_OML_sync_control");
   g_glx_get_msc_rate_oml_supported = g_glx_oml_sync_control_supported;
+  g_glx_sgi_video_sync_supported =
+      HasGLXExtension("GLX_SGI_video_sync");
 
+  if (!g_glx_get_msc_rate_oml_supported && g_glx_sgi_video_sync_supported)
+    SGIVideoSyncProviderThreadShim::g_display = XOpenDisplay(NULL);
 
   initialized = true;
   return true;
@@ -119,6 +394,10 @@ GLSurfaceGLX::~GLSurfaceGLX() {}
 NativeViewGLSurfaceGLX::NativeViewGLSurfaceGLX(gfx::AcceleratedWidget window)
   : window_(window),
     config_(NULL) {
+  if (g_glx_oml_sync_control_supported)
+    vsync_provider_.reset(new OMLSyncControlVSyncProvider(window_));
+  else if (g_glx_sgi_video_sync_supported)
+    vsync_provider_.reset(new SGIVideoSyncVSyncProvider(window_));
 }
 
 bool NativeViewGLSurfaceGLX::Initialize() {
@@ -240,77 +519,10 @@ bool NativeViewGLSurfaceGLX::PostSubBuffer(
   return true;
 }
 
-bool NativeViewGLSurfaceGLX::GetVSyncParameters(base::TimeTicks* timebase,
-                                                base::TimeDelta* interval) {
-  if (!g_glx_oml_sync_control_supported)
-    return false;
-
-  // The actual clock used for the system time returned by glXGetSyncValuesOML
-  // is unspecified. In practice, the clock used is likely to be either
-  // CLOCK_REALTIME or CLOCK_MONOTONIC, so we compare the returned time to the
-  // current time according to both clocks, and assume that the returned time
-  // was produced by the clock whose current time is closest to it, subject
-  // to the restriction that the returned time must not be in the future (since
-  // it is the time of a vblank that has already occurred).
-  int64 system_time;
-  int64 media_stream_counter;
-  int64 swap_buffer_counter;
-  if (!glXGetSyncValuesOML(g_display, window_, &system_time,
-                           &media_stream_counter, &swap_buffer_counter))
-    return false;
-
-  struct timespec real_time;
-  struct timespec monotonic_time;
-  clock_gettime(CLOCK_REALTIME, &real_time);
-  clock_gettime(CLOCK_MONOTONIC, &monotonic_time);
-
-  int64 real_time_in_microseconds =
-      real_time.tv_sec * base::Time::kMicrosecondsPerSecond +
-      real_time.tv_nsec / base::Time::kNanosecondsPerMicrosecond;
-  int64 monotonic_time_in_microseconds =
-      monotonic_time.tv_sec * base::Time::kMicrosecondsPerSecond +
-      monotonic_time.tv_nsec / base::Time::kNanosecondsPerMicrosecond;
-
-  if ((system_time > real_time_in_microseconds) &&
-      (system_time > monotonic_time_in_microseconds))
-    return false;
-
-  // We need the time according to CLOCK_MONOTONIC, so if we've been given
-  // a time from CLOCK_REALTIME, we need to convert.
-  bool time_conversion_needed =
-    (system_time > monotonic_time_in_microseconds) ||
-    (real_time_in_microseconds - system_time <
-     monotonic_time_in_microseconds - system_time);
-
-  if (time_conversion_needed) {
-    int64 time_difference =
-        real_time_in_microseconds - monotonic_time_in_microseconds;
-    *timebase = base::TimeTicks::FromInternalValue(
-        system_time - time_difference);
-  } else {
-    *timebase = base::TimeTicks::FromInternalValue(system_time);
-  }
-
-  // On platforms where glXGetMscRateOML doesn't work, we fall back to the
-  // assumption that we're displaying 60 frames per second.
-  const int64 kDefaultIntervalTime =
-      base::Time::kMicrosecondsPerSecond / 60;
-  int64 interval_time = kDefaultIntervalTime;
-  int32 numerator;
-  int32 denominator;
-  if (g_glx_get_msc_rate_oml_supported) {
-    if (glXGetMscRateOML(g_display, window_, &numerator, &denominator)) {
-      interval_time =
-          (base::Time::kMicrosecondsPerSecond * denominator) / numerator;
-    } else {
-      // Once glXGetMscRateOML has been found to fail, don't try again,
-      // since each failing call may spew an error message.
-      g_glx_get_msc_rate_oml_supported = false;
-    }
-  }
-
-  *interval = base::TimeDelta::FromMicroseconds(interval_time);
-  return true;
+void NativeViewGLSurfaceGLX::GetVSyncParameters(
+    const UpdateVSyncCallback& callback) {
+  if (vsync_provider_)
+    vsync_provider_->GetVSyncParameters(callback);
 }
 
 NativeViewGLSurfaceGLX::NativeViewGLSurfaceGLX()