Apply the Surface Texture transformation matrix during texture copy.

content/common/gpu/stream_texture_android.cc

Index: content/common/gpu/stream_texture_android.cc
diff --git a/content/common/gpu/stream_texture_android.cc b/content/common/gpu/stream_texture_android.cc
index 84a0d1fa64223664b8c9fb6e41e062820ee38e1b..807e312638f78abbfe52f77484c5a4bbbdc43ed5 100644
--- a/content/common/gpu/stream_texture_android.cc
+++ b/content/common/gpu/stream_texture_android.cc
@@ -5,6 +5,7 @@
 #include "content/common/gpu/stream_texture_android.h"
 
 #include "base/bind.h"
+#include "base/strings/stringize_macros.h"
 #include "content/common/android/surface_texture_peer.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/gpu_messages.h"
@@ -14,6 +15,8 @@
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/gl_context.h"
+#include "ui/gl/gl_helper.h"
+#include "ui/gl/scoped_binders.h"
 #include "ui/gl/scoped_make_current.h"
 
 namespace content {
@@ -64,7 +67,14 @@ StreamTexture::StreamTexture(GpuCommandBufferStub* owner_stub,
       owner_stub_(owner_stub),
       route_id_(route_id),
       has_listener_(false),
+      matrix_modified_(false),
       texture_id_(texture_id),
+      framebuffer_(0),
+      vertex_shader_(0),
+      fragment_shader_(0),
+      program_(0),
+      vertex_buffer_(0),
+      u_xform_loc_(-1),
       weak_factory_(this) {
   owner_stub->AddDestructionObserver(this);
   memset(current_matrix_, 0, sizeof(current_matrix_));
@@ -83,6 +93,23 @@ StreamTexture::~StreamTexture() {
 void StreamTexture::OnWillDestroyStub() {
   owner_stub_->RemoveDestructionObserver(this);
   owner_stub_->channel()->RemoveRoute(route_id_);
+
+  if (framebuffer_) {
+    scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current(MakeStubCurrent());
+
+    glDeleteProgram(program_);
+    glDeleteShader(vertex_shader_);
+    glDeleteShader(fragment_shader_);
+    glDeleteBuffersARB(1, &vertex_buffer_);
+    glDeleteFramebuffersEXT(1, &framebuffer_);
+    program_ = 0;
+    vertex_shader_ = 0;
+    fragment_shader_ = 0;
+    vertex_buffer_ = 0;
+    framebuffer_ = 0;
+    u_xform_loc_ = -1;
+  }
+
   owner_stub_ = NULL;
 
   // If the owner goes away, there is no need to keep the SurfaceTexture around.
@@ -94,7 +121,142 @@ void StreamTexture::Destroy(bool have_context) {
   NOTREACHED();
 }
 
+scoped_ptr<ui::ScopedMakeCurrent> StreamTexture::MakeStubCurrent() {
+  scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current;
+  bool needs_make_current =
+      !owner_stub_->decoder()->GetGLContext()->IsCurrent(NULL);
+  // On Android we should not have to perform a real context switch here when
+  // using virtual contexts.
+  DCHECK(!needs_make_current ||
+         !owner_stub_->decoder()
+              ->GetContextGroup()
+              ->feature_info()
+              ->workarounds()
+              .use_virtualized_gl_contexts);
+  if (needs_make_current) {
+    scoped_make_current.reset(new ui::ScopedMakeCurrent(
+        owner_stub_->decoder()->GetGLContext(), owner_stub_->surface()));
+  }
+  return scoped_make_current;
+}
+
+void StreamTexture::DoUpdateTexImage() {
+  DCHECK(surface_texture_.get());
+
+  scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current(MakeStubCurrent());
+
+  surface_texture_->UpdateTexImage();
+  has_valid_frame_ = true;
+  has_pending_frame_ = false;
+
+  if (scoped_make_current.get()) {
+    // UpdateTexImage() implies glBindTexture().
+    // The cmd decoder takes care of restoring the binding for this GLImage as
+    // far as the current context is concerned, but if we temporarily change
+    // it, we have to keep the state intact in *that* context also.
+    const gpu::gles2::ContextState* state =
+        owner_stub_->decoder()->GetContextState();
+    const gpu::gles2::TextureUnit& active_unit =
+        state->texture_units[state->active_texture_unit];
+    glBindTexture(GL_TEXTURE_EXTERNAL_OES,
+                  active_unit.bound_texture_external_oes.get()
+                      ? active_unit.bound_texture_external_oes->service_id()
+                      : 0);
+  }
+}
+
+bool StreamTexture::DoCopyTexture(const gfx::Point& offset,
+                                  const gfx::Rect& rect) {
+  if (!owner_stub_ || !surface_texture_.get())

[reveman, 2015/12/23 16:05:36]

when does this happen and why is it correct to return true in that case?

[Tobias Sargeant, 2015/12/23 20:06:14]

This could happen when the command buffer is in the process of being destroyed.
Returning true early-outs the texture copy code, which presumably would fail
anyway.

It also mirrors existing code on line 263

[reveman, 2015/12/23 23:11:48]

Ok, maybe fine then but I'm somewhat worried that this might result in us
drawing uninitialized content which would be a security issue. returning false
would feel better as that would make the decoder code either not draw the
texture or initialize it to black.

+    return true;
+
+  if (!offset.IsOrigin()) {
+    LOG(ERROR) << "Non-origin offset is not supported";
+    return false;
+  }
+
+  if (rect != gfx::Rect(size_)) {
+    LOG(ERROR) << "Sub-rectangle is not supported";
+    return false;
+  }
+  GLint target_texture = 0;
+  glGetIntegerv(GL_TEXTURE_BINDING_2D, &target_texture);
+  DCHECK(target_texture);
+
+  DoUpdateTexImage();
+  UpdateTransformMatrix();
+
+  if (!framebuffer_) {
+    LOG(WARNING) << "XXX " << "first copy init";

[reveman, 2015/12/23 16:05:36]

I assume this is not supposed to land.

[Tobias Sargeant, 2015/12/23 20:06:14]

You're right. Sorry I missed it when I went through the diff. I'll remove it in
the next PS.

+
+    glGenFramebuffersEXT(1, &framebuffer_);
+
+    // clang-format off
+    const char kVertexShader[] = STRINGIZE(
+      attribute vec2 a_position;
+      varying vec2 v_texCoord;
+      uniform mat4 u_xform;
+      void main() {
+        gl_Position = vec4(a_position.x, a_position.y, 0.0, 1.0);
+        vec2 uv_untransformed = (a_position + vec2(1.0, 1.0)) * 0.5;
+        v_texCoord = (u_xform * vec4(uv_untransformed, 0.0, 1.0)).xy;
+      }
+    );
+    const char kFragmentShader[] =
+      "#extension GL_OES_EGL_image_external : require\n" STRINGIZE(
+      precision mediump float;
+      uniform samplerExternalOES a_texture;
+      varying vec2 v_texCoord;
+      void main() {
+        gl_FragColor = texture2D(a_texture, v_texCoord);
+      }
+    );
+    // clang-format on
+
+    vertex_buffer_ = gfx::GLHelper::SetupQuadVertexBuffer();
+    vertex_shader_ = gfx::GLHelper::LoadShader(GL_VERTEX_SHADER, kVertexShader);
+    fragment_shader_ =
+        gfx::GLHelper::LoadShader(GL_FRAGMENT_SHADER, kFragmentShader);
+    program_ = gfx::GLHelper::SetupProgram(vertex_shader_, fragment_shader_);
+    gfx::ScopedUseProgram use_program(program_);
+    int sampler_location = glGetUniformLocation(program_, "a_texture");
+    DCHECK_NE(-1, sampler_location);
+    glUniform1i(sampler_location, 0);
+    u_xform_loc_ = glGetUniformLocation(program_, "u_xform");
+    DCHECK_NE(-1, u_xform_loc_);
+  }
+
+  gfx::ScopedActiveTexture active_texture(GL_TEXTURE0);
+  // UpdateTexImage() call below will bind the surface texture to
+  // TEXTURE_EXTERNAL_OES. This scoped texture binder will restore the current
+  // binding before this function returns.
+  gfx::ScopedTextureBinder texture_binder(GL_TEXTURE_EXTERNAL_OES, texture_id_);
+
+  {
+    gfx::ScopedFrameBufferBinder framebuffer_binder(framebuffer_);
+    gfx::ScopedViewport viewport(0, 0, size_.width(), size_.height());
+    glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                              GL_TEXTURE_2D, target_texture, 0);
+    DCHECK_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE),
+              glCheckFramebufferStatusEXT(GL_FRAMEBUFFER));
+    gfx::ScopedUseProgram use_program(program_);
+
+    glUniformMatrix4fv(u_xform_loc_, 1, false, current_matrix_);
+    gfx::GLHelper::DrawQuad(vertex_buffer_);
+
+    // Detach the output texture from the fbo.
+    glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                              GL_TEXTURE_2D, 0, 0);
+  }
+
+  return true;
+}
+
 bool StreamTexture::CopyTexImage(unsigned target) {
+  if (target == GL_TEXTURE_2D) {
+    return DoCopyTexture(gfx::Point(), gfx::Rect(size_));
+  }
+
   if (target != GL_TEXTURE_EXTERNAL_OES)

[reveman, 2015/12/23 16:05:36]

do we still need this non-TEXTURE_2D code?

[Tobias Sargeant, 2015/12/23 20:06:14]

Do you mean the return false for !2D, !EXTERNAL_OES? I presume it's possible
that we could be passed a RECT or CUBE_MAP texture, which the texture copying
code above will not handle. The default path may still, and returning false here
is the minimal change in behaviour for that case.

If you mean the EXTERNAL_OES path that follows, then we do need it because in
the non sync-IPC case (and for clank, which doesn't have the context problems
that webview has) we still rely on passing this texture directly in a resource,
and drawing from it without a copy. Spitzer might change this, but for the
moment the current behaviour should stay.

[reveman, 2015/12/23 23:11:49]

Acknowledged.

     return false;
 
@@ -110,39 +272,7 @@ bool StreamTexture::CopyTexImage(unsigned target) {
   if (static_cast<unsigned>(texture_id) != texture_id_)
     return false;
 
-  if (has_pending_frame_) {
-    scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current;
-    bool needs_make_current =
-        !owner_stub_->decoder()->GetGLContext()->IsCurrent(NULL);
-    // On Android we should not have to perform a real context switch here when
-    // using virtual contexts.
-    DCHECK(!needs_make_current || !owner_stub_->decoder()
-                                       ->GetContextGroup()
-                                       ->feature_info()
-                                       ->workarounds()
-                                       .use_virtualized_gl_contexts);
-    if (needs_make_current) {
-      scoped_make_current.reset(new ui::ScopedMakeCurrent(
-          owner_stub_->decoder()->GetGLContext(), owner_stub_->surface()));
-    }
-    surface_texture_->UpdateTexImage();
-    has_valid_frame_ = true;
-    has_pending_frame_ = false;
-    if (scoped_make_current.get()) {
-      // UpdateTexImage() implies glBindTexture().
-      // The cmd decoder takes care of restoring the binding for this GLImage as
-      // far as the current context is concerned, but if we temporarily change
-      // it, we have to keep the state intact in *that* context also.
-      const gpu::gles2::ContextState* state =
-          owner_stub_->decoder()->GetContextState();
-      const gpu::gles2::TextureUnit& active_unit =
-          state->texture_units[state->active_texture_unit];
-      glBindTexture(GL_TEXTURE_EXTERNAL_OES,
-                    active_unit.bound_texture_external_oes.get()
-                        ? active_unit.bound_texture_external_oes->service_id()
-                        : 0);
-    }
-  }
+  DoUpdateTexImage();
 
   TextureManager* texture_manager =
       owner_stub_->decoder()->GetContextGroup()->texture_manager();
@@ -156,22 +286,29 @@ bool StreamTexture::CopyTexImage(unsigned target) {
                            gpu::gles2::Texture::UNBOUND);
   }
 
-  if (has_listener_ && has_valid_frame_) {
+  UpdateTransformMatrix();
+  return true;
+}
+
+void StreamTexture::UpdateTransformMatrix() {
+  if (has_valid_frame_) {
     float mtx[16];
     surface_texture_->GetTransformMatrix(mtx);
 
     // Only query the matrix once we have bound a valid frame.
     if (memcmp(current_matrix_, mtx, sizeof(mtx)) != 0) {
+      matrix_modified_ = true;
       memcpy(current_matrix_, mtx, sizeof(mtx));
+    }
 
+    if (has_listener_ && owner_stub_) {
       GpuStreamTextureMsg_MatrixChanged_Params params;
       memcpy(&params.m00, mtx, sizeof(mtx));
       owner_stub_->channel()->Send(
           new GpuStreamTextureMsg_MatrixChanged(route_id_, params));
+      matrix_modified_ = false;
     }
   }
-
-  return true;
 }
 
 void StreamTexture::OnFrameAvailable() {
@@ -206,6 +343,7 @@ bool StreamTexture::OnMessageReceived(const IPC::Message& message) {
 void StreamTexture::OnStartListening() {
   DCHECK(!has_listener_);
   has_listener_ = true;
+  matrix_modified_ = true;
 }
 
 void StreamTexture::OnEstablishPeer(int32 primary_id, int32 secondary_id) {
@@ -230,6 +368,10 @@ void StreamTexture::ReleaseTexImage(unsigned target) {
 bool StreamTexture::CopyTexSubImage(unsigned target,
                                     const gfx::Point& offset,
                                     const gfx::Rect& rect) {
+  if (target == GL_TEXTURE_2D) {
+    return DoCopyTexture(offset, rect);
+  }
+
   return false;
 }