2009-02-05 Scott Violet <sky@google.com>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Feb 2009 23:28:27 +0000 (23:28 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Feb 2009 23:28:27 +0000 (23:28 +0000)
        Reviewed by Eric Seidel.

        https://bugs.webkit.org/show_bug.cgi?id=23625
        Additional fix: Skia platform doesn't render text to a canvas or support clipping to an image buffer

        Fixes three bugs in PlatformContextSkia:

        * When a new layer was started clipped to an image we used the
          assignment operator to copy the SkBitmap. If the SkBitmap owns it's
          pixels, this is not the right thing to do. Instead we need to create
          a copy of the image.
        * State holds an SkBitmap by value. State's copy constructor does a
          memcpy. This is confusing and subtle, I've converted to use a member
          initializer list which I think is clearer and less error prone.
        * When creating a new layer there is no need to copy the clip image.

        * platform/graphics/skia/PlatformContextSkia.cpp:
        (PlatformContextSkia::State::State):
        (PlatformContextSkia::save):
        (PlatformContextSkia::beginLayerClippedToImage):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@40689 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/platform/graphics/skia/PlatformContextSkia.cpp

index 23d6699..99228e7 100644 (file)
@@ -1,5 +1,28 @@
 2009-02-05  Scott Violet  <sky@google.com>
 
+        Reviewed by Eric Seidel.
+
+        https://bugs.webkit.org/show_bug.cgi?id=23625
+        Additional fix: Skia platform doesn't render text to a canvas or support clipping to an image buffer
+
+        Fixes three bugs in PlatformContextSkia:
+
+        * When a new layer was started clipped to an image we used the
+          assignment operator to copy the SkBitmap. If the SkBitmap owns it's
+          pixels, this is not the right thing to do. Instead we need to create
+          a copy of the image.
+        * State holds an SkBitmap by value. State's copy constructor does a
+          memcpy. This is confusing and subtle, I've converted to use a member
+          initializer list which I think is clearer and less error prone.
+        * When creating a new layer there is no need to copy the clip image.
+
+        * platform/graphics/skia/PlatformContextSkia.cpp:
+        (PlatformContextSkia::State::State):
+        (PlatformContextSkia::save):
+        (PlatformContextSkia::beginLayerClippedToImage):
+
+2009-02-05  Scott Violet  <sky@google.com>
+
         Reviewed by George Staikos.
 
         https://bugs.webkit.org/show_bug.cgi?id=23462
index 8b12847..73256a2 100644 (file)
@@ -121,9 +121,28 @@ PlatformContextSkia::State::State()
 }
 
 PlatformContextSkia::State::State(const State& other)
+    : m_alpha(other.m_alpha)
+    , m_porterDuffMode(other.m_porterDuffMode)
+    , m_gradient(other.m_gradient)
+    , m_pattern(other.m_pattern)
+    , m_useAntialiasing(other.m_useAntialiasing)
+    , m_looper(other.m_looper)
+    , m_fillColor(other.m_fillColor)
+    , m_strokeStyle(other.m_strokeStyle)
+    , m_strokeColor(other.m_strokeColor)
+    , m_strokeThickness(other.m_strokeThickness)
+    , m_dashRatio(other.m_dashRatio)
+    , m_miterLimit(other.m_miterLimit)
+    , m_lineCap(other.m_lineCap)
+    , m_lineJoin(other.m_lineJoin)
+    , m_dash(other.m_dash)
+    , m_textDrawingMode(other.m_textDrawingMode)
+#if defined(__linux__) || PLATFORM(WIN_OS)
+    , m_imageBufferClip(other.m_imageBufferClip)
+    , m_clip(other.m_clip)
+#endif
 {
-    memcpy(this, &other, sizeof(State));
-
+    // Up the ref count of these. saveRef does nothing if 'this' is NULL.
     m_looper->safeRef();
     m_dash->safeRef();
     m_gradient->safeRef();
@@ -199,6 +218,12 @@ void PlatformContextSkia::save()
     m_stateStack.append(*m_state);
     m_state = &m_stateStack.last();
 
+#if defined(__linux__) || PLATFORM(WIN_OS)
+    // The clip image only needs to be applied once. Reset the image so that we
+    // don't attempt to clip multiple times.
+    m_state->m_imageBufferClip.reset();
+#endif
+
     // Save our native canvas.
     canvas()->save();
 }
@@ -217,9 +242,22 @@ void PlatformContextSkia::beginLayerClippedToImage(const WebCore::FloatRect& rec
     canvas()->saveLayerAlpha(&bounds, 255,
                              static_cast<SkCanvas::SaveFlags>(SkCanvas::kHasAlphaLayer_SaveFlag | SkCanvas::kFullColorLayer_SaveFlag));
     // Copy off the image as |imageBuffer| may be deleted before restore is invoked.
-    m_state->m_imageBufferClip = *(imageBuffer->context()->platformContext()->bitmap());
+    const SkBitmap* bitmap = imageBuffer->context()->platformContext()->bitmap();
+    if (!bitmap->pixelRef()) {
+        // The bitmap owns it's pixels. This happens when we've allocated the
+        // pixels in some way and assigned them directly to the bitmap (as
+        // happens when we allocate a DIB). In this case the assignment operator
+        // does not copy the pixels, rather the copied bitmap ends up
+        // referencing the same pixels. As the pixels may not live as long as we
+        // need it to, we copy the image.
+        bitmap->copyTo(&m_state->m_imageBufferClip, SkBitmap::kARGB_8888_Config);
+    } else {
+        // If there is a pixel ref, we can safely use the assignment operator.
+        m_state->m_imageBufferClip = *bitmap;
+    }
 }
 #endif
+
 void PlatformContextSkia::restore()
 {
 #if defined(__linux__) || PLATFORM(WIN_OS)