WebVR: Use content CVC size for compositor rendering

chrome/browser/android/vr_shell/vr_shell.cc

Index: chrome/browser/android/vr_shell/vr_shell.cc
diff --git a/chrome/browser/android/vr_shell/vr_shell.cc b/chrome/browser/android/vr_shell/vr_shell.cc
index 881af78ea80ad4fa08807c50d1e987647f6b70f0..9334877d83edcc23a523470254a8dafa1098d16a 100644
--- a/chrome/browser/android/vr_shell/vr_shell.cc
+++ b/chrome/browser/android/vr_shell/vr_shell.cc
@@ -251,30 +251,51 @@ void VrShell::InitializeGl(JNIEnv* env,
   content_texture_id_ = content_texture_handle;
   ui_texture_id_ = ui_texture_handle;
 
+  // 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.
+  webvr_texture_id_ = content_texture_id_;
+
   gvr_api_->InitializeGl();
   std::vector<gvr::BufferSpec> specs;
+  // For kFramePrimaryBuffer (primary VrShell and WebVR content)
   specs.push_back(gvr_api_->CreateBufferSpec());
-  render_size_ = specs[0].GetSize();
+  render_size_primary_ = specs[kFramePrimaryBuffer].GetSize();
 
-  // For WebVR content
+  // 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[specs.size() - 1].SetSize(1024, 1024);

[mthiesse, 2016/11/16 21:30:41]

magic numbers to constants with comment

[mthiesse, 2016/11/16 21:30:41]

nit: specs.back()

[klausw, 2016/11/16 23:37:39]

Done.

[klausw, 2016/11/16 23:37:39]

Done:

// Pixel dimensions and field of view for the head-locked content. This
// is currently sized to fit the WebVR "insecure transport" warnings,
// adjust it as needed if there is additional content.
static constexpr gvr::Sizei kHeadlockedBufferDimensions = {1024, 1024};
static constexpr gvr::Rectf kHeadlockedBufferFov = {20.f, 20.f, 20.f, 20.f};

+  render_size_headlocked_ = specs[kFrameHeadlockedBuffer].GetSize();
 
   swap_chain_.reset(new gvr::SwapChain(gvr_api_->CreateSwapChain(specs)));
 
   vr_shell_renderer_.reset(new VrShellRenderer());
+
+  // Allocate a buffer viewport for use in UI drawing. This isn't
+  // initialized at this point, it'll be set from other viewport list
+  // entries as needed.
+  buffer_viewport_.reset(
+      new gvr::BufferViewport(gvr_api_->CreateBufferViewport()));
+
+  // Set up main content viewports. The list has two elements, 0=left
+  // eye and 1=right eye.
   buffer_viewport_list_.reset(
       new gvr::BufferViewportList(gvr_api_->CreateEmptyBufferViewportList()));
   buffer_viewport_list_->SetToRecommendedBufferViewports();
 
-  buffer_viewport_.reset(
-      new gvr::BufferViewport(gvr_api_->CreateBufferViewport()));
-
+  // Set up head-locked UI viewports, these will be elements 2=left eye
+  // and 3=right eye. For now, use a hardcoded 20-degree-from-center FOV
+  // frustum to reduce rendering cost for this overlay. This fits the
+  // current content, but will need to be adjusted once there's more dynamic
+  // head-locked content that could be larger.
   headlocked_left_viewport_.reset(
       new gvr::BufferViewport(gvr_api_->CreateBufferViewport()));
   buffer_viewport_list_->GetBufferViewport(GVR_LEFT_EYE,
                                            headlocked_left_viewport_.get());
   headlocked_left_viewport_->SetSourceBufferIndex(kFrameHeadlockedBuffer);
   headlocked_left_viewport_->SetReprojection(GVR_REPROJECTION_NONE);
+  headlocked_left_viewport_->SetSourceFov({20.f, 20.f, 20.f, 20.f});
 
   headlocked_right_viewport_.reset(
       new gvr::BufferViewport(gvr_api_->CreateBufferViewport()));
@@ -282,7 +303,10 @@ void VrShell::InitializeGl(JNIEnv* env,
                                            headlocked_right_viewport_.get());
   headlocked_right_viewport_->SetSourceBufferIndex(kFrameHeadlockedBuffer);
   headlocked_right_viewport_->SetReprojection(GVR_REPROJECTION_NONE);
+  headlocked_right_viewport_->SetSourceFov({20.f, 20.f, 20.f, 20.f});
 
+  // Save copies of the first two viewport items for use by WebVR, it
+  // sets its own UV bounds.
   webvr_left_viewport_.reset(
       new gvr::BufferViewport(gvr_api_->CreateBufferViewport()));
   buffer_viewport_list_->GetBufferViewport(GVR_LEFT_EYE,
@@ -498,6 +522,15 @@ uint32_t GetPixelEncodedPoseIndex() {
 void VrShell::DrawFrame(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   buffer_viewport_list_->SetToRecommendedBufferViewports();
 
+  if (html_interface_->GetMode() == UiInterface::Mode::WEB_VR) {

[mthiesse, 2016/11/16 21:30:41]

Do you need to undo this when not in webVR?

[klausw, 2016/11/16 23:37:38]

I don't think so, I added this comment:

    // If needed, resize the primary buffer for use with WebVR.
    // For now, this is never reverted, at this point there is no
    // transition from WebVR mode to VrShell mode other than
    // destroying/recreating the VrShell instance. Revisit this if
    // that assumption changes.

[klausw, 2016/11/16 23:50:04]

On second thought this isn't hard to fix to make it possible
to enter WebVR from VrShell mode and return. I'll send an update.

[klausw, 2016/11/17 00:07:42]

Done, though it doesn't currently seem to work right at ToT + this patch.

+    if (render_size_primary_.width != webvr_width_ ||
+        render_size_primary_.height != webvr_height_) {
+      render_size_primary_.width = webvr_width_;
+      render_size_primary_.height = webvr_height_;
+      swap_chain_->ResizeBuffer(kFramePrimaryBuffer, render_size_primary_);
+    }
+  }
+
   gvr::Frame frame = swap_chain_->AcquireFrame();
   gvr::ClockTimePoint target_time = gvr::GvrApi::GetTimePointNow();
   target_time.monotonic_system_time_nanos += kPredictionTimeWithoutVsyncNanos;
@@ -574,28 +607,43 @@ void VrShell::DrawVrShell(const gvr::Mat4f& head_pose,
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
   }
 
-  DrawUiView(&head_pose, world_elements);
+  DrawUiView(&head_pose, world_elements, render_size_primary_, 0);

[bajones, 2016/11/16 21:23:52]

Would prefer that this used something like kPrimaryViewportOffset instead of
magic number 0

[klausw, 2016/11/16 23:37:39]

Done, using kViewportListPrimaryOffset.

 
   if (!head_locked_elements.empty()) {
     // Switch to head-locked viewports.
     size_t last_viewport = buffer_viewport_list_->GetSize();
-    buffer_viewport_list_->SetBufferViewport(last_viewport++,
-        *headlocked_left_viewport_);
-    buffer_viewport_list_->SetBufferViewport(last_viewport++,
-        *headlocked_right_viewport_);
+    if (last_viewport <= 2) {

[bajones, 2016/11/16 21:23:52]

Not an issue with this patch, but I'm curious: Did you find that this was
actually a problem?

[klausw, 2016/11/16 23:37:39]

I don't think it was a problem, SetToRecommendedBufferViewports gets
run every frame and apparently installs the pair of viewports we use
for the primary buffer.

I changed it to avoid the "++" which looked worrying:

    buffer_viewport_list_->SetBufferViewport(
        kViewportListHeadlockedOffset + GVR_LEFT_EYE,
        *headlocked_left_viewport_);
    buffer_viewport_list_->SetBufferViewport(
        kViewportListHeadlockedOffset + GVR_RIGHT_EYE,
        *headlocked_right_viewport_);

+      buffer_viewport_list_->SetBufferViewport(last_viewport++,
+                                               *headlocked_left_viewport_);
+      buffer_viewport_list_->SetBufferViewport(last_viewport++,
+                                               *headlocked_right_viewport_);
+    }
 
     // Bind the headlocked framebuffer.
     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);
+    DrawUiView(nullptr, head_locked_elements, render_size_headlocked_, 2);

[bajones, 2016/11/16 21:23:52]

Same magic number concern here: kHeadlockedViewportOffset?

[klausw, 2016/11/16 23:37:38]

Done, using kViewportListHeadlockedOffset

   }
 }
 
+void VrShell::SetWebVRRenderSurfaceSize(int width, int height) {
+  webvr_width_ = width;
+  webvr_height_ = height;
+  // TODO(klausw): set the WebVR render surface size here once we have that.
+}
+
+gvr::Sizei VrShell::GetWebVRCompositorSurfaceSize() {
+  return render_size_primary_;
+}
+
+
 void VrShell::DrawUiView(const gvr::Mat4f* head_pose,
-                         const std::vector<const ContentRectangle*>& elements) {
+                         const std::vector<const ContentRectangle*>& elements,
+                         const gvr::Sizei& render_size, int viewport_offset) {
   for (auto eye : {GVR_LEFT_EYE, GVR_RIGHT_EYE}) {
-    buffer_viewport_list_->GetBufferViewport(eye, buffer_viewport_.get());
+    buffer_viewport_list_->GetBufferViewport(
+        eye + viewport_offset, buffer_viewport_.get());
 
     gvr::Mat4f view_matrix = gvr_api_->GetEyeFromHeadMatrix(eye);
     if (head_pose != nullptr) {
@@ -603,7 +651,7 @@ void VrShell::DrawUiView(const gvr::Mat4f* head_pose,
     }
 
     gvr::Recti pixel_rect =
-        CalculatePixelSpaceRect(render_size_, buffer_viewport_->GetSourceUv());
+        CalculatePixelSpaceRect(render_size, buffer_viewport_->GetSourceUv());
     glViewport(pixel_rect.left, pixel_rect.bottom,
                pixel_rect.right - pixel_rect.left,
                pixel_rect.top - pixel_rect.bottom);
@@ -728,11 +776,13 @@ void VrShell::DrawWebVr() {
   glDisable(GL_BLEND);
   glDisable(GL_POLYGON_OFFSET_FILL);
 
-  glViewport(0, 0, render_size_.width, render_size_.height);
-  vr_shell_renderer_->GetWebVrRenderer()->Draw(content_texture_id_);
+  glViewport(0, 0, render_size_primary_.width, render_size_primary_.height);
+  vr_shell_renderer_->GetWebVrRenderer()->Draw(webvr_texture_id_);
 
-  buffer_viewport_list_->SetBufferViewport(0, *webvr_left_viewport_);
-  buffer_viewport_list_->SetBufferViewport(1, *webvr_right_viewport_);
+  buffer_viewport_list_->SetBufferViewport(GVR_LEFT_EYE,
+                                           *webvr_left_viewport_);
+  buffer_viewport_list_->SetBufferViewport(GVR_RIGHT_EYE,
+                                           *webvr_right_viewport_);
 }
 
 void VrShell::OnTriggerEvent(JNIEnv* env, const JavaParamRef<jobject>& obj) {
@@ -784,6 +834,7 @@ void VrShell::SetWebVrMode(JNIEnv* env,
 
 void VrShell::SetWebVRSecureOrigin(bool secure_origin) {
   html_interface_->SetSecureOrigin(secure_origin);
+  webvr_needs_ui_ = !secure_origin;

[bajones, 2016/11/16 21:23:52]

Doesn't look like this is actually used anywhere?

[klausw, 2016/11/16 23:37:38]

Correct, removed here and in the .h file.

This used to be a shortcut to avoid UI rendering, I've now added a
"if (!world_elements.empty())" check in DrawVrShell instead along with
some minor rearrangement.

 }
 
 void VrShell::SubmitWebVRFrame() {