CL removing inheritance of SkDevice from PlatformDevice. Flavours of PlatformDevice classes now ...

skia/ext/vector_platform_device_skia.cc

Index: skia/ext/vector_platform_device_skia.cc
===================================================================
--- skia/ext/vector_platform_device_skia.cc	(revision 96545)
+++ skia/ext/vector_platform_device_skia.cc	(working copy)
@@ -5,6 +5,7 @@
 #include "skia/ext/vector_platform_device_skia.h"
 
 #include "skia/ext/bitmap_platform_device.h"
+#include "skia/ext/platform_canvas.h"
 #include "third_party/skia/include/core/SkClipStack.h"
 #include "third_party/skia/include/core/SkDraw.h"
 #include "third_party/skia/include/core/SkRect.h"
@@ -20,8 +21,8 @@
 }
 
 VectorPlatformDeviceSkia::VectorPlatformDeviceSkia(SkPDFDevice* pdf_device)
-    : PlatformDevice(makeABitmap(pdf_device->width(), pdf_device->height())),
-      pdf_device_(pdf_device) {
+    : PlatformDevice(pdf_device) {//PlatformDevice(makeABitmap(pdf_device->width(), pdf_device->height())),
+      //pdf_device_(pdf_device) {
 }
 
 VectorPlatformDeviceSkia::~VectorPlatformDeviceSkia() {
@@ -39,36 +40,49 @@
   // image in EndPlatformPaint.
   DCHECK(raster_surface_ == NULL);
 #if defined(OS_WIN)
-  raster_surface_ = BitmapPlatformDevice::create(pdf_device_->width(),
-                                                 pdf_device_->height(),
+  raster_surface_ = BitmapPlatformDevice::create(GetOwningDevice()->width(),
+                                                 GetOwningDevice()->height(),
                                                  false, /* not opaque */
                                                  NULL);
 #elif defined(OS_POSIX) && !defined(OS_MACOSX)
-  raster_surface_ = BitmapPlatformDevice::Create(pdf_device_->width(),
-                                                 pdf_device_->height(),
+  raster_surface_ = BitmapPlatformDevice::Create(GetOwningDevice()->width(),
+                                                 GetOwningDevice()->height(),
                                                  false /* not opaque */);
 #endif
   raster_surface_->unref();  // SkRefPtr and create both took a reference.
 
-  SkCanvas canvas(raster_surface_.get());
-  return raster_surface_->BeginPlatformPaint();
+  //SkCanvas canvas(raster_surface_.get());
+  raster_canvas_ = new SkCanvas(raster_surface_.get());
+  //return raster_surface_->BeginPlatformPaint();
+  return skia::BeginPlatformPaint(raster_canvas_.get());
 }
 
 void VectorPlatformDeviceSkia::EndPlatformPaint() {
   DCHECK(raster_surface_ != NULL);
+
+  // BitmapPlatformDevice matches begin and end calls.
+  skia::EndPlatformPaint(raster_canvas_.get());

[Jeff Timanus, 2011/08/12 23:59:22]

This was an example of where BitmapPlatformDevice::onAccessBitmap was required. 
The code was drawing from a GDI backed bitmap before calling EndPlatformPaint,
and so no gdiFlush would have been called.  By placing the EndPlatformPaint call
at the entrance to the routine, the problem should be fixed.

   SkPaint paint;
   // SkPDFDevice checks the passed SkDraw for an empty clip (only).  Fake
   // it out by setting a non-empty clip.
   SkDraw draw;
-  SkRegion clip(SkIRect::MakeWH(pdf_device_->width(), pdf_device_->height()));
+  SkRegion clip(SkIRect::MakeWH(GetOwningDevice()->width(),
+                                GetOwningDevice()->height()));
   draw.fClip=&clip;
-  pdf_device_->drawSprite(draw, raster_surface_->accessBitmap(false), 0, 0,
-                          paint);
-  // BitmapPlatformDevice matches begin and end calls.
-  raster_surface_->EndPlatformPaint();
+
+  SkCanvas pdf_canvas(GetOwningDevice());

[vandebo (ex-Chrome), 2011/08/15 19:52:10]

Will using two different canvases (with different clips and transforms) confuse
the device if it uses setMatrixClip?  Seems like we might need to match the
matrix and clip in this canvas even though drawSprite doesn't really need them.

Another solution might be a static canvas method to draw a sprite to a device
without molesting the device's other state.

[Jeff Timanus, 2011/08/15 20:06:25]

A third option is to make drawSprite a public entry point to SkDevice.  Are
there reasons why we should not take this approach?  Which is cleaner, from an
API perspective - public drawSprite call, or static method that preserves state?

[vandebo (ex-Chrome), 2011/08/15 20:13:56]

I don't think either of these are particularly clean...  The consumer isn't
really meant to deal with drawing to the device directly, that's the job of
canvas.

Is there enough information exposed from canvas to clone the state into the
local canvas you already have?

There aren't any particular ramifications for PDF for any of these options, but
that might not be true for GPU.  Maybe we can get Brian to weigh in.

+  pdf_canvas.drawSprite(raster_surface_->accessBitmap(false), 0, 0, &paint);
+#if 0
+  GetOwningDevice()->drawSprite(
+      draw,
+      raster_surface_->accessBitmap(false), 0, 0, paint);
+#endif
+  //raster_surface_->EndPlatformPaint();
+  raster_canvas_ = NULL;
   raster_surface_ = NULL;
 }
 
+#if 0
 uint32_t VectorPlatformDeviceSkia::getDeviceCapabilities() {
   return SkDevice::getDeviceCapabilities() | kVector_Capability;
 }
@@ -190,6 +204,7 @@
   }
   pdf_device_->drawDevice(draw, real_device, x, y, paint);
 }
+#endif
 
 #if defined(OS_WIN)
 void VectorPlatformDeviceSkia::DrawToNativeContext(HDC dc,
@@ -211,6 +226,7 @@
 
 #endif
 
+#if 0
 SkDevice* VectorPlatformDeviceSkia::onCreateCompatibleDevice(
     SkBitmap::Config config, int width, int height, bool isOpaque, 
     Usage /*usage*/) {
@@ -219,5 +235,6 @@
                                                                  isOpaque));
   return new VectorPlatformDeviceSkia(static_cast<SkPDFDevice*>(dev.get()));
 }
+#endif
 
 }  // namespace skia