Allow VRDisplay to specify which frame the layer bounds should be updated at.

chrome/browser/android/vr_shell/vr_shell_gl.h

Index: chrome/browser/android/vr_shell/vr_shell_gl.h
diff --git a/chrome/browser/android/vr_shell/vr_shell_gl.h b/chrome/browser/android/vr_shell/vr_shell_gl.h
index 24ce9627f29d5f2323d00e060f8e370615c26838..60c21e60f773ae88b7cf0e1b74279580200fe62a 100644
--- a/chrome/browser/android/vr_shell/vr_shell_gl.h
+++ b/chrome/browser/android/vr_shell/vr_shell_gl.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_ANDROID_VR_SHELL_VR_SHELL_GL_H_
 
 #include <memory>
+#include <queue>
 
 #include "base/cancelable_callback.h"
 #include "base/macros.h"
@@ -79,7 +80,8 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
   void UIPhysicalBoundsChanged(int width, int height);
   base::WeakPtr<VrShellGl> GetWeakPtr();
 
-  void UpdateWebVRTextureBounds(const gvr::Rectf& left_bounds,
+  void UpdateWebVRTextureBounds(int16_t frame_index,
+                                const gvr::Rectf& left_bounds,
                                 const gvr::Rectf& right_bounds);
   gvr::GvrApi* gvr_api();
   void SetGvrPoseForWebVr(const gvr::Mat4f& pose, uint32_t pose_num);
@@ -113,7 +115,7 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
 
   void OnUIFrameAvailable();
   void OnContentFrameAvailable();
-  bool GetPixelEncodedPoseIndexByte(int* pose_index);
+  bool GetPixelEncodedFrameIndex(uint16_t* frame_index);
 
   void OnVSync();
 
@@ -122,7 +124,7 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
 
   void ForceExitVr();
 
-  device::mojom::VRPosePtr GetPose();
+  void SendVSync(base::TimeDelta time, const GetVSyncCallback& callback);
 
   // samplerExternalOES texture data for UI content image.
   int ui_texture_id_ = 0;
@@ -147,6 +149,8 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
   std::unique_ptr<gvr::BufferViewport> webvr_left_viewport_;
   std::unique_ptr<gvr::BufferViewport> webvr_right_viewport_;
   std::unique_ptr<gvr::SwapChain> swap_chain_;
+  typedef std::pair<gvr::Rectf, gvr::Rectf> BoundsPair;

[dcheng, 2017/01/27 09:41:33]

Nit: prefer using A = B over typedef B A;

[mthiesse, 2017/01/27 16:15:31]

Done.

+  std::queue<std::pair<uint8_t, BoundsPair>> pending_bounds_;
 
   // Current sizes for the render buffers.
   gvr::Sizei render_size_primary_;
@@ -167,10 +171,6 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
   gvr::Sizei content_tex_physical_size_ = {0, 0};
   gvr::Sizei ui_tex_physical_size_ = {0, 0};
 
-  // The pose ring buffer size must be a power of two to avoid glitches when
-  // the pose index wraps around. It should be large enough to handle the
-  // current backlog of poses which is 2-3 frames.
-  static constexpr int kPoseRingBufferSize = 8;
   std::vector<gvr::Mat4f> webvr_head_pose_;
   int webvr_texture_id_ = 0;
   bool web_vr_mode_;
@@ -196,8 +196,9 @@ class VrShellGl : public device::mojom::VRVSyncProvider {
   base::WeakPtr<VrShellDelegate> delegate_provider_;
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
 
-  uint32_t pose_index_ = 1;
-  int last_pose_ = 0;
+  uint8_t frame_index_ = 0;
+  // larger than frame_index_ so it can be initialized out-of-band.
+  uint16_t last_frame_index_ = -1;

[klausw (use chromium instead), 2017/01/19 01:34:27]

Initializing unsigned integers to negative numbers seems odd. Assuming it just
needs to be big enough to store a frame index and -1, just make it a signed
int16_t?

[mthiesse, 2017/01/26 23:23:03]

dcheng asked for all types to be unsigned, so we don't get a mismatch of signed
and unsigned. I tend to agree.

Initializing unsigned to -1 isn't very common, but it's not unheard of. See
https://cs.chromium.org/search/?q=uint.*%5C+%3D%5C+-1;&sq=package:chromium&ty...

[klausw, 2017/01/27 00:24:55]

How about UINT16_MAX from cstdint then? The -1 initialization seems like a
potential trap for someone doing a (last_frame_index >= 0) check somewhere,
though presumably the compiler would catch that.

[mthiesse, 2017/01/27 00:57:28]

So std::numeric_limits<decltype(last_frame_index_)>::max() is an option, but I
decided against it because it doesn't really convey the intent like -1 does. It
isn't really obvious that UINT16_MAX is impossible to reach after
initialization, especially when comparing against it in the cc file. -1 clearly
conveys that this is an uninitialized value that shouldn't be reached after
initialization.

Writing last_frame_index >= 0 for a uint type is just clearly a mistake in any
case, so we shouldn't worry about it.

 
   base::WeakPtrFactory<VrShellGl> weak_ptr_factory_;