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

chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc

Index: chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc
diff --git a/chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc b/chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc
new file mode 100644
index 0000000000000000000000000000000000000000..68048bf7ad1b863b59549e9270be20191bc8c14b
--- /dev/null
+++ b/chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc
@@ -0,0 +1,323 @@
+// Copyright 2017 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/mailbox_to_surface_bridge.h"
+
+#include <string>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "content/public/browser/android/compositor.h"
+#include "gpu/GLES2/gl2extchromium.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "gpu/command_buffer/common/mailbox.h"
+#include "gpu/command_buffer/common/mailbox_holder.h"
+#include "gpu/command_buffer/common/sync_token.h"
+#include "gpu/ipc/client/gpu_channel_host.h"
+#include "gpu/ipc/common/gpu_surface_tracker.h"
+#include "ui/gl/android/surface_texture.h"
+
+#include <android/native_window_jni.h>
+
+#define VOID_OFFSET(x) reinterpret_cast<void*>(x)
+#define SHADER(Src) #Src
+
+// TODO(klausw): move to static functions?
+namespace {
+
+const char* kQuadCopyVertex =
+    SHADER(attribute vec4 a_Position; attribute vec2 a_TexCoordinate;
+           varying vec2 v_TexCoordinate;
+           void main() {
+             v_TexCoordinate = a_TexCoordinate;
+             gl_Position = a_Position;
+           });
+
+const char* kQuadCopyFragment = SHADER(
+    precision highp float; uniform sampler2D u_Texture;
+    varying vec2 v_TexCoordinate;
+    void main() { gl_FragColor = texture2D(u_Texture, v_TexCoordinate); });
+
+static constexpr float kQuadVertices[16] = {
+    //   x     y    u,   v
+    -1.f, 1.f,  0.f, 1.f, -1.f, -1.f, 0.f, 0.f,
+    1.f,  -1.f, 1.f, 0.f, 1.f,  1.f,  1.f, 1.f};
+static constexpr int kQuadVerticesSize = sizeof(float) * 16;
+
+GLuint CompileShader(gpu::gles2::GLES2Interface* gl,

[mthiesse, 2017/03/08 01:00:00]

Move this into vr_gl_util.h/cc?

We could share code by calling "glShaderSource" or "gl->glShaderSource"
depending on whether gl is null.

[klausw, 2017/03/08 02:59:22]

I'd strongly prefer to keep this independent since at boliu@'s request
I'll be looking into moving this class elsewhere, and we're unlikely
to be able to use vr_gl_util from the new location.

+                     GLenum shaderType,
+                     const GLchar* shaderSource,
+                     std::string& error) {
+  GLuint shaderHandle = gl->CreateShader(shaderType);
+  if (shaderHandle != 0) {
+    // Pass in the shader source.
+    int len = strlen(shaderSource);
+    gl->ShaderSource(shaderHandle, 1, &shaderSource, &len);
+    // Compile the shader.
+    gl->CompileShader(shaderHandle);
+    // Get the compilation status.
+    GLint status = 0;
+    gl->GetShaderiv(shaderHandle, GL_COMPILE_STATUS, &status);
+    if (status == GL_FALSE) {
+      GLint infoLogLength = 0;
+      gl->GetShaderiv(shaderHandle, GL_INFO_LOG_LENGTH, &infoLogLength);

[mthiesse, 2017/03/08 01:00:01]

nit: I know you're copying this from elsewhere, but we should move the logging -
especially the string - behind "#if DCHECK_IS_ON()".

[klausw, 2017/03/08 02:59:22]

This code only gets executed if shader compilation failed, and in that case I
think it's very helpful to retrieve the driver's log information. Unless I'm
misunderstanding things, it's not logging or allocating any strings in the
success case. Unless you feel strongly about it, I'd prefer to keep it.

[mthiesse, 2017/03/08 04:15:42]

This is good now. Generally we avoid unnecessary strings in source code for
binary size, so I just wanted the 'error = "Error compiling shader: ";' line
gone.

Can you make this change in vr_gl_util too please?

+      GLchar* strInfoLog = new GLchar[infoLogLength + 1];
+      gl->GetShaderInfoLog(shaderHandle, infoLogLength, nullptr, strInfoLog);
+      error = "Error compiling shader: ";
+      error += strInfoLog;
+      delete[] strInfoLog;
+      gl->DeleteShader(shaderHandle);
+      shaderHandle = 0;
+    }
+  }
+
+  return shaderHandle;
+}
+
+GLuint CreateAndLinkProgram(gpu::gles2::GLES2Interface* gl,
+                            GLuint vertextShaderHandle,
+                            GLuint fragmentShaderHandle,
+                            std::string& error) {
+  GLuint programHandle = gl->CreateProgram();
+
+  if (programHandle != 0) {
+    // Bind the vertex shader to the program.
+    gl->AttachShader(programHandle, vertextShaderHandle);
+
+    // Bind the fragment shader to the program.
+    gl->AttachShader(programHandle, fragmentShaderHandle);
+
+    // Link the two shaders together into a program.
+    gl->LinkProgram(programHandle);
+
+    // Get the link status.
+    GLint linkStatus = 0;
+    gl->GetProgramiv(programHandle, GL_LINK_STATUS, &linkStatus);
+
+    // If the link failed, delete the program.
+    if (linkStatus == GL_FALSE) {
+      GLint infoLogLength;
+      gl->GetProgramiv(programHandle, GL_INFO_LOG_LENGTH, &infoLogLength);
+
+      GLchar* strInfoLog = new GLchar[infoLogLength + 1];
+      gl->GetProgramInfoLog(programHandle, infoLogLength, nullptr, strInfoLog);
+      error = "Error compiling program: ";
+      error += strInfoLog;
+      delete[] strInfoLog;
+      gl->DeleteProgram(programHandle);
+      programHandle = 0;
+    }
+  }
+
+  return programHandle;
+}
+
+}  // namespace
+
+namespace vr_shell {
+
+MailboxToSurfaceBridge::MailboxToSurfaceBridge() : weak_ptr_factory_(this) {}
+
+MailboxToSurfaceBridge::~MailboxToSurfaceBridge() {
+  if (surface_handle_) {
+    // Unregister from the surface tracker to avoid a resource leak.
+    gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get();
+    tracker->UnregisterViewSurface(surface_handle_);
+  }
+}
+
+void MailboxToSurfaceBridge::OnGpuChannelEstablished(
+    scoped_refptr<gpu::GpuChannelHost> gpu_channel_host) {
+  // Our attributes must be compatible with the shared offscreen

[mthiesse, 2017/03/08 01:00:00]

Add a comment to wherever the code lives that these values have to be compatible
with?

Also, file a bug?

[klausw, 2017/03/08 02:59:22]

Done. I filed 699330 and also added an IsLowEndDevice() conditional to be
compatible with CompositorImplAndroid's GetCompositorContextAttributes, though I
doubt that people will have a good experience on low end devices.

+  // surface used by virtualized contexts, otherwise mailbox
+  // synchronization doesn't work properly - it assumes a shared
+  // underlying GL context. TODO(klausw): is there a more official
+  // way to get default-compatible settings?
+  gpu::gles2::ContextCreationAttribHelper attributes;
+  attributes.alpha_size = -1;
+  attributes.red_size = 8;
+  attributes.green_size = 8;
+  attributes.blue_size = 8;
+  attributes.stencil_size = 0;
+  attributes.depth_size = 0;
+  attributes.samples = 0;
+  attributes.sample_buffers = 0;
+  attributes.bind_generates_resource = false;
+
+  bool automatic_flushes = false;
+  bool support_locking = false;
+  constexpr ui::ContextProviderCommandBuffer* shared_context_provider = nullptr;
+  context_provider_command_buffer_ =
+      make_scoped_refptr(new ui::ContextProviderCommandBuffer(
+          std::move(gpu_channel_host), gpu::GPU_STREAM_DEFAULT,
+          gpu::GpuStreamPriority::NORMAL, surface_handle_,
+          GURL("chrome://gpu/WebVRContextFactory"), automatic_flushes,
+          support_locking, gpu::SharedMemoryLimits::ForMailboxContext(),
+          attributes, shared_context_provider,
+          ui::command_buffer_metrics::CONTEXT_TYPE_UNKNOWN));
+
+  if (!context_provider_command_buffer_->BindToCurrentThread()) {
+    LOG(ERROR) << __FUNCTION__ << ";;; failed to init ContextProvider";

[mthiesse, 2017/03/08 01:00:00]

Remove this? (Or DLOG)

[klausw, 2017/03/08 02:59:22]

Changed to DLOG

+    return;
+  }
+
+  gl_ = context_provider_command_buffer_->ContextGL();
+
+  InitializeRenderer();
+}
+
+std::unique_ptr<gl::ScopedJavaSurface> MailboxToSurfaceBridge::CreateSurface(
+    scoped_refptr<gl::SurfaceTexture> surface_texture) {
+  ANativeWindow* window = surface_texture->CreateSurface();
+  gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get();
+  ANativeWindow_acquire(window);
+  // Skip ANativeWindow_setBuffersGeometry, the default size appears to work.
+  surface_handle_ = tracker->AddSurfaceForNativeWidget(window);
+
+  auto surface = base::MakeUnique<gl::ScopedJavaSurface>(surface_texture.get());
+  tracker->RegisterViewSurface(surface_handle_, surface->j_surface().obj());
+  // Unregistering happens in the destructor.
+  ANativeWindow_release(window);
+
+  gpu::GpuChannelEstablishFactory* factory =
+      content::Compositor::GetGpuChannelFactory();
+
+  factory->EstablishGpuChannel(
+      base::Bind(&MailboxToSurfaceBridge::OnGpuChannelEstablished,
+                 weak_ptr_factory_.GetWeakPtr()));
+
+  return surface;
+}
+
+void MailboxToSurfaceBridge::ResizeSurface(int width, int height) {
+  if (!gl_) {
+    LOG(ERROR) << "Cannot resize, not initialized";
+    return;
+  }
+  gl_->ResizeCHROMIUM(width, height, 1.f, false);
+  gl_->Viewport(0, 0, width, height);
+}
+
+bool MailboxToSurfaceBridge::CopyFrameToSurface(

[mthiesse, 2017/03/08 01:00:01]

nit: This function is somewhat confusing. Maybe split it into two?
ConsumeFrame(frame_index, mailbox)->GLuint, and DrawFrame(GLUint).

[klausw, 2017/03/08 02:59:22]

I've created a separate ConsumeMailbox function, simplified the
logic by moving the "discard" logic to the caller, and renamed
it to CopyMailboxToSurfaceAndSwap(mailbox). I'd like to keep
the consume + draw + swap in one place since that's the basic
abstraction the class provides, I don't want the underlying
texture available outside. It wouldn't work outside the private
GL context in this class anyway. 

If I understand it right, I don't have to do anything if I don't 
intend to consume the mailbox, it'll just expire when the VRDisplay 
side context drops its reference. (I had this happen unintentionally
in earlier versions of this CL when my synchronization was wrong.)

+    int frame_index,
+    const gpu::MailboxHolder& mailbox,
+    bool discard) {
+  TRACE_EVENT1("gpu", "MailboxToSurfaceBridge::CopyFrameToSurface", "frame",
+               frame_index);
+  if (!gl_) {
+    // We may not have a context yet, i.e. due to surface initialization
+    // being incomplete. This is not an error, but we obviously can't draw
+    // yet.
+    return false;
+  }
+
+  {
+    TRACE_EVENT0("gpu", "MailboxToSurfaceBridge::WaitSyncToken");
+    gl_->WaitSyncTokenCHROMIUM(mailbox.sync_token.GetConstData());
+  }
+
+  GLuint vrSourceTexture =
+      gl_->CreateAndConsumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.mailbox.name);
+
+  if (discard) {
+    // We've consumed the texture, but the caller requested that we
+    // don't draw it because the previous one hasn't been consumed
+    // yet. (Swapping twice on a Surfacewithout consuming one in

[mthiesse, 2017/03/08 01:00:00]

nit: s/Surfacewithout/Surface without/

[klausw, 2017/03/08 02:59:22]

Done.

+    // between from the SurfaceTexture can lose frames.) This should
+    // be rare, it's a waste of resources and can cause jerky
+    // animation due to the lost frames.
+    return false;
+  } else {
+    DrawQuad(vrSourceTexture);
+    gl_->SwapBuffers();
+    return true;
+  }
+}
+
+void MailboxToSurfaceBridge::InitializeRenderer() {
+  std::string error;
+  GLuint vertexShaderHandle =
+      CompileShader(gl_, GL_VERTEX_SHADER, kQuadCopyVertex, error);
+  CHECK(vertexShaderHandle) << error;
+
+  GLuint fragmentShaderHandle =
+      CompileShader(gl_, GL_FRAGMENT_SHADER, kQuadCopyFragment, error);
+  CHECK(fragmentShaderHandle) << error;
+
+  GLuint programHandle = CreateAndLinkProgram(gl_, vertexShaderHandle,
+                                              fragmentShaderHandle, error);
+  CHECK(programHandle) << error;
+
+  // Once the program is linked the shader objects are no longer needed
+  gl_->DeleteShader(vertexShaderHandle);
+  gl_->DeleteShader(fragmentShaderHandle);
+
+  GLuint positionHandle = gl_->GetAttribLocation(programHandle, "a_Position");
+  GLuint texCoordHandle =
+      gl_->GetAttribLocation(programHandle, "a_TexCoordinate");
+  GLuint texUniformHandle = gl_->GetUniformLocation(programHandle, "u_Texture");
+
+  GLuint vertexBuffer = 0;
+  gl_->GenBuffers(1, &vertexBuffer);
+  gl_->BindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
+  gl_->BufferData(GL_ARRAY_BUFFER, kQuadVerticesSize, kQuadVertices,
+                  GL_STATIC_DRAW);
+
+  // Set state once only, we assume that nobody else modifies GL state in a way
+  // that would interfere with our operations.
+  gl_->Disable(GL_CULL_FACE);
+  gl_->DepthMask(GL_FALSE);
+  gl_->Disable(GL_DEPTH_TEST);
+  gl_->Disable(GL_SCISSOR_TEST);
+  gl_->Disable(GL_BLEND);
+  gl_->Disable(GL_POLYGON_OFFSET_FILL);
+
+  // Not using gl_->Viewport, we assume that it defaults to the whole
+  // surface and gets updated by ResizeSurface externally as
+  // appropriate.
+
+  gl_->UseProgram(programHandle);
+
+  // Bind vertex attributes
+  gl_->BindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
+
+  gl_->EnableVertexAttribArray(positionHandle);
+  gl_->EnableVertexAttribArray(texCoordHandle);
+
+  static constexpr size_t VERTEX_STRIDE = sizeof(float) * 4;
+  static constexpr size_t POSITION_ELEMENTS = 2;
+  static constexpr size_t TEXCOORD_ELEMENTS = 2;
+  static constexpr size_t POSITION_OFFSET = 0;
+  static constexpr size_t TEXCOORD_OFFSET = sizeof(float) * 2;
+
+  gl_->VertexAttribPointer(positionHandle, POSITION_ELEMENTS, GL_FLOAT, false,
+                           VERTEX_STRIDE, VOID_OFFSET(POSITION_OFFSET));
+  gl_->VertexAttribPointer(texCoordHandle, TEXCOORD_ELEMENTS, GL_FLOAT, false,
+                           VERTEX_STRIDE, VOID_OFFSET(TEXCOORD_OFFSET));
+
+  gl_->ActiveTexture(GL_TEXTURE0);
+  gl_->Uniform1i(texUniformHandle, 0);
+}
+
+void MailboxToSurfaceBridge::DrawQuad(unsigned int textureHandle) {
+  // No need to clear since we're redrawing on top of the entire
+  // viewport, but let the GPU know we don't need the old content
+  // anymore.
+  GLenum discards[] = {GL_COLOR_EXT};
+  gl_->DiscardFramebufferEXT(GL_FRAMEBUFFER, arraysize(discards), discards);
+
+  // Configure texture. This is a 1:1 pixel copy since the surface
+  // size is resized to match the source canvas, so we can use
+  // GL_NEAREST.
+  gl_->BindTexture(GL_TEXTURE_2D, textureHandle);
+  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  gl_->DrawArrays(GL_TRIANGLE_FAN, 0, 4);
+
+  gl_->Flush();
+}
+
+}  // namespace vr_shell