Review URL: http://codereview.chromium.org/176026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26273 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 194 insertions, 44 deletions

diff --git a/o3d/core/cross/command_buffer/render_surface_cb.cc b/o3d/core/cross/command_buffer/render_surface_cb.cc
index 0f57536e..f65ce3d 100644
--- a/o3d/core/cross/command_buffer/render_surface_cb.cc
+++ b/o3d/core/cross/command_buffer/render_surface_cb.cc
@@ -49,8 +49,12 @@ RenderSurfaceCB::RenderSurfaceCB(ServiceLocator *service_locator,
                                  RendererCB *renderer)
     : RenderSurface(service_locator, width, height, texture),
       resource_id_(command_buffer::kInvalidResource),
-      renderer_(renderer) {
+      renderer_(renderer) {

+  DCHECK_GT(width, 0);

+  DCHECK_GT(height, 0);

+  DCHECK_GT(mip_level, -1);
   DCHECK(texture);
+  DCHECK(renderer);
 
   ResourceID id = renderer_->render_surface_ids().AllocateID();
   resource_id_ = id;
@@ -58,13 +62,13 @@ RenderSurfaceCB::RenderSurfaceCB(ServiceLocator *service_locator,
   CommandBufferEntry args[4];
   args[0].value_uint32 = id;
   args[1].value_uint32 =
-      create_render_surface_cmd::Width::MakeValue(width) |
-      create_render_surface_cmd::Height::MakeValue(height);
+    create_render_surface_cmd::Width::MakeValue(width) |
+    create_render_surface_cmd::Height::MakeValue(height);
   args[2].value_uint32 =
     create_render_surface_cmd::Levels::MakeValue(mip_level) |
     create_render_surface_cmd::Side::MakeValue(side);
   args[3].value_uint32 =
-      reinterpret_cast<ResourceID>(texture->GetTextureHandle());
+    reinterpret_cast<ResourceID>(texture->GetTextureHandle());
   helper->AddCommand(command_buffer::CREATE_RENDER_SURFACE, 4, args);
 }
 
@@ -73,7 +77,7 @@ RenderSurfaceCB::~RenderSurfaceCB() {
 }
 
 void RenderSurfaceCB::Destroy() {
-  // This should never get called during rendering.
+  // This should never be called during rendering.
   if (resource_id_ != command_buffer::kInvalidResource) {
     CommandBufferHelper *helper = renderer_->helper();
     CommandBufferEntry args[1];
@@ -91,15 +95,18 @@ RenderDepthStencilSurfaceCB::RenderDepthStencilSurfaceCB(
     RendererCB *renderer)
     : RenderDepthStencilSurface(service_locator, width, height),
       resource_id_(command_buffer::kInvalidResource),
-      renderer_(renderer) {
+      renderer_(renderer) {

+  DCHECK_GT(width, 0);

+  DCHECK_GT(height, 0);
+  DCHECK(renderer);
   ResourceID id = renderer_->depth_surface_ids().AllocateID();
   resource_id_ = id;
   CommandBufferHelper *helper = renderer_->helper();
   CommandBufferEntry args[2];
   args[0].value_uint32 = id;
   args[1].value_uint32 =
-      create_render_surface_cmd::Width::MakeValue(width) |
-      create_render_surface_cmd::Height::MakeValue(height);
+    create_render_surface_cmd::Width::MakeValue(width) |
+    create_render_surface_cmd::Height::MakeValue(height);
   helper->AddCommand(command_buffer::CREATE_DEPTH_SURFACE, 2, args);
 }
 
diff --git a/o3d/core/cross/command_buffer/render_surface_cb.h b/o3d/core/cross/command_buffer/render_surface_cb.h
index 76581ac..357d0f7 100644
--- a/o3d/core/cross/command_buffer/render_surface_cb.h
+++ b/o3d/core/cross/command_buffer/render_surface_cb.h
@@ -33,16 +33,39 @@
 #ifndef O3D_CORE_CROSS_COMMAND_BUFFER_RENDER_SURFACE_CB_H_

 #define O3D_CORE_CROSS_COMMAND_BUFFER_RENDER_SURFACE_CB_H_

 

+// This file contains the definition of the CB versions of

+// render surface sub-classes.

+

 #include "core/cross/render_surface.h"

 #include "core/cross/command_buffer/renderer_cb.h"

 #include "command_buffer/common/cross/resource.h"

 

 namespace o3d {

 

+// The RenderSurfaceCB class represents a render surface in the core library

+// of the client for command buffers. This class is responsible for sending

+// calls across the command buffer to create an actual render surface resource

+// on the server.

 class RenderSurfaceCB : public RenderSurface {

  public:

   typedef SmartPointer<RenderSurfaceCB> Ref;

 

+  // The render surface maintains a reference to its texture and renderer but

+  // does not own them. The owner of the render surface is responsible for

+  // properly deleting any textures.

+  // Parameters:

+  //   service_locator - for central lookup. Not owned by RenderSurfaceCB.

+  //   width - width of the bitmap for this render surface. It must match the

+  //     the width of the texture at 'mip_level'

+  //   height - height of the bitmap for this render surface. It must match the

+  //     the height of the texture at 'mip_level'

+  //   mip_level - mip level of 'texture' for this render surface.

+  //   side - which side of a cube texture the render surface represents. The

+  //     'side' parameter will not be used for a texture2d render surface.

+  //   texture - the texture this render surface maps to. Not owned by

+  //     RenderSurfaceCB.

+  //   renderer - the renderer to render to render surface. Not owned by

+  //     RenderSurfaceCB.

   RenderSurfaceCB(ServiceLocator *service_locator,

                   int width,

                   int height,

@@ -50,41 +73,65 @@ class RenderSurfaceCB : public RenderSurface {
                   int side,

                   Texture *texture,

                   RendererCB *renderer);

-  virtual ~RenderSurfaceCB();
-
+  virtual ~RenderSurfaceCB();

+

+  // The CB specific implementation of GetBitmap.

+  // Returns:

+  //   a reference to the underlying bitmap of the render surface.

   virtual Bitmap::Ref PlatformSpecificGetBitmap() const {

     // TODO(rlp): Add this functionality.

     DCHECK(false);

     return Bitmap::Ref();

   }

 

+  // Destroys any data structures associated with the render surface and

+  // resets any allocated IDs. This function should never be called during

+  // rendering.

   virtual void Destroy();

 

-  // Gets the render surface resource ID.

+  // Returns the render surface resource ID.

   command_buffer::ResourceID resource_id() const { return resource_id_; }

+

  private:

   command_buffer::ResourceID resource_id_;

-  RendererCB* renderer_;

+  RendererCB *renderer_;

   DISALLOW_COPY_AND_ASSIGN(RenderSurfaceCB);

 };

 

+// The RenderDepthStencilSurfaceCB class represents a depth stencil surface in

+// the core library of the client for command buffers. This class is

+// responsible for sending calls across the command buffer to create an actual

+// depth stencil surface resource on the server.

 class RenderDepthStencilSurfaceCB : public RenderDepthStencilSurface {

  public:

   typedef SmartPointer<RenderDepthStencilSurfaceCB> Ref;

 

+  // The depth stencil surface maintains a reference to its renderer which is

+  // also what typically creates it (though not always). The depth stencil

+  // does not maintain ownership of the renderer.

+  // Parameters:

+  //   service_locator - for central lookup.

+  //   width - width of the bitmap for this render surface.

+  //   height - height of the bitmap for this render surface.

+  //   renderer - the renderer to render to render surface. Not owned by

+  //     RenderDepthStencilSurfaceCB.

   RenderDepthStencilSurfaceCB(ServiceLocator *service_locator,

                               int width,

                               int height,

                               RendererCB *renderer);

   virtual ~RenderDepthStencilSurfaceCB() {}

 

+  // Destroys any data structures associated with the render surface and

+  // resets any allocated IDs. This function should never be called during

+  // rendering.

   virtual void Destroy();

 

-  // Gets the render depth stencil surface resource ID.

+  // Returns the render depth stencil surface resource ID.

   command_buffer::ResourceID resource_id() const { return resource_id_; }

+

  private:

   command_buffer::ResourceID resource_id_;

-  RendererCB* renderer_;

+  RendererCB *renderer_;

   DISALLOW_COPY_AND_ASSIGN(RenderDepthStencilSurfaceCB);

 };

 

diff --git a/o3d/core/cross/render_surface_test.cc b/o3d/core/cross/render_surface_test.cc
index 34e1e6d..5aafbfe 100644
--- a/o3d/core/cross/render_surface_test.cc
+++ b/o3d/core/cross/render_surface_test.cc
@@ -36,6 +36,7 @@
 #include "core/cross/pack.h"

 #include "core/cross/renderer.h"

 #include "core/cross/bitmap.h"

+#include "core/cross/features.h"

 #include "core/cross/texture.h"

 #include "core/cross/render_surface.h"

 #include "core/cross/render_surface_set.h"

@@ -43,48 +44,102 @@
 #include "core/cross/error_status.h"

 

 // Defined in testing_common.cc, for each platform.

-extern o3d::DisplayWindow* g_display_window;

+extern o3d::DisplayWindow *g_display_window;

 

 namespace o3d {

 

-class RenderSurfaceTest : public testing::Test {

+// A mock render which pushes commands to the renderer so that

+// actual rendering calls that maintain state can be handled by a

+// variable rather than actually rendering.

+class MockRenderer {

  public:

-  RenderSurfaceTest()

-      : object_manager_(g_service_locator),

-        error_status_(g_service_locator) {

+  // Creates a forwarding render class that pushes functionality to be tested

+  // to the desired renderer and handles other functionality on its own.

+  explicit MockRenderer(Renderer *renderer) : renderer_(renderer) {}

+

+  virtual ~MockRenderer() {}

+

+  // Rather than actually rendering, this just sets the state in the renderer.

+  void StartRendering() {

+    renderer_->set_rendering(true);

+  }

+

+  // This resets the state in the renderer.

+  void FinishRendering() {

+    renderer_->set_rendering(false);

+  }

+

+  // Pushes SetRenderSurfaces to the renderer.

+  void SetRenderSurfaces(const RenderSurface *surface,

+                         const RenderDepthStencilSurface *depth_surface,

+                         bool is_back_buffer) {

+    renderer_->SetRenderSurfaces(surface, depth_surface, is_back_buffer);

   }

 

-  Renderer* renderer() {

-    return g_renderer;

+  // Pushes GetRenderSurfaces to the renderer.

+  void GetRenderSurfaces(const RenderSurface **surface,

+                         const RenderDepthStencilSurface **depth_surface,

+                         bool *is_back_buffer) {

+    renderer_->GetRenderSurfaces(surface, depth_surface, is_back_buffer);

   }

+ private:

+  Renderer *renderer_;

+};

+

+// Class for testing render surfaces and associated functionality.

+class RenderSurfaceTest : public testing::Test {

+ public:

+  RenderSurfaceTest()

+      : object_manager_(g_service_locator),

+        error_status_(g_service_locator) {}

 

  protected:

   virtual void SetUp() {

+    service_locator_ = new ServiceLocator;

+    features_ = new Features(service_locator_);

     pack_ = object_manager_->CreatePack();

-    g_renderer->StartRendering();

+    renderer_ = new MockRenderer(g_renderer);

+    renderer_->StartRendering();

   }

 

   virtual void TearDown() {

-    g_renderer->FinishRendering();

+    renderer_->FinishRendering();

     pack_->Destroy();

     error_status_.ClearLastError();

+    delete features_;

+    delete service_locator_;

+    delete renderer_;

+  }

+

+  ServiceLocator* service_locator() const {

+    return service_locator_;

+  }

+

+  MockRenderer* renderer() const {

+    return renderer_;

   }

 

-  Pack* pack() { return pack_; }

+  Pack* pack() const {

+    return pack_;

+  }

 

   ServiceDependency<ObjectManager> object_manager_;

   ErrorStatus error_status_;

-  Pack* pack_;

+  ServiceLocator *service_locator_;

+  Features *features_;

+  Pack *pack_;

+  MockRenderer *renderer_;

 };

 

-// Test that non PoT textures can't make render surfaces

+// Tests that non PoT textures can't make render surfaces.

 TEST_F(RenderSurfaceTest, NonPowerOfTwoRenderSurfaceEnabled) {

-  Texture2D* texture = pack()->CreateTexture2D(20, 32, Texture::ARGB8, 2, true);

+  Texture2D *texture = pack()->CreateTexture2D(20, 32, Texture::ARGB8, 2, true);

   ASSERT_TRUE(NULL == texture);

 }

-// Test that a render surface can be created

+

+// Tests that a render surface can be created from a texture 2d.

 TEST_F(RenderSurfaceTest, CreateRenderSurfaceFromTexture2D) {

-  Texture2D* texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

+  Texture2D *texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

   ASSERT_TRUE(NULL != texture);

 

   RenderSurface::Ref render_surface = texture->GetRenderSurface(0);

@@ -94,33 +149,36 @@ TEST_F(RenderSurfaceTest, CreateRenderSurfaceFromTexture2D) {
   ASSERT_EQ(render_surface->height(), 32);

 }

 

+// Tests that a render surface can be created from a cube texture.

 TEST_F(RenderSurfaceTest, CreateRenderSurfaceFromTextureCUBE) {

-  TextureCUBE* texture = pack()->CreateTextureCUBE(16, Texture::ARGB8, 2, true);

+  TextureCUBE *texture = pack()->CreateTextureCUBE(16, Texture::ARGB8, 2, true);

   ASSERT_TRUE(NULL != texture);

 

   RenderSurface::Ref render_surface = texture->GetRenderSurface(

-      TextureCUBE::FACE_POSITIVE_X, 0);

+    TextureCUBE::FACE_POSITIVE_X, 0);

   ASSERT_TRUE(NULL != render_surface);

   ASSERT_TRUE(NULL != render_surface->texture());

   ASSERT_EQ(render_surface->width(), 16);

   ASSERT_EQ(render_surface->height(), 16);

 }

 

+// Tests the renderer's functionality to swap render surfaces and that

+// the correct render surfaces are set.

 TEST_F(RenderSurfaceTest, SwapRenderSurfaces) {

-  Texture2D* texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

+  Texture2D *texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

   ASSERT_TRUE(NULL != texture);

 

   RenderSurface::Ref render_surface = texture->GetRenderSurface(0);

   ASSERT_TRUE(NULL != render_surface);

   ASSERT_TRUE(texture == render_surface->texture());

 

-  RenderDepthStencilSurface* depth_surface =

-      pack()->CreateDepthStencilSurface(16, 32);

+  RenderDepthStencilSurface *depth_surface =

+    pack()->CreateDepthStencilSurface(16, 32);

 

   // Now swap surfaces.

   renderer()->SetRenderSurfaces(render_surface, depth_surface, false);

-  const RenderSurface* test_render_surface = NULL;

-  const RenderDepthStencilSurface* test_depth_surface = NULL;

+  const RenderSurface *test_render_surface = NULL;

+  const RenderDepthStencilSurface *test_depth_surface = NULL;

   bool test_is_back_buffer;

   renderer()->GetRenderSurfaces(&test_render_surface, &test_depth_surface,

                                 &test_is_back_buffer);

@@ -129,19 +187,57 @@ TEST_F(RenderSurfaceTest, SwapRenderSurfaces) {
   ASSERT_FALSE(test_is_back_buffer);

 }

 

+// Tests the renderer's functionality to swap render surfaces and return

+// the old one to the main rendering surface.

+TEST_F(RenderSurfaceTest, SetBackSurfaces) {

+  Texture2D *texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

+  ASSERT_TRUE(NULL != texture);

+

+  RenderSurface::Ref render_surface = texture->GetRenderSurface(0);

+  ASSERT_TRUE(NULL != render_surface);

+  ASSERT_TRUE(texture == render_surface->texture());

+

+  RenderDepthStencilSurface *depth_surface =

+    pack()->CreateDepthStencilSurface(16, 32);

+

+  // Save the original surfaces for comparison.

+  const RenderSurface *original_render_surface = NULL;

+  const RenderDepthStencilSurface *original_depth_surface = NULL;

+  bool original_is_back_buffer;

+  renderer()->GetRenderSurfaces(&original_render_surface,

+                                &original_depth_surface,

+                                &original_is_back_buffer);

+  // Now swap surfaces.

+  renderer()->SetRenderSurfaces(render_surface, depth_surface, false);

+  // Return the back buffers

+  renderer()->SetRenderSurfaces(NULL, NULL, true);

+  // Get the original surfaces again for comparison.

+  const RenderSurface *restored_render_surface = NULL;

+  const RenderDepthStencilSurface *restored_depth_surface = NULL;

+  bool restored_is_back_buffer;

+  renderer()->GetRenderSurfaces(&original_render_surface,

+                                &original_depth_surface,

+                                &restored_is_back_buffer);

+  ASSERT_TRUE(original_render_surface == restored_render_surface);

+  ASSERT_TRUE(original_depth_surface == restored_depth_surface);

+  ASSERT_TRUE(restored_is_back_buffer);

+}

+

+// Tests the render surfaces interaction as part of a render surface set

+// which is how they are commonly used in practice.

 TEST_F(RenderSurfaceTest, RenderSurfaceSetTest) {

-  Texture2D* texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

+  Texture2D *texture = pack()->CreateTexture2D(16, 32, Texture::ARGB8, 2, true);

   ASSERT_TRUE(NULL != texture);

 

   RenderSurface::Ref render_surface = texture->GetRenderSurface(0);

   ASSERT_TRUE(NULL != render_surface);

   ASSERT_TRUE(texture == render_surface->texture());

 

-  RenderDepthStencilSurface* depth_surface =

-      pack()->CreateDepthStencilSurface(16, 32);

+  RenderDepthStencilSurface *depth_surface =

+    pack()->CreateDepthStencilSurface(16, 32);

   ASSERT_TRUE(depth_surface != NULL);

 

-  RenderSurfaceSet* render_surface_set = pack()->Create<RenderSurfaceSet>();

+  RenderSurfaceSet *render_surface_set = pack()->Create<RenderSurfaceSet>();

   ASSERT_TRUE(render_surface_set != NULL);

   render_surface_set->set_render_surface(render_surface);

   render_surface_set->set_render_depth_stencil_surface(depth_surface);

@@ -149,15 +245,15 @@ TEST_F(RenderSurfaceTest, RenderSurfaceSetTest) {
 

   RenderContext render_context(g_renderer);

 

-  const RenderSurface* old_render_surface = NULL;

-  const RenderDepthStencilSurface* old_depth_surface = NULL;

+  const RenderSurface *old_render_surface = NULL;

+  const RenderDepthStencilSurface *old_depth_surface = NULL;

   bool old_is_back_buffer = false;

   renderer()->GetRenderSurfaces(&old_render_surface, &old_depth_surface,

                                 &old_is_back_buffer);

 

   render_surface_set->Render(&render_context);

-  const RenderSurface* test_render_surface = NULL;

-  const RenderDepthStencilSurface* test_depth_surface = NULL;

+  const RenderSurface *test_render_surface = NULL;

+  const RenderDepthStencilSurface *test_depth_surface = NULL;

   bool test_is_back_buffer = false;

   renderer()->GetRenderSurfaces(&test_render_surface, &test_depth_surface,

                                 &test_is_back_buffer);