Encode frame number in pixel data for pose sync

third_party/WebKit/Source/modules/vr/VRDisplay.cpp

Index: third_party/WebKit/Source/modules/vr/VRDisplay.cpp
diff --git a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
index 8cab7bf6b5992028d69c81bdc05eb5494ef2e37f..9b7cf37834a63020273488d0700a791500d14fe6 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -8,6 +8,7 @@
 #include "core/dom/Fullscreen.h"
 #include "core/frame/UseCounter.h"
 #include "core/inspector/ConsoleMessage.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
 #include "modules/vr/NavigatorVR.h"
 #include "modules/vr/VRController.h"
 #include "modules/vr/VRDisplayCapabilities.h"
@@ -204,6 +205,9 @@ ScriptPromise VRDisplay::requestPresent(ScriptState* scriptState,
     return promise;
   }
 
+  // Save the GL context for use by submitFrame().
+  m_contextGL = toWebGLRenderingContextBase(renderingContext)->contextGL();
+
   if ((m_layer.leftBounds().size() != 0 && m_layer.leftBounds().size() != 4) ||
       (m_layer.rightBounds().size() != 0 &&
        m_layer.rightBounds().size() != 4)) {
@@ -343,6 +347,33 @@ HeapVector<VRLayer> VRDisplay::getLayers() {
 }
 
 void VRDisplay::submitFrame() {
+  // Write the frame number for the pose used into a bottom left pixel block.
+  // It is read by chrome/browser/android/vr_shell/vr_shell.cc to associate
+  // the correct corresponding pose for submission.
+  auto gl = m_contextGL;
+  int frame = m_framePose->poseNum;
+
+  // We must ensure that the WebGL app's GL state is preserved. We do this by
+  // calling low-level GL commands directly so that the rendering context's
+  // saved parameters don't get overwritten. When compositing, the rendering
+  // context restores the saved parameters in clearIfComposited().
+
+  gl->Enable(GL_SCISSOR_TEST);
+  // Use a few pixels to ensure we get a clean color. The resolution for the
+  // WebGL buffer may not match the final rendered destination size, and
+  // texture filtering could interfere for single pixels. This isn't visible
+  // since the final rendering hides the edges via a vignette effect.
+  gl->Scissor(0, 0, 4, 4);
+  gl->ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+  // Careful with the arithmetic here. Float color 1.f is equivalent to int 255.
+  gl->ClearColor((frame & 255) / 255.0f, ((frame >> 8) & 255) / 255.0f,
+                 ((frame >> 16) & 255) / 255.0f, 1.0f);
+  gl->Clear(GL_COLOR_BUFFER_BIT);
+
+  // TODO(klausw): If/when we end up bypassing the compositor, we'll likely
+  // need to call ctx->restoreStateAfterClear() or equivalent manually here
+  // since we'd be bypassing its clearIfComposited() method.

[bajones, 2016/10/04 06:33:57]

I'd prefer that we explicitly call restoreStateAfterClear() here. This currently
only works because of limitations of he fullscreen hack we're using that don't
allow any rendering to occur after submitFrame() without it showing up in the
HMD, but the spec requires that to work. (It's used for advanced mirroring on
desktops, not as useful on mobile.) Even if it happens to work without it for
now I'd prefer to double up on the reverts now in favor of accidentally missing
them in a future CL.

[klausw, 2016/10/04 06:47:04]

I can do that. restoreStateAfterClear() is a protected method though that will
need to be made available as public, is that ok? Seems a bit inelegant, would be
nice if there were a more generic method to track and restore GL state
client-side.

[klausw, 2016/10/04 17:52:47]

I've added a new public method to the rendering context to restore specific
state items based on a bitmask, and changed the private restoreStateAfterClear()
to use that. What do you think?

+
   controller()->submitFrame(m_displayId, m_framePose.Clone());
   m_canUpdateFramePose = true;
 }