Cleanup Renderer.

I believe there were some mis-understandings
about the code. It's supposed to work

StartRendering();
  BeginDraw();
  EndDraw();
  BeginDraw();
  EndDraw();
  BeginDraw();
  EndDraw();
FinishRendering();

To try to enforce correct usage I separated
some platform dependant stuff from shared
stuff and added a few flags.

Also in this CL I made Texture::GetRenderSurface
not require a pack. This is so I because it feels
cleaner that way but also because I wanted to
use surfaces without a pack to take a screenshot.

Finally I put some screenshot code in for GL.

Also fixed some bugs with locking textures.

1) I thought it was x, y, width, height but it's
actually x1, y1, x2, y2

2) I was using width in places I should have been
using mip_width

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

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

4 files changed, 87 insertions, 117 deletions

diff --git a/o3d/core/win/d3d9/renderer_d3d9.cc b/o3d/core/win/d3d9/renderer_d3d9.cc
index 3cad2d9..01ddd1f 100644
--- a/o3d/core/win/d3d9/renderer_d3d9.cc
+++ b/o3d/core/win/d3d9/renderer_d3d9.cc
@@ -1040,7 +1040,6 @@ void RendererD3D9::Clear(const Float4 &color,
                              color[3]),
          depth,
          stencil));
-  set_need_to_render(false);
 }
 
 void RendererD3D9::SetViewportInPixels(int left,
@@ -1367,45 +1366,23 @@ bool RendererD3D9::SetFullscreen(bool fullscreen,
 }
 
 // Resets the rendering stats and
-bool RendererD3D9::StartRendering() {
-  ++render_frame_count_;
-  transforms_culled_ = 0;
-  transforms_processed_ = 0;
-  draw_elements_culled_ = 0;
-  draw_elements_processed_ = 0;
-  draw_elements_rendered_ = 0;
-  primitives_rendered_ = 0;
-
+bool RendererD3D9::PlatformSpecificStartRendering() {
   // Determine whether the device is lost, resetting if possible.
   TestLostDevice();
 
-  // Only perform ops with the device if we have it.
-  if (have_device_) {
-    // Clear the client if we need to.
-    if (clear_client_) {
-      clear_client_ = false;
-      Clear(Float4(0.5f, 0.5f, 0.5f, 1.0f), true, 1.0f, true, 0, true);
-    }
-    return true;
-  } else {
-    // Return false if we have lost the device.
-    return false;
-  }
+  return have_device_;
 }
 
-// prepares DX9 for rendering the frame. Returns true on success.
-bool RendererD3D9::BeginDraw() {
+// prepares DX9 for rendering PART of the frame. Returns true on success.
+bool RendererD3D9::PlatformSpecificBeginDraw() {
   // Only perform ops with the device if we have it.
   if (have_device_) {
-    set_need_to_render(true);
     if (!HR(d3d_device_->GetRenderTarget(0, &back_buffer_surface_)))
       return false;
     if (!HR(d3d_device_->GetDepthStencilSurface(&back_buffer_depth_surface_)))
       return false;
     if (!HR(d3d_device_->BeginScene()))
       return false;
-    // Reset the viewport.
-    SetViewport(Float4(0.0f, 0.0f, 1.0f, 1.0f), Float2(0.0f, 1.0f));
     return true;
   } else {
     back_buffer_surface_ = NULL;
@@ -1416,6 +1393,8 @@ bool RendererD3D9::BeginDraw() {
   }
 }
 
+// TODO(gman): Why is this code in here? Shouldn't this use O3D to render this
+//    instead of D3D?
 void RendererD3D9::ShowFullscreenMessage(float elapsed_time,
     float display_duration) {
   RECT rect;
@@ -1473,9 +1452,22 @@ void RendererD3D9::ShowFullscreenMessage(float elapsed_time,
   d3d_device_->SetRenderState(D3DRS_ZENABLE, z_enable);
 }
 
-// Notifies DX9 that rendering of the frame is complete and swaps the buffers.
-void RendererD3D9::EndDraw() {
+// NOTE: End draw can be called multiple times per frame. If want something
+// to happen only once per frame it belongs in FinishRendering.
+void RendererD3D9::PlatformSpecificEndDraw() {
+  if (have_device_) {
+    HR(d3d_device_->EndScene());
+
+    // Release the back-buffer references.
+    back_buffer_surface_ = NULL;
+    back_buffer_depth_surface_ = NULL;
+  }
+}
+
+void RendererD3D9::PlatformSpecificFinishRendering() {
   if (have_device_) {
+    // No need to call Present(...) if we are rendering to an off-screen
+    // target.
     if (showing_fullscreen_message_) {
       // Message should display for 3 seconds after transition to fullscreen.
       float elapsed_time =
@@ -1484,24 +1476,15 @@ void RendererD3D9::EndDraw() {
       if (elapsed_time > display_duration) {
         showing_fullscreen_message_ = false;
       } else {
-        ShowFullscreenMessage(elapsed_time, display_duration);
+        if (BeginDraw()) {
+          ShowFullscreenMessage(elapsed_time, display_duration);
+          EndDraw();
+        }
       }
     }
-    HR(d3d_device_->EndScene());
-
-    set_need_to_render(false);
-
-    // Release the back-buffer references.
-    back_buffer_surface_ = NULL;
-    back_buffer_depth_surface_ = NULL;
-  }
-}
-
-void RendererD3D9::FinishRendering() {
-  // No need to call Present(...) if we are rendering to an off-screen
-  // target.
-  if (have_device_ && !off_screen_surface_ && !need_to_render()) {
-    d3d_device_->Present(NULL, NULL, NULL, NULL);
+    if (!off_screen_surface_) {
+      d3d_device_->Present(NULL, NULL, NULL, NULL);
+    }
   }
 }
 
@@ -1511,7 +1494,7 @@ void RendererD3D9::RenderElement(Element* element,
                                  Material* material,
                                  ParamObject* override,
                                  ParamCache* param_cache) {
-  ++draw_elements_rendered_;
+  IncrementDrawElementsRendered();
   // If this a new state then reset the old state.
   State *current_state = material ? material->state() : NULL;
   PushRenderStates(current_state);
@@ -1627,7 +1610,7 @@ RenderDepthStencilSurface::Ref RendererD3D9::CreateDepthStencilSurface(
                                         depth_constructor));
 }
 
-Bitmap::Ref RendererD3D9::TakeScreenshot() {
+Bitmap::Ref RendererD3D9::PlatformSpecificTakeScreenshot() {
   Bitmap::Ref empty;
   LPDIRECT3DDEVICE9 device = d3d_device();
   CComPtr<IDirect3DSurface9> system_surface;
diff --git a/o3d/core/win/d3d9/renderer_d3d9.h b/o3d/core/win/d3d9/renderer_d3d9.h
index aa04490..37572c8 100644
--- a/o3d/core/win/d3d9/renderer_d3d9.h
+++ b/o3d/core/win/d3d9/renderer_d3d9.h
@@ -74,19 +74,6 @@ class RendererD3D9 : public Renderer {
   // Released all hardware resources.
   virtual void Destroy();
 
-  // This method should be called before any draw calls take place in a
-  // frame. It clears the back buffer, stencil and depth buffers.
-  virtual bool BeginDraw();
-
-  // Finalizes the drawing of the frame.
-  virtual void EndDraw();
-
-  // Does any pre-rendering preparation
-  virtual bool StartRendering();
-
-  // Presents the results of the draw calls for this frame.
-  virtual void FinishRendering();
-
   // Attempts to reset the back buffer to its new dimensions.
   virtual void Resize(int width, int height);
 
@@ -156,9 +143,6 @@ class RendererD3D9 : public Renderer {
       int width,
       int height);
 
-  // Overridden from Renderer.
-  virtual Bitmap::Ref TakeScreenshot();
-
   inline LPDIRECT3DDEVICE9 d3d_device() const { return d3d_device_; }
   inline LPDIRECT3D9 d3d() const { return d3d_; }
 
@@ -175,6 +159,21 @@ class RendererD3D9 : public Renderer {
   explicit RendererD3D9(ServiceLocator* service_locator);
 
   // Overridden from Renderer.
+  virtual bool PlatformSpecificBeginDraw();
+
+  // Overridden from Renderer.
+  virtual void PlatformSpecificEndDraw();
+
+  // Overridden from Renderer.
+  virtual bool PlatformSpecificStartRendering();
+
+  // Overridden from Renderer.
+  virtual void PlatformSpecificFinishRendering();
+
+  // Overridden from Renderer.
+  virtual Bitmap::Ref PlatformSpecificTakeScreenshot();
+
+  // Overridden from Renderer.
   virtual ParamCache* CreatePlatformSpecificParamCache();
 
   // Overridden from Renderer.
diff --git a/o3d/core/win/d3d9/texture_d3d9.cc b/o3d/core/win/d3d9/texture_d3d9.cc
index 3707c42..a352e98 100644
--- a/o3d/core/win/d3d9/texture_d3d9.cc
+++ b/o3d/core/win/d3d9/texture_d3d9.cc
@@ -175,21 +175,26 @@ class CubeFaceSurfaceConstructor : public SurfaceConstructor {
   CubeFaceSurfaceConstructor(TextureCUBED3D9 *texture,
                              TextureCUBE::CubeFace face,
                              int mip_level)
-      : cube_texture_(texture),
+      : cube_texture_(texture->GetWeakPointer()),
         face_(face),
         mip_level_(mip_level) {
   }
 
   virtual HRESULT ConstructSurface(IDirect3DSurface9** surface) {
+    TextureCUBED3D9* texture =
+        down_cast<TextureCUBED3D9*>(cube_texture_.Get());
+    if (!texture) {
+      return E_FAIL;
+    }
     IDirect3DCubeTexture9* d3d_cube_texture =
-        static_cast<IDirect3DCubeTexture9*>(cube_texture_->GetTextureHandle());
+        static_cast<IDirect3DCubeTexture9*>(texture->GetTextureHandle());
     return d3d_cube_texture->GetCubeMapSurface(DX9CubeFace(face_),
                                                mip_level_,
                                                surface);
   }
 
  private:
-  TextureCUBED3D9::Ref cube_texture_;
+  Texture::WeakPointerType cube_texture_;
   TextureCUBE::CubeFace face_;
   int mip_level_;
   DISALLOW_COPY_AND_ASSIGN(CubeFaceSurfaceConstructor);
@@ -203,18 +208,22 @@ class CubeFaceSurfaceConstructor : public SurfaceConstructor {
 class TextureSurfaceConstructor : public SurfaceConstructor {
  public:
   TextureSurfaceConstructor(Texture2DD3D9* texture, int mip_level)
-      : texture_(texture),
+      : texture_(texture->GetWeakPointer()),
         mip_level_(mip_level) {
   }
 
   virtual HRESULT ConstructSurface(IDirect3DSurface9** surface) {
+    Texture2DD3D9* texture = down_cast<Texture2DD3D9*>(texture_.Get());
+    if (!texture) {
+      return E_FAIL;
+    }
     IDirect3DTexture9* d3d_texture =
-        static_cast<IDirect3DTexture9*>(texture_->GetTextureHandle());
+        static_cast<IDirect3DTexture9*>(texture->GetTextureHandle());
     return d3d_texture->GetSurfaceLevel(mip_level_, surface);
   }
 
  private:
-  Texture2DD3D9::Ref texture_;
+  Texture::WeakPointerType texture_;
   int mip_level_;
   DISALLOW_COPY_AND_ASSIGN(TextureSurfaceConstructor);
 };
@@ -265,7 +274,7 @@ void SetTextureRect(
   DCHECK(src_data);
   bool compressed = Texture::IsCompressedFormat(format);
 
-  RECT rect = {dst_left, dst_top, src_width, src_height};
+  RECT rect = {dst_left, dst_top, dst_left + src_width, dst_top + src_height};
   D3DLOCKED_RECT out_rect = {0};
 
   if (!HR(d3d_texture->LockRect(
@@ -304,7 +313,7 @@ void SetTextureFaceRect(
   DCHECK(src_data);
   bool compressed = Texture::IsCompressedFormat(format);
 
-  RECT rect = {dst_left, dst_top, src_width, src_height};
+  RECT rect = {dst_left, dst_top, dst_left + src_width, dst_top + src_height};
   D3DLOCKED_RECT out_rect = {0};
 
   D3DCUBEMAP_FACES d3d_face = DX9CubeFace(face);
@@ -391,7 +400,7 @@ Texture2DD3D9* Texture2DD3D9::Create(ServiceLocator* service_locator,
                                              resize_to_pot,
                                              enable_render_surfaces);
   if (resize_to_pot) {
-    texture->backing_bitmap_->Allocate(format, width, height, levels, 
+    texture->backing_bitmap_->Allocate(format, width, height, levels,
                                        Bitmap::IMAGE);
   }
 
@@ -455,8 +464,8 @@ void Texture2DD3D9::UpdateBackedMipLevel(unsigned int level) {
   }
 }
 
-RenderSurface::Ref Texture2DD3D9::GetRenderSurface(int mip_level, Pack* pack) {
-  DCHECK(pack);
+RenderSurface::Ref Texture2DD3D9::PlatformSpecificGetRenderSurface(
+    int mip_level) {
   if (!render_surfaces_enabled()) {
     O3D_ERROR(service_locator())
         << "Attempting to get RenderSurface from non-render-surface-enabled"
@@ -471,19 +480,13 @@ RenderSurface::Ref Texture2DD3D9::GetRenderSurface(int mip_level, Pack* pack) {
     return RenderSurface::Ref(NULL);
   }
 
-  RenderSurface::Ref render_surface(
+  return RenderSurface::Ref(
       new RenderSurfaceD3D9(
           service_locator(),
           width() >> mip_level,
           height() >> mip_level,
           this,
           new TextureSurfaceConstructor(this, mip_level)));
-
-  if (!render_surface.IsNull()) {
-    RegisterSurface(render_surface.Get(), pack);
-  }
-
-  return render_surface;
 }
 
 void Texture2DD3D9::SetRect(int level,
@@ -571,16 +574,18 @@ bool Texture2DD3D9::Lock(int level, void** texture_data, int* pitch) {
         << "Attempting to lock a render-target texture: " << name();
     return false;
   }
+
+  unsigned int mip_width = image::ComputeMipDimension(level, width());
+  unsigned int mip_height = image::ComputeMipDimension(level, height());
+
   if (resize_to_pot_) {
     DCHECK(backing_bitmap_->image_data());
     *texture_data = backing_bitmap_->GetMipData(level);
-    unsigned int mip_width = image::ComputeMipDimension(level, width());
-    unsigned int mip_height = image::ComputeMipDimension(level, height());
     *pitch = image::ComputePitch(format(), mip_width);
     locked_levels_ |= 1 << level;
     return true;
   } else {
-    RECT rect = {0, 0, width(), height()};
+    RECT rect = {0, 0, mip_width, mip_height};
     D3DLOCKED_RECT out_rect = {0};
 
     if (HR(d3d_texture_->LockRect(level, &out_rect, &rect, 0))) {
@@ -793,9 +798,9 @@ void TextureCUBED3D9::UpdateBackedMipLevel(TextureCUBE::CubeFace face,
   }
 }
 
-RenderSurface::Ref TextureCUBED3D9::GetRenderSurface(TextureCUBE::CubeFace face,
-                                                     int mip_level,
-                                                     Pack* pack) {
+RenderSurface::Ref TextureCUBED3D9::PlatformSpecificGetRenderSurface(
+    TextureCUBE::CubeFace face,
+    int mip_level) {
   if (!render_surfaces_enabled()) {
     O3D_ERROR(service_locator())
         << "Attempting to get RenderSurface from non-render-surface-enabled"
@@ -811,19 +816,13 @@ RenderSurface::Ref TextureCUBED3D9::GetRenderSurface(TextureCUBE::CubeFace face,
   }
 
   int edge = edge_length() >> mip_level;
-  RenderSurface::Ref render_surface(
+  return RenderSurface::Ref(
       new RenderSurfaceD3D9(
           service_locator(),
           edge,
           edge,
           this,
           new CubeFaceSurfaceConstructor(this, face, mip_level)));
-
-  if (!render_surface.IsNull()) {
-    RegisterSurface(render_surface.Get(), pack);
-  }
-
-  return render_surface;
 }
 
 void TextureCUBED3D9::SetRect(TextureCUBE::CubeFace face,
@@ -921,17 +920,19 @@ bool TextureCUBED3D9::Lock(
         << "Attempting to lock a render-target texture: " << name();
     return false;
   }
+
+  unsigned int mip_width = image::ComputeMipDimension(level, edge_length());
+  unsigned int mip_height = mip_width;
+
   if (resize_to_pot_) {
     Bitmap* backing_bitmap = backing_bitmaps_[face].Get();
     DCHECK(backing_bitmap->image_data());
     *texture_data = backing_bitmap->GetMipData(level);
-    unsigned int mip_width = image::ComputeMipDimension(level, edge_length());
-    unsigned int mip_height = mip_width;
     *pitch = image::ComputePitch(format(), mip_width);
     locked_levels_[face] |= 1 << level;
     return true;
   } else {
-    RECT rect = {0, 0, edge_length(), edge_length()};
+    RECT rect = {0, 0, mip_width, mip_height};
     D3DLOCKED_RECT out_rect = {0};
 
     if (HR(d3d_cube_texture_->LockRect(DX9CubeFace(face), level,
diff --git a/o3d/core/win/d3d9/texture_d3d9.h b/o3d/core/win/d3d9/texture_d3d9.h
index 311f096..c8c948b 100644
--- a/o3d/core/win/d3d9/texture_d3d9.h
+++ b/o3d/core/win/d3d9/texture_d3d9.h
@@ -80,14 +80,6 @@ class Texture2DD3D9 : public Texture2D {
   // Returns the implementation-specific texture handle for this texture.
   virtual void* GetTextureHandle() const { return d3d_texture_; }
 
-  // Returns a RenderSurface object associated with a mip_level of a texture.
-  // Parameters:
-  //  mip_level: [in] The mip-level of the surface to be returned.
-  //  pack: [in] The pack in which the surface will reside.
-  // Returns:
-  //  Reference to the RenderSurface object.
-  virtual RenderSurface::Ref GetRenderSurface(int mip_level, Pack* pack);
-
   // Handler for lost device. This invalidates the texture for a device reset.
   bool OnLostDevice();
 
@@ -105,6 +97,9 @@ class Texture2DD3D9 : public Texture2D {
   // Overridden from Texture2D
   virtual bool Unlock(int level);
 
+  // Overridden from Texture2D
+  virtual RenderSurface::Ref PlatformSpecificGetRenderSurface(int mip_level);
+
  private:
   // Initializes the Texture2DD3D9 from a DX9 texture.
   Texture2DD3D9(ServiceLocator* service_locator,
@@ -163,18 +158,6 @@ class TextureCUBED3D9 : public TextureCUBE {
   // Returns the implementation-specific texture handle for this texture.
   virtual void* GetTextureHandle() const { return d3d_cube_texture_; }
 
-  // Returns a RenderSurface object associated with a given cube face and
-  // mip_level of a texture.
-  // Parameters:
-  //  face: [in] The cube face from which to extract the surface.
-  //  mip_level: [in] The mip-level of the surface to be returned.
-  //  pack: [in] The pack in which the surface will reside.
-  // Returns:
-  //  Reference to the RenderSurface object.
-  virtual RenderSurface::Ref GetRenderSurface(CubeFace face,
-                                              int level,
-                                              Pack* pack);
-
   // Handler for lost device. This invalidates the texture for a device reset.
   bool OnLostDevice();
 
@@ -192,6 +175,10 @@ class TextureCUBED3D9 : public TextureCUBE {
   // Overridden from TextureCUBE
   bool Unlock(CubeFace face, int level);
 
+  // Overridden from TextureCUBE.
+  virtual RenderSurface::Ref PlatformSpecificGetRenderSurface(CubeFace face,
+                                                              int level);
+
  private:
   TextureCUBED3D9(ServiceLocator* service_locator,
                   IDirect3DCubeTexture9* tex,