GPU client side changes for GpuMemoryBuffers

gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc

Index: gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc
diff --git a/gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc b/gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc
new file mode 100644
index 0000000000000000000000000000000000000000..32b857c7f5a31dfe675e4eb48f83ed0169824084
--- /dev/null
+++ b/gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc
@@ -0,0 +1,147 @@
+// Copyright (c) 2013 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 <GLES2/gl2.h>
+#include <GLES2/gl2chromium.h>
+#include <GLES2/gl2ext.h>
+#include <GLES2/gl2extchromium.h>
+
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "gpu/command_buffer/client/gpu_memory_buffer_factory.h"
+#include "gpu/command_buffer/client/gpu_memory_buffer_mock.h"
+#include "gpu/command_buffer/service/image_manager.h"
+#include "gpu/command_buffer/tests/gl_manager.h"
+#include "gpu/command_buffer/tests/gl_test_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/native_widget_types.h"
+#include "ui/gl/gl_image.h"
+#include "ui/gl/gl_image_mock.h"
+
+using testing::_;
+using testing::Return;
+using testing::SetArgPointee;
+using testing::StrictMock;
+
+namespace gpu {
+
+static const int kGpuMemoryBufferWidth = 256;
+static const int kGpuMemoryBufferHeight = 256;
+static const int kGpuMemoryBufferBytesPerPixel = 4;
+
+class GLGpuMemoryBufferTest : public testing::Test {

[greggman, 2013/04/25 23:45:34]

I'm conflicted on these tests. The test in gpu/command_buffer/tests are supposed
to be functional tests. That means they are supposed to call into the actual
driver. These ones don't call into the actual driver because they're using a
GpuMemoryBufferMock.

Can you move these to gles2_implementation_unittests.cc

If you want you can keep these tests too but remove the mock and use a real
factory.

Change the code to check glGetString (like some of the other tests do) to check
that your extension exists. Then make the tests actually expect to work, not
just mock.

[no sievers, 2013/04/26 00:17:43]

Here is the dilemna: We can't run the real test upstream because of dependencies
on Android (this is for WebVIew) on the actual GpuMemoryBuffer impl.

I like the readability of this test that uses the gl*() APIs and only cares
about how gles2_implementation creates and calls into other objects.

But so obviously it should be a gles2 impl unittest then. But would be nice it
could be pretty much copied 1:1 and not be cluttered with setting up cmdbuffer
expectations.

You think we can somehow set up a new test class with faked/ignored cmdbuffer
and helper calls and is part of gpu_unittests? 

+ public:
+  scoped_ptr<GpuMemoryBuffer> CreateGpuMemoryBufferMock(int width, int height) {
+    scoped_ptr<StrictMock<GpuMemoryBufferMock> > gpu_memory_buffer(
+        new StrictMock<GpuMemoryBufferMock>(width, height));
+    gpu_memory_buffer_ = gpu_memory_buffer.get();
+    return gpu_memory_buffer.PassAs<GpuMemoryBuffer>();
+  }
+ protected:
+  virtual void SetUp() {
+    GLManager::Options options;
+    image_manager_ = new gles2::ImageManager;
+    options.image_manager = image_manager_.get();
+    gl_.Initialize(options);
+    gl_.MakeCurrent();
+    mapped_buffer_.reset(new uint8[
+        kGpuMemoryBufferWidth * kGpuMemoryBufferHeight
+        * kGpuMemoryBufferBytesPerPixel]);
+    SetProcessDefaultGpuMemoryBufferFactory(
+        base::Bind(&gpu::GLGpuMemoryBufferTest::CreateGpuMemoryBufferMock,
+            base::Unretained(this)));
+  }
+
+  virtual void TearDown() {
+    gl_.Destroy();
+  }
+
+  scoped_refptr<gles2::ImageManager> image_manager_;
+  scoped_ptr<uint8[]> mapped_buffer_;
+  StrictMock<GpuMemoryBufferMock>* gpu_memory_buffer_;
+  GLManager gl_;
+};
+
+// An end to end test that tests the whole GpuMemoryBuffer lifecycle.
+TEST_F(GLGpuMemoryBufferTest, Lifecycle) {

[reveman, 2013/04/26 02:47:57]

The code below doesn't look right to me. The glGetString() stuff is really
abusing the api.

Should glImageBufferDataCHROMIUM be implemented at a higher level
(WebGraphicsContext3DInProcessCommandBufferImpl)? At this level I would expect
you to just call gpu_memory_buffer_tracker_->CreateBuffer(buffer_id, width,
height) and create a GLImage from that. Calling glImageBufferDataCHROMIUM here
and then having dig up a gfx::GpuMemoryBufferHandle seem backwards.

[kaanb1, 2013/04/26 03:01:21]

Do we have access to ImageManager from GLES2Implementation? That was the
motivating factor to push a gfx::GpuMemoryBufferHandle to WGC3D.

+  // Create a client texture id.
+  GLuint texture_id;
+  glGenTextures(1, &texture_id);
+
+  // Create a client buffer id.
+  GLuint buffer_id;
+  glGenBuffers(1, &buffer_id);
+
+  // Allocate the GpuMemoryBuffer.
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, buffer_id);
+  glImageBufferDataCHROMIUM(
+      GL_IMAGE_BUFFER_CHROMIUM, kGpuMemoryBufferWidth, kGpuMemoryBufferHeight);
+  // Create the GLImage and insert it into the ImageManager.
+  glFlush();
+  EXPECT_CALL(*gpu_memory_buffer_, GetNativeBuffer())
+      .Times(1)
+      .WillOnce(Return(gpu_memory_buffer_))
+      .RetiresOnSaturation();
+  gfx::GpuMemoryBufferHandle buffer =
+      const_cast<GLubyte*>(
+          glGetString(GL_BOUND_GPU_MEMORY_BUFFER_CHROMIUM_POINTER));
+  EXPECT_NE(static_cast<gfx::GpuMemoryBufferHandle>(NULL), buffer);
+
+  gfx::Size size(kGpuMemoryBufferWidth, kGpuMemoryBufferHeight);
+  gfx::GLImageMock* gl_image = new gfx::GLImageMock(buffer, size);
+  image_manager_->AddImage(gl_image, buffer_id);
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, 0);
+
+  // Map the buffer.
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, buffer_id);
+  EXPECT_CALL(*gpu_memory_buffer_, Map(_, _))
+      .Times(1)
+      .WillOnce(SetArgPointee<1>(mapped_buffer_.get()))
+      .RetiresOnSaturation();
+  void* mapped_buffer =
+      glMapBufferCHROMIUM(GL_IMAGE_BUFFER_CHROMIUM, GL_WRITE_ONLY);
+  EXPECT_EQ(mapped_buffer_.get(), mapped_buffer);
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, 0);
+
+  // Unmap the buffer.
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, buffer_id);
+  EXPECT_CALL(*gpu_memory_buffer_, Unmap())
+      .Times(1)
+      .RetiresOnSaturation();
+  glUnmapBufferCHROMIUM(GL_IMAGE_BUFFER_CHROMIUM);
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, 0);
+
+
+  // Bind the texture and the image.
+  glBindTexture(GL_TEXTURE_2D, texture_id);
+  EXPECT_CALL(*gl_image, BindTexImage())
+      .Times(1)
+      .WillOnce(Return(true))
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_image, GetSize())
+      .Times(1)
+      .WillOnce(Return(size))
+      .RetiresOnSaturation();
+  glBindTexImage2DCHROMIUM(GL_TEXTURE_2D, buffer_id);
+
+  // Release the buffer.
+  EXPECT_CALL(*gpu_memory_buffer_, Die())
+      .Times(1)
+      .RetiresOnSaturation();
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, buffer_id);
+  glImageBufferDataCHROMIUM(
+      GL_IMAGE_BUFFER_CHROMIUM, 0, 0);
+  glBindBuffer(GL_IMAGE_BUFFER_CHROMIUM, 0);
+
+  // Delete the buffer and remove it from the ImageManager.
+  glFlush();
+  image_manager_->RemoveImage(buffer_id);
+  glDeleteBuffers(1, &buffer_id);
+
+  // Delete the texture.
+  glDeleteTextures(1, &texture_id);
+}
+
+}  // namespace gpu