Re-land^2 WebVR compositor bypass via BrowserMain context + mailbox

chrome/browser/android/vr_shell/vr_shell_gl.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 "chrome/browser/android/vr_shell/vr_shell_gl.h"
 
 #include <limits>
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/callback_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/android/vr_shell/mailbox_to_surface_bridge.h"
 #include "chrome/browser/android/vr_shell/ui_elements.h"
 #include "chrome/browser/android/vr_shell/ui_scene.h"
 #include "chrome/browser/android/vr_shell/vr_controller.h"
 #include "chrome/browser/android/vr_shell/vr_gl_util.h"
 #include "chrome/browser/android/vr_shell/vr_math.h"
 #include "chrome/browser/android/vr_shell/vr_shell.h"
 #include "chrome/browser/android/vr_shell/vr_shell_delegate.h"
 #include "chrome/browser/android/vr_shell/vr_shell_renderer.h"
 #include "device/vr/android/gvr/gvr_device.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
@@ skipping 48 matching lines @@
 
 // The GVR viewport list has two entries (left eye and right eye) for each
 // GVR buffer.
 static constexpr int kViewportListPrimaryOffset = 0;
 static constexpr int kViewportListHeadlockedOffset = 2;
 
 // Buffer size large enough to handle the current backlog of poses which is
 // 2-3 frames.
 static constexpr unsigned kPoseRingBufferSize = 8;
 
-// Magic numbers used to mark valid pose index values encoded in frame
-// data. Must match the magic numbers used in blink's VRDisplay.cpp.
-static constexpr std::array<uint8_t, 2> kWebVrPosePixelMagicNumbers{{42, 142}};
+// Default downscale factor for computing the recommended WebVR
+// renderWidth/Height from the 1:1 pixel mapped size. Using a rather
+// aggressive downscale due to the high overhead of copying pixels
+// twice before handing off to GVR. For comparison, the polyfill
+// uses approximately 0.55 on a Pixel XL.
+static constexpr float kWebVrRecommendedResolutionScale = 0.5;
 
 float Distance(const gvr::Vec3f& vec1, const gvr::Vec3f& vec2) {
   float xdiff = (vec1.x - vec2.x);
   float ydiff = (vec1.y - vec2.y);
   float zdiff = (vec1.z - vec2.z);
   float scale = xdiff * xdiff + ydiff * ydiff + zdiff * zdiff;
   return std::sqrt(scale);
 }
 
 // Generate a quaternion representing the rotation from the negative Z axis
@@ skipping 69 matching lines @@
       binding_(this),
       weak_vr_shell_(weak_vr_shell),
       delegate_provider_(delegate_provider),
       main_thread_task_runner_(std::move(main_thread_task_runner)),
       weak_ptr_factory_(this) {
   GvrInit(gvr_api);
 }
 
 VrShellGl::~VrShellGl() {
   vsync_task_.Cancel();
+  binding_.Close();

[dcheng, 2017/03/08 07:23:39]

Please add a comment why it's necessary to explicitly close.

[klausw, 2017/03/08 08:11:09]

Does the comment below the next line make it clearer? The VRDisplay side of the
connection outlives VrShellGl due to how it switches between the presenting
delegate (which is VrShellGl) and the non-presenting delegate (for "magic
window" non-headset mode) on entering/exiting presentation.

[mthiesse, 2017/03/08 13:44:48]

I wrote this - my concern was that it might be possible, if both ends of the
binding ever live in the same process for some reason, that we could receive
another VSync request in response to the closing message while in the
destructor, then fail to respond to the new callback. If this isn't a reasonable
concern, then we don't need to explicitly close the binding.

[klausw, 2017/03/08 16:58:51]

Added as a comment. Just to clarify, mthiesse@ contributed many fixes and
enhancements as noted in the ps history, I wasn't meaning to imply otherwise.
Thank you again for your help with mysterious mojo magic!

[dcheng, 2017/03/08 23:08:05]

OK, thanks. In the case of non-sync calls (which is the default), there
shouldn't be anything to worry about, so I would say this can probably be
removed. But investigating this in a followup is fine.

   if (!callback_.is_null()) {
     // When this VSync provider is going away we have to respond to pending
     // callbacks, so instead of providing a VSync, tell the requester to try
     // again. A VSyncProvider is guaranteed to exist, so the request in response
     // to this message will go through some other VSyncProvider.
     base::ResetAndReturn(&callback_)
         .Run(nullptr, base::TimeDelta(), -1,
-             device::mojom::VRVSyncProvider::Status::RETRY);
-  }
-  if (binding_.is_bound()) {
-    main_thread_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&VrShellDelegate::OnVRVsyncProviderRequest,
-                   delegate_provider_, base::Passed(binding_.Unbind())));
+             device::mojom::VRVSyncProvider::Status::CLOSING);
   }
 }
 
 void VrShellGl::Initialize() {
   scene_.reset(new UiScene);
 
   if (surfaceless_rendering_) {
     // If we're rendering surfaceless, we'll never get a java surface to render
     // into, so we can initialize GL right away.
     InitializeGl(nullptr);
@@ skipping 26 matching lines @@
     LOG(ERROR) << "gl::init::CreateGLContext failed";
     ForceExitVr();
     return;
   }
   if (!context_->MakeCurrent(surface_.get())) {
     LOG(ERROR) << "gl::GLContext::MakeCurrent() failed";
     ForceExitVr();
     return;
   }
 
-  unsigned int textures[2];
-  glGenTextures(2, textures);
+  unsigned int textures[3];
+  glGenTextures(3, textures);
   ui_texture_id_ = textures[0];
   content_texture_id_ = textures[1];
+  webvr_texture_id_ = textures[2];
   ui_surface_texture_ = gl::SurfaceTexture::Create(ui_texture_id_);
   content_surface_texture_ = gl::SurfaceTexture::Create(content_texture_id_);
+  webvr_surface_texture_ = gl::SurfaceTexture::Create(webvr_texture_id_);
   CreateUiSurface();
   CreateContentSurface();
   ui_surface_texture_->SetFrameAvailableCallback(base::Bind(
       &VrShellGl::OnUIFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
   content_surface_texture_->SetFrameAvailableCallback(base::Bind(
       &VrShellGl::OnContentFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
+  webvr_surface_texture_->SetFrameAvailableCallback(base::Bind(
+      &VrShellGl::OnWebVRFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
+  ui_surface_texture_->SetDefaultBufferSize(ui_tex_physical_size_.width,
+                                            ui_tex_physical_size_.height);
   content_surface_texture_->SetDefaultBufferSize(
       content_tex_physical_size_.width, content_tex_physical_size_.height);
-  ui_surface_texture_->SetDefaultBufferSize(ui_tex_physical_size_.width,
-                                            ui_tex_physical_size_.height);
   InitializeRenderer();
 
+  // Pick a reasonable default size for the WebVR transfer surface
+  // based on a downscaled 1:1 render resolution. This size will also
+  // be reported to the client via CreateVRDisplayInfo as the
+  // client-recommended renderWidth/renderHeight and for the GVR
+  // framebuffer. If the client chooses a different size or resizes it
+  // while presenting, we'll resize the transfer surface and GVR
+  // framebuffer to match.
+  auto render_target_size = gvr_api_->GetMaximumEffectiveRenderTargetSize();

[dcheng, 2017/03/08 07:23:39]

Can this be explicitly typed?

[klausw, 2017/03/08 08:11:09]

Done.

+  gvr::Sizei webvr_size = {static_cast<int>(render_target_size.width *
+                                            kWebVrRecommendedResolutionScale),
+                           static_cast<int>(render_target_size.height *
+                                            kWebVrRecommendedResolutionScale)};
+
+  // TODO(klausw,crbug.com/699350): should we round the recommended
+  // size to a multiple of 2^N pixels to be friendlier to the GPU? The
+  // exact size doesn't matter.
+
+  CreateOrResizeWebVRSurface(webvr_size);
+
   vsync_task_.Reset(base::Bind(&VrShellGl::OnVSync, base::Unretained(this)));
   OnVSync();
 
   ready_to_draw_ = true;
 }
 
 void VrShellGl::CreateContentSurface() {
   content_surface_ =
       base::MakeUnique<gl::ScopedJavaSurface>(content_surface_texture_.get());
   main_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&VrShell::ContentSurfaceChanged, weak_vr_shell_,
                             content_surface_->j_surface().obj()));
 }
 
 void VrShellGl::CreateUiSurface() {
   ui_surface_ =
       base::MakeUnique<gl::ScopedJavaSurface>(ui_surface_texture_.get());
   main_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&VrShell::UiSurfaceChanged, weak_vr_shell_,
                             ui_surface_->j_surface().obj()));
 }
 
+void VrShellGl::CreateOrResizeWebVRSurface(const gvr::Sizei& size) {
+  if (!webvr_surface_texture_) {
+    DLOG(ERROR) << "No WebVR surface texture available";
+    return;
+  }
+
+  // ContentPhysicalBoundsChanged is getting called twice with
+  // identical sizes? Avoid thrashing the existing context.
+  if (size == webvr_surface_size_) {
+    return;
+  }
+
+  if (!size.width || !size.height) {
+    // Invalid size, defer until a new size arrives on a future bounds update.
+    return;
+  }
+
+  webvr_surface_texture_->SetDefaultBufferSize(size.width, size.height);
+  webvr_surface_size_ = size;
+
+  if (mailbox_bridge_) {
+    mailbox_bridge_->ResizeSurface(size.width, size.height);
+  } else {
+    mailbox_bridge_ = base::MakeUnique<MailboxToSurfaceBridge>();
+    mailbox_bridge_->CreateSurface(webvr_surface_texture_);
+  }
+}
+
+void VrShellGl::SubmitWebVRFrame(int16_t frame_index,
+                                 const gpu::MailboxHolder& mailbox) {
+  TRACE_EVENT0("gpu", "VrShellGl::SubmitWebVRFrame");
+
+  // Swapping twice on a Surface without calling updateTexImage in
+  // between can lose frames, so don't draw+swap if we already have
+  // a pending frame we haven't consumed yet.
+  bool swapped = false;
+  if (pending_frames_.empty()) {
+    swapped = mailbox_bridge_->CopyMailboxToSurfaceAndSwap(mailbox);
+    if (swapped) {
+      // Tell OnWebVRFrameAvailable to expect a new frame to arrive on
+      // the SurfaceTexture, and save the associated frame index.
+      pending_frames_.emplace(frame_index);
+    }
+  }
+  // Always notify the client that we're done with the mailbox even
+  // if we haven't drawn it, so that it's eligible for destruction.
+  submit_client_->OnSubmitFrameTransferred();
+  if (!swapped) {
+    // We dropped without drawing, report this as completed rendering
+    // now to unblock the client. We're not going to receive it in
+    // OnWebVRFrameAvailable where we'd normally report that.
+    submit_client_->OnSubmitFrameRendered();
+  }
+
+  TRACE_EVENT0("gpu", "VrShellGl::glFinish");
+  // This is a load-bearing glFinish, please don't remove it without
+  // before/after timing comparisons. Goal is to clear the GPU queue
+  // of the native GL context to avoid stalls later in GVR frame
+  // acquire/submit.
+  glFinish();
+}
+
+void VrShellGl::SetSubmitClient(
+    device::mojom::VRSubmitFrameClientPtrInfo submit_client_info) {
+  submit_client_.Bind(std::move(submit_client_info));
+}
+
 void VrShellGl::OnUIFrameAvailable() {
   ui_surface_texture_->UpdateTexImage();
 }
 
 void VrShellGl::OnContentFrameAvailable() {
   content_surface_texture_->UpdateTexImage();
   received_frame_ = true;
 }
 
-bool VrShellGl::GetPixelEncodedFrameIndex(uint16_t* frame_index) {
-  TRACE_EVENT0("gpu", "VrShellGl::GetPixelEncodedFrameIndex");
-  if (!received_frame_) {
-    if (last_frame_index_ == (uint16_t)-1)
-      return false;
-    *frame_index = last_frame_index_;
-    return true;
+void VrShellGl::OnWebVRFrameAvailable() {
+  // A "while" loop here is a bad idea. It's legal to call
+  // UpdateTexImage repeatedly even if no frames are available, but
+  // that does *not* wait for a new frame, it just reuses the most
+  // recent one. That would mess up the count.
+  if (pending_frames_.empty()) {
+    // We're expecting a frame, but it's not here yet. Retry in OnVsync.
+    ++premature_received_frames_;
+    return;
   }
-  received_frame_ = false;
 
-  // Read the pose index encoded in a bottom left pixel as color values.
-  // See also third_party/WebKit/Source/modules/vr/VRDisplay.cpp which
-  // encodes the pose index, and device/vr/android/gvr/gvr_device.cc
-  // which tracks poses. Returns the low byte (0..255) if valid, or -1
-  // if not valid due to bad magic number.
-  uint8_t pixels[4];
-  // Assume we're reading from the framebuffer we just wrote to.
-  // That's true currently, we may need to use glReadBuffer(GL_BACK)
-  // or equivalent if the rendering setup changes in the future.
-  glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+  webvr_surface_texture_->UpdateTexImage();
+  int frame_index = pending_frames_.front();
+  TRACE_EVENT1("gpu", "VrShellGl::OnWebVRFrameAvailable", "frame", frame_index);
+  pending_frames_.pop();
 
-  // Check for the magic number written by VRDevice.cpp on submit.
-  // This helps avoid glitches from garbage data in the render
-  // buffer that can appear during initialization or resizing. These
-  // often appear as flashes of all-black or all-white pixels.
-  if (pixels[1] == kWebVrPosePixelMagicNumbers[0] &&
-      pixels[2] == kWebVrPosePixelMagicNumbers[1]) {
-    // Pose is good.
-    *frame_index = pixels[0];
-    last_frame_index_ = pixels[0];
-    return true;
-  }
-  VLOG(1) << "WebVR: reject decoded pose index " << static_cast<int>(pixels[0])
-          << ", bad magic number " << static_cast<int>(pixels[1]) << ", "
-          << static_cast<int>(pixels[2]);
-  return false;
+  // It is legal for the WebVR client to submit a new frame now, since
+  // we've consumed the image. TODO(klausw): would timing be better if
+  // we move the "rendered" notification after draw, or suppress
+  // the next vsync until that's done?
+
+  submit_client_->OnSubmitFrameRendered();
+
+  DrawFrame(frame_index);
 }
 
 void VrShellGl::GvrInit(gvr_context* gvr_api) {
   gvr_api_ = gvr::GvrApi::WrapNonOwned(gvr_api);
   controller_.reset(new VrController(gvr_api));
 
   ViewerType viewerType;
   switch (gvr_api_->GetViewerType()) {
     case gvr::ViewerType::GVR_VIEWER_TYPE_DAYDREAM:
       viewerType = ViewerType::DAYDREAM;
       break;
     case gvr::ViewerType::GVR_VIEWER_TYPE_CARDBOARD:
       viewerType = ViewerType::CARDBOARD;
       break;
     default:
       NOTREACHED();
       viewerType = ViewerType::UNKNOWN_TYPE;
       break;
   }
   UMA_HISTOGRAM_ENUMERATION("VRViewerType", static_cast<int>(viewerType),
                             static_cast<int>(ViewerType::VIEWER_TYPE_MAX));
 }
 
 void VrShellGl::InitializeRenderer() {
-  // While WebVR is going through the compositor path, it shares
-  // the same texture ID. This will change once it gets its own
-  // surface, but store it separately to avoid future confusion.
-  // TODO(klausw,crbug.com/655722): remove this.
-  webvr_texture_id_ = content_texture_id_;
-
   gvr_api_->InitializeGl();
   webvr_head_pose_.assign(kPoseRingBufferSize,
                           gvr_api_->GetHeadSpaceFromStartSpaceRotation(
                               gvr::GvrApi::GetTimePointNow()));
 
   std::vector<gvr::BufferSpec> specs;
   // For kFramePrimaryBuffer (primary VrShell and WebVR content)
   specs.push_back(gvr_api_->CreateBufferSpec());
   render_size_primary_ = specs[kFramePrimaryBuffer].GetSize();
+  render_size_vrshell_ = render_size_primary_;
 
   // For kFrameHeadlockedBuffer (for WebVR insecure content warning).
   // Set this up at fixed resolution, the (smaller) FOV gets set below.
   specs.push_back(gvr_api_->CreateBufferSpec());
   specs.back().SetSize(kHeadlockedBufferDimensions);
   render_size_headlocked_ = specs[kFrameHeadlockedBuffer].GetSize();
 
   swap_chain_.reset(new gvr::SwapChain(gvr_api_->CreateSwapChain(specs)));
 
   vr_shell_renderer_.reset(new VrShellRenderer());
@@ skipping 256 matching lines @@
                             std::unique_ptr<blink::WebInputEvent> event) {
   DCHECK(input_target != InputTarget::NONE);
   auto&& target = input_target == InputTarget::CONTENT
                       ? &VrShell::ProcessContentGesture
                       : &VrShell::ProcessUIGesture;
   main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(target, weak_vr_shell_, base::Passed(std::move(event))));
 }
 
-void VrShellGl::DrawFrame() {
-  TRACE_EVENT0("gpu", "VrShellGl::DrawFrame");
+void VrShellGl::DrawFrame(int frame_index) {

[dcheng, 2017/03/08 07:23:39]

int16_t to match the other frame_index type?

[klausw, 2017/03/08 08:11:09]

Done.

+  TRACE_EVENT1("gpu", "VrShellGl::DrawFrame", "frame", frame_index);
 
   // Reset the viewport list to just the pair of viewports for the
   // primary buffer each frame. Head-locked viewports get added by
   // DrawVrShell if needed.
   buffer_viewport_list_->SetToRecommendedBufferViewports();
 
+  // If needed, resize the primary buffer for use with WebVR.
+  if (web_vr_mode_) {
+    if (render_size_primary_ != webvr_surface_size_) {
+      if (!webvr_surface_size_.width) {
+        return;
+      }
+      render_size_primary_ = webvr_surface_size_;
+      swap_chain_->ResizeBuffer(kFramePrimaryBuffer, render_size_primary_);
+    }
+  } else {
+    if (render_size_primary_ != render_size_vrshell_) {
+      render_size_primary_ = render_size_vrshell_;
+      swap_chain_->ResizeBuffer(kFramePrimaryBuffer, render_size_primary_);
+    }
+  }
+
+  TRACE_EVENT_BEGIN0("gpu", "VrShellGl::AcquireFrame");
   gvr::Frame frame = swap_chain_->AcquireFrame();
+  TRACE_EVENT_END0("gpu", "VrShellGl::AcquireFrame");
   if (!frame.is_valid()) {
     return;
   }
   frame.BindBuffer(kFramePrimaryBuffer);
   if (web_vr_mode_) {
     DrawWebVr();
   }
 
-  uint16_t frame_index;
   gvr::Mat4f head_pose;
 
   // When using async reprojection, we need to know which pose was used in
-  // the WebVR app for drawing this frame. Due to unknown amounts of
-  // buffering in the compositor and SurfaceTexture, we read the pose number
-  // from a corner pixel. There's no point in doing this for legacy
-  // distortion rendering since that doesn't need a pose, and reading back
-  // pixels is an expensive operation. TODO(klausw,crbug.com/655722): stop
-  // doing this once we have working no-compositor rendering for WebVR.
-  if (web_vr_mode_ && gvr_api_->GetAsyncReprojectionEnabled() &&
-      GetPixelEncodedFrameIndex(&frame_index)) {
+  // the WebVR app for drawing this frame. Only needed if reprojection is
+  // in use.
+  if (web_vr_mode_ && gvr_api_->GetAsyncReprojectionEnabled()) {
     static_assert(!((kPoseRingBufferSize - 1) & kPoseRingBufferSize),
                   "kPoseRingBufferSize must be a power of 2");
     head_pose = webvr_head_pose_[frame_index % kPoseRingBufferSize];
     // Process all pending_bounds_ changes targeted for before this frame, being
     // careful of wrapping frame indices.
     static constexpr unsigned max =
         std::numeric_limits<decltype(frame_index_)>::max();
     static_assert(max > kPoseRingBufferSize * 2,
                   "To detect wrapping, kPoseRingBufferSize must be smaller "
                   "than half of frame_index_ range.");
     while (!pending_bounds_.empty()) {
       uint16_t index = pending_bounds_.front().first;
       // If index is less than the frame_index it's possible we've wrapped, so
       // we extend the range and 'un-wrap' to account for this.
       if (index < frame_index)
         index += max;
       // If the pending bounds change is for an upcoming frame within our buffer
       // size, wait to apply it. Otherwise, apply it immediately. This
       // guarantees that even if we miss many frames, the queue can't fill up
       // with stale bounds.
       if (index > frame_index && index <= frame_index + kPoseRingBufferSize)
         break;
 
-      const BoundsPair& bounds = pending_bounds_.front().second;
-      webvr_left_viewport_->SetSourceUv(bounds.first);
-      webvr_right_viewport_->SetSourceUv(bounds.second);
+      const WebVrBounds& bounds = pending_bounds_.front().second;
+      webvr_left_viewport_->SetSourceUv(bounds.left_bounds);
+      webvr_right_viewport_->SetSourceUv(bounds.right_bounds);
+      CreateOrResizeWebVRSurface(bounds.source_size);
       pending_bounds_.pop();
     }
     buffer_viewport_list_->SetBufferViewport(GVR_LEFT_EYE,
                                              *webvr_left_viewport_);
     buffer_viewport_list_->SetBufferViewport(GVR_RIGHT_EYE,
                                              *webvr_right_viewport_);
   } else {
     gvr::ClockTimePoint target_time = gvr::GvrApi::GetTimePointNow();
     target_time.monotonic_system_time_nanos += kPredictionTimeWithoutVsyncNanos;
     head_pose = gvr_api_->GetHeadSpaceFromStartSpaceRotation(target_time);
   }
 
   gvr::Vec3f position = GetTranslation(head_pose);
   if (position.x == 0.0f && position.y == 0.0f && position.z == 0.0f) {
     // This appears to be a 3DOF pose without a neck model. Add one.
     // The head pose has redundant data. Assume we're only using the
     // object_from_reference_matrix, we're not updating position_external.
     // TODO: Not sure what object_from_reference_matrix is. The new api removed
     // it. For now, removing it seems working fine.
     gvr_api_->ApplyNeckModel(head_pose, 1.0f);
   }
 
   // Update the render position of all UI elements (including desktop).
   scene_->UpdateTransforms(TimeInMicroseconds());
 
-  UpdateController(GetForwardVector(head_pose));
+  {
+    TRACE_EVENT0("gpu", "VrShellGl::UpdateController");
+    UpdateController(GetForwardVector(head_pose));
+  }
 
-  DrawVrShell(head_pose, frame);
+  // Finish drawing in the primary buffer, and draw the headlocked buffer
+  // if needed. This must be the last drawing call, this method will
+  // return with no frame being bound.
+  DrawVrShellAndUnbind(head_pose, frame);
 
-  frame.Unbind();
-  frame.Submit(*buffer_viewport_list_, head_pose);
+  {
+    TRACE_EVENT0("gpu", "VrShellGl::Submit");
+    frame.Submit(*buffer_viewport_list_, head_pose);
+  }
 
   // No need to swap buffers for surfaceless rendering.
   if (!surfaceless_rendering_) {
     // TODO(mthiesse): Support asynchronous SwapBuffers.
+    TRACE_EVENT0("gpu", "VrShellGl::SwapBuffers");
     surface_->SwapBuffers();
   }
 }
 
-void VrShellGl::DrawVrShell(const gvr::Mat4f& head_pose, gvr::Frame& frame) {
+void VrShellGl::DrawVrShellAndUnbind(const gvr::Mat4f& head_pose,
+                                     gvr::Frame& frame) {
   TRACE_EVENT0("gpu", "VrShellGl::DrawVrShell");
   std::vector<const ContentRectangle*> head_locked_elements;
   std::vector<const ContentRectangle*> world_elements;
   for (const auto& rect : scene_->GetUiElements()) {
     if (!rect->IsVisible())
       continue;
     if (rect->lock_to_fov) {
       head_locked_elements.push_back(rect.get());
     } else {
       world_elements.push_back(rect.get());
@@ skipping 16 matching lines @@
 
     const Colorf& backgroundColor = scene_->GetBackgroundColor();
     glClearColor(backgroundColor.r, backgroundColor.g, backgroundColor.b,
                  backgroundColor.a);
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
   }
   if (!world_elements.empty()) {
     DrawUiView(&head_pose, world_elements, render_size_primary_,
                kViewportListPrimaryOffset);
   }
+  frame.Unbind();  // Done with the primary buffer.
 
   if (!head_locked_elements.empty()) {
     // Add head-locked viewports. The list gets reset to just
     // the recommended viewports (for the primary buffer) each frame.
     buffer_viewport_list_->SetBufferViewport(
         kViewportListHeadlockedOffset + GVR_LEFT_EYE,
         *headlocked_left_viewport_);
     buffer_viewport_list_->SetBufferViewport(
         kViewportListHeadlockedOffset + GVR_RIGHT_EYE,
         *headlocked_right_viewport_);
 
     // Bind the headlocked framebuffer.
-    // TODO(mthiesse): We don't unbind this? Maybe some cleanup is in order
-    // here.
     frame.BindBuffer(kFrameHeadlockedBuffer);
     glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
     DrawUiView(nullptr, head_locked_elements, render_size_headlocked_,
                kViewportListHeadlockedOffset);
+    frame.Unbind();  // Done with the headlocked buffer.
   }
 }
 
-gvr::Sizei VrShellGl::GetWebVRCompositorSurfaceSize() {
-  // This is a stopgap while we're using the WebVR compositor rendering path.
-  // TODO(klausw,crbug.com/655722): Remove this method and member once we're
-  // using a separate WebVR render surface.
-  return content_tex_physical_size_;
-}
-
 void VrShellGl::DrawUiView(const gvr::Mat4f* head_pose,
                            const std::vector<const ContentRectangle*>& elements,
                            const gvr::Sizei& render_size,
                            int viewport_offset) {
   TRACE_EVENT0("gpu", "VrShellGl::DrawUiView");
 
   gvr::Mat4f view_matrix;
   if (head_pose) {
     view_matrix = *head_pose;
   } else {
@@ skipping 185 matching lines @@
 void VrShellGl::DrawWebVr() {
   TRACE_EVENT0("gpu", "VrShellGl::DrawWebVr");
   // Don't need face culling, depth testing, blending, etc. Turn it all off.
   glDisable(GL_CULL_FACE);
   glDepthMask(GL_FALSE);
   glDisable(GL_DEPTH_TEST);
   glDisable(GL_SCISSOR_TEST);
   glDisable(GL_BLEND);
   glDisable(GL_POLYGON_OFFSET_FILL);
 
-  glViewport(0, 0, render_size_primary_.width, render_size_primary_.height);
+  glViewport(0, 0, webvr_surface_size_.width, webvr_surface_size_.height);
   vr_shell_renderer_->GetWebVrRenderer()->Draw(webvr_texture_id_);
 }
 
 void VrShellGl::OnTriggerEvent() {
   // Set a flag to handle this on the render thread at the next frame.
   touch_pending_ = true;
 }
 
 void VrShellGl::OnPause() {
   vsync_task_.Cancel();
@@ skipping 10 matching lines @@
     OnVSync();
   }
 }
 
 void VrShellGl::SetWebVrMode(bool enabled) {
   web_vr_mode_ = enabled;
 }
 
 void VrShellGl::UpdateWebVRTextureBounds(int16_t frame_index,
                                          const gvr::Rectf& left_bounds,
-                                         const gvr::Rectf& right_bounds) {
+                                         const gvr::Rectf& right_bounds,
+                                         const gvr::Sizei& source_size) {
   if (frame_index < 0) {
     webvr_left_viewport_->SetSourceUv(left_bounds);
     webvr_right_viewport_->SetSourceUv(right_bounds);
   } else {
-    pending_bounds_.emplace(
-        std::make_pair(frame_index, std::make_pair(left_bounds, right_bounds)));
+    pending_bounds_.emplace(std::make_pair(

[dcheng, 2017/03/08 07:23:40]

std::make_pair should be unnecessary with emplace

[klausw, 2017/03/08 08:11:09]

Done.

+        frame_index, WebVrBounds(left_bounds, right_bounds, source_size)));
   }
 }
 
 void VrShellGl::ContentBoundsChanged(int width, int height) {
   TRACE_EVENT0("gpu", "VrShellGl::ContentBoundsChanged");
   content_tex_css_width_ = width;
   content_tex_css_height_ = height;
 }
 
 void VrShellGl::ContentPhysicalBoundsChanged(int width, int height) {
@@ skipping 13 matching lines @@
     ui_surface_texture_->SetDefaultBufferSize(width, height);
   ui_tex_physical_size_.width = width;
   ui_tex_physical_size_.height = height;
 }
 
 base::WeakPtr<VrShellGl> VrShellGl::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 void VrShellGl::OnVSync() {
+  while (premature_received_frames_ > 0) {
+    TRACE_EVENT0("gpu", "VrShellGl::OnWebVRFrameAvailableRetry");
+    --premature_received_frames_;
+    OnWebVRFrameAvailable();
+  }
+
   base::TimeTicks now = base::TimeTicks::Now();
   base::TimeTicks target;
 
   // Don't send VSyncs until we have a timebase/interval.
   if (vsync_interval_.is_zero())
     return;
   target = now + vsync_interval_;
   int64_t intervals = (target - vsync_timebase_) / vsync_interval_;
   target = vsync_timebase_ + intervals * vsync_interval_;
   task_runner_->PostDelayedTask(FROM_HERE, vsync_task_.callback(),
                                 target - now);
 
   base::TimeDelta time = intervals * vsync_interval_;
   if (!callback_.is_null()) {
     SendVSync(time, base::ResetAndReturn(&callback_));
   } else {
     pending_vsync_ = true;
     pending_time_ = time;
   }
-  DrawFrame();
+  if (!web_vr_mode_) {
+    DrawFrame(-1);
+  }
 }
 
 void VrShellGl::OnRequest(device::mojom::VRVSyncProviderRequest request) {
   binding_.Close();
   binding_.Bind(std::move(request));
 }
 
 void VrShellGl::GetVSync(const GetVSyncCallback& callback) {
   if (!pending_vsync_) {
     if (!callback_.is_null()) {
@@ skipping 49 matching lines @@
 void VrShellGl::ResetPose() {
   // Should never call RecenterTracking when using with Daydream viewers. On
   // those devices recentering should only be done via the controller.
   if (gvr_api_ && gvr_api_->GetViewerType() == GVR_VIEWER_TYPE_CARDBOARD)
     gvr_api_->RecenterTracking();
 }
 
 void VrShellGl::CreateVRDisplayInfo(
     const base::Callback<void(device::mojom::VRDisplayInfoPtr)>& callback,
     uint32_t device_id) {
+  // This assumes that the initial webvr_surface_size_ was set to the
+  // appropriate recommended render resolution as the default size during
+  // InitializeGl. Revisit if the initialization order changes.
   device::mojom::VRDisplayInfoPtr info = VrShell::CreateVRDisplayInfo(
-      gvr_api_.get(), content_tex_physical_size_, device_id);
+      gvr_api_.get(), webvr_surface_size_, device_id);
   main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RunVRDisplayInfoCallback, callback, base::Passed(&info)));
 }
 
 }  // namespace vr_shell