Changeset 123665 in webkit

Timestamp:

Jul 25, 2012, 2:47:21 PM (13 years ago)

Author:

reed@google.com

Message:

fix test in beginLayerClippedToImage to check for immutability if we're going to do a shallow-copy
​https://bugs.webkit.org/show_bug.cgi?id=92276

Reviewed by Stephen White.

PlatformContextSkia::beginLayerClippedToImage

This function wants to apply the provided ImageBuffer as a clip. Skia does not support this natively yet,
so the code makes a "copy" of that imageBuffer, to be applied later. The old code, wanting to avoid a
deep copy if possible, checked for the presence of a SkPixelRef. If it found one, it made a shallow copy.
This is flawed, since the contents of a pixelref are not guaranteed to be immutable. The new code checks
against this attribute: if the bitmap is "immutable" then we can make a shallow-copy, else we make a
deep copy.

No new tests. Existing svg layouttests work w/ or w/out this change, but at the next Skia deps roll, we see
failures w/o this change. The change is more "correct", though the problem case does not exhibit itself until
Skia rev. 4722 or later lands.

• platform/graphics/skia/PlatformContextSkia.cpp:

(WebCore::PlatformContextSkia::beginLayerClippedToImage):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/skia/PlatformContextSkia.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-25  Mike Reed  <reed@google.com>
+
+        fix test in beginLayerClippedToImage to check for immutability if we're going to do a shallow-copy
+        https://bugs.webkit.org/show_bug.cgi?id=92276
+
+        Reviewed by Stephen White.
+
+        PlatformContextSkia::beginLayerClippedToImage
+
+        This function wants to apply the provided ImageBuffer as a clip. Skia does not support this natively yet,
+        so the code makes a "copy" of that imageBuffer, to be applied later. The old code, wanting to avoid a
+        deep copy if possible, checked for the presence of a SkPixelRef. If it found one, it made a shallow copy.
+        This is flawed, since the contents of a pixelref are not guaranteed to be immutable. The new code checks
+        against this attribute: if the bitmap is "immutable" then we can make a shallow-copy, else we make a
+        deep copy.
+
+        No new tests. Existing svg layouttests work w/ or w/out this change, but at the next Skia deps roll, we see
+        failures w/o this change. The change is more "correct", though the problem case does not exhibit itself until
+        Skia rev. 4722 or later lands.
+
+        * platform/graphics/skia/PlatformContextSkia.cpp:
+        (WebCore::PlatformContextSkia::beginLayerClippedToImage):
+
 2012-07-25  Li Yin  <li.yin@intel.com>
 

trunk/Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp

@@ -285 +285 @@
 
     // Copy off the image as |imageBuffer| may be deleted before restore is invoked.
-    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.
+    if (bitmap->isImmutable())
+        m_state->m_imageBufferClip = *bitmap;
+    else {
+        // We need to make a deep-copy of the pixels themselves, so they don't
+        // change on us between now and when we want to apply them in restore()
         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;
     }
 }