Title: [187534] trunk/Source/WebCore
Revision
187534
Author
[email protected]
Date
2015-07-28 18:43:58 -0700 (Tue, 28 Jul 2015)

Log Message

Remove dispatch_apply_f and instead use vImage more directly
https://bugs.webkit.org/show_bug.cgi?id=147391
<rdar://problem/21893047>

Reviewed by Simon Fraser.

Use vImage unmultiplication and premultiplication functions on the
entire ImageBufferData object, rather than getting and setting data on an
line by line using dispatch_apply.

We were seeing some crashes in vImage with the smaller buffer sizes, and
hopefully this will either fix the problem, or give us a better
stack trace to diagnose.

I also did a drive-by change of "dst" to "dest". It was inconsistent throughout
the file.

Convered by the tests in fast/canvas and imported/w3c/canvas

* platform/graphics/cg/ImageBufferDataCG.cpp: Remove the ScanlineData structure. It is
no longer needed.
(WebCore::premultiplyBufferData): New function that calls vImagePremultiplyData_RGBA8888.
(WebCore::unpremultiplyBufferData): New function that calls vImageUnpremultiplyData_RGBA8888.
(WebCore::affineWarpBufferData): Extracting some common code into a function.
(WebCore::ImageBufferData::getData): Use the two new functions as appropriate. Move
some of the code around now that more is shared between the different #if branches.
(WebCore::ImageBufferData::putData):
(WebCore::convertScanline): Deleted.
(WebCore::unpremultitplyScanline): Deleted.
(WebCore::premultitplyScanline): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (187533 => 187534)


--- trunk/Source/WebCore/ChangeLog	2015-07-29 01:26:49 UTC (rev 187533)
+++ trunk/Source/WebCore/ChangeLog	2015-07-29 01:43:58 UTC (rev 187534)
@@ -1,3 +1,36 @@
+2015-07-28  Dean Jackson  <[email protected]>
+
+        Remove dispatch_apply_f and instead use vImage more directly
+        https://bugs.webkit.org/show_bug.cgi?id=147391
+        <rdar://problem/21893047>
+
+        Reviewed by Simon Fraser.
+
+        Use vImage unmultiplication and premultiplication functions on the
+        entire ImageBufferData object, rather than getting and setting data on an
+        line by line using dispatch_apply.
+
+        We were seeing some crashes in vImage with the smaller buffer sizes, and
+        hopefully this will either fix the problem, or give us a better
+        stack trace to diagnose.
+
+        I also did a drive-by change of "dst" to "dest". It was inconsistent throughout
+        the file.
+
+        Convered by the tests in fast/canvas and imported/w3c/canvas
+
+        * platform/graphics/cg/ImageBufferDataCG.cpp: Remove the ScanlineData structure. It is
+        no longer needed.
+        (WebCore::premultiplyBufferData): New function that calls vImagePremultiplyData_RGBA8888.
+        (WebCore::unpremultiplyBufferData): New function that calls vImageUnpremultiplyData_RGBA8888.
+        (WebCore::affineWarpBufferData): Extracting some common code into a function.
+        (WebCore::ImageBufferData::getData): Use the two new functions as appropriate. Move
+        some of the code around now that more is shared between the different #if branches.
+        (WebCore::ImageBufferData::putData):
+        (WebCore::convertScanline): Deleted.
+        (WebCore::unpremultitplyScanline): Deleted.
+        (WebCore::premultitplyScanline): Deleted.
+
 2015-07-28  Alexey Proskuryakov  <[email protected]>
 
         Clean up usesAsyncCallbacks handling in ResourceHandle

Modified: trunk/Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp (187533 => 187534)


--- trunk/Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp	2015-07-29 01:26:49 UTC (rev 187533)
+++ trunk/Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp	2015-07-29 01:43:58 UTC (rev 187534)
@@ -42,16 +42,6 @@
 #include <dispatch/dispatch.h>
 #endif
 
-#if USE(ACCELERATE)
-struct ScanlineData {
-    vImagePixelCount scanlineWidth;
-    unsigned char* srcData;
-    size_t srcRowBytes;
-    unsigned char* destData;
-    size_t destRowBytes;
-};
-#endif
-
 // CA uses ARGB32 for textures and ARGB32 -> ARGB32 resampling is optimized.
 #define USE_ARGB32 PLATFORM(IOS)
 
@@ -66,47 +56,41 @@
 }
 
 #if USE(ACCELERATE)
-
 #if USE_ARGB32 || USE(IOSURFACE_CANVAS_BACKING_STORE)
-static void convertScanline(void* data, size_t tileNumber, bool premultiply)
+static void premultiplyBufferData(const vImage_Buffer& src, const vImage_Buffer& dest)
 {
-    ScanlineData* scanlineData = static_cast<ScanlineData*>(data);
+    ASSERT(src->data);
+    ASSERT(dest->data);
 
-    vImage_Buffer src;
-    src.data = "" + tileNumber * scanlineData->srcRowBytes;
-    src.height = 1;
-    src.width = scanlineData->scanlineWidth;
-    src.rowBytes = scanlineData->srcRowBytes;
+    if (kvImageNoError != vImagePremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags))
+        return;
 
-    vImage_Buffer dest;
-    dest.data = "" + tileNumber * scanlineData->destRowBytes;
-    dest.height = 1;
-    dest.width = scanlineData->scanlineWidth;
-    dest.rowBytes = scanlineData->destRowBytes;
-
-    if (premultiply) {
-        if (kvImageNoError != vImagePremultiplyData_RGBA8888(&src, &dest, kvImageDoNotTile))
-            return;
-    } else {
-        if (kvImageNoError != vImageUnpremultiplyData_RGBA8888(&src, &dest, kvImageDoNotTile))
-            return;
-    }
-
     // Swap channels 1 and 3, to convert BGRA<->RGBA. IOSurfaces are BGRA, ImageData expects RGBA.
     const uint8_t map[4] = { 2, 1, 0, 3 };
-    vImagePermuteChannels_ARGB8888(&dest, &dest, map, kvImageDoNotTile);
+    vImagePermuteChannels_ARGB8888(&dest, &dest, map, kvImageNoFlags);
 }
 
-static void unpremultitplyScanline(void* data, size_t tileNumber)
+static void unpremultiplyBufferData(const vImage_Buffer& src, const vImage_Buffer& dest)
 {
-    convertScanline(data, tileNumber, false);
+    ASSERT(src->data);
+    ASSERT(dest->data);
+
+    if (kvImageNoError != vImageUnpremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags))
+        return;
+
+    // Swap channels 1 and 3, to convert BGRA<->RGBA. IOSurfaces are BGRA, ImageData expects RGBA.
+    const uint8_t map[4] = { 2, 1, 0, 3 };
+    vImagePermuteChannels_ARGB8888(&dest, &dest, map, kvImageNoFlags);
 }
+#endif // USE_ARGB32 || USE(IOSURFACE_CANVAS_BACKING_STORE)
 
-static void premultitplyScanline(void* data, size_t tileNumber)
+static void affineWarpBufferData(const vImage_Buffer& src, const vImage_Buffer& dest, float scale)
 {
-    convertScanline(data, tileNumber, true);
+    vImage_AffineTransform scaleTransform = { scale, 0, 0, scale, 0, 0 }; // FIXME: Add subpixel translation.
+    Pixel_8888 backgroundColor;
+    vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
 }
-#endif // USE(IOSURFACE_CANVAS_BACKING_STORE)
+
 #endif // USE(ACCELERATE)
 
 RefPtr<Uint8ClampedArray> ImageBufferData::getData(const IntRect& rect, const IntSize& size, bool accelerateRendering, bool unmultiplied, float resolutionScale) const
@@ -167,41 +151,34 @@
     if (!accelerateRendering) {
         srcBytesPerRow = bytesPerRow.unsafeGet();
         srcRows = reinterpret_cast<unsigned char*>(data) + originy * srcBytesPerRow + originx * 4;
-        
+
 #if USE(ACCELERATE)
         if (unmultiplied) {
-#if USE_ARGB32
-            ScanlineData scanlineData;
-            scanlineData.scanlineWidth = destw.unsafeGet();
-            scanlineData.srcData = srcRows;
-            scanlineData.srcRowBytes = srcBytesPerRow;
-            scanlineData.destData = destRows;
-            scanlineData.destRowBytes = destBytesPerRow;
 
-            dispatch_apply_f(desth.unsafeGet(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), &scanlineData, unpremultitplyScanline);
-#else
             vImage_Buffer src;
-            src.height = height.unsafeGet();
             src.width = width.unsafeGet();
+            src.height = height.unsafeGet();
             src.rowBytes = srcBytesPerRow;
             src.data = ""
-            
-            vImage_Buffer dst;
-            dst.height = desth.unsafeGet();
-            dst.width = destw.unsafeGet();
-            dst.rowBytes = destBytesPerRow;
-            dst.data = ""
 
+            vImage_Buffer dest;
+            dest.width = destw.unsafeGet();
+            dest.height = desth.unsafeGet();
+            dest.rowBytes = destBytesPerRow;
+            dest.data = ""
+
+#if USE_ARGB32
+            unpremultiplyBufferData(src, dest);
+#else
             if (resolutionScale != 1) {
-                vImage_AffineTransform scaleTransform = { 1 / resolutionScale, 0, 0, 1 / resolutionScale, 0, 0 }; // FIXME: Add subpixel translation.
-                Pixel_8888 backgroundColor;
-                vImageAffineWarp_ARGB8888(&src, &dst, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
+                affineWarpBufferData(src, dest, 1 / resolutionScale);
                 // The unpremultiplying will be done in-place.
-                src = ""
+                src = ""
             }
 
-            vImageUnpremultiplyData_RGBA8888(&src, &dst, kvImageNoFlags);
+            vImageUnpremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags);
 #endif
+
             return result;
         }
 #endif
@@ -279,42 +256,28 @@
         srcRows = static_cast<unsigned char*>(IOSurfaceGetBaseAddress(surfaceRef)) + originy * srcBytesPerRow + originx * 4;
 
 #if USE(ACCELERATE)
+
         vImage_Buffer src;
-        src.height = height.unsafeGet();
         src.width = width.unsafeGet();
+        src.height = height.unsafeGet();
         src.rowBytes = srcBytesPerRow;
         src.data = ""
 
         vImage_Buffer dest;
-        dest.height = desth.unsafeGet();
         dest.width = destw.unsafeGet();
+        dest.height = desth.unsafeGet();
         dest.rowBytes = destBytesPerRow;
         dest.data = ""
 
         if (resolutionScale != 1) {
-            vImage_AffineTransform scaleTransform = { 1 / resolutionScale, 0, 0, 1 / resolutionScale, 0, 0 }; // FIXME: Add subpixel translation.
-            Pixel_8888 backgroundColor;
-            vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
+            affineWarpBufferData(src, dest, 1 / resolutionScale);
             // The unpremultiplying and channel-swapping will be done in-place.
-            if (unmultiplied) {
-                srcRows = destRows;
-                width = destw;
-                height = desth;
-                srcBytesPerRow = destBytesPerRow;
-            } else
-                src = ""
+            src = ""
         }
 
-        if (unmultiplied) {
-            ScanlineData scanlineData;
-            scanlineData.scanlineWidth = destw.unsafeGet();
-            scanlineData.srcData = srcRows;
-            scanlineData.srcRowBytes = srcBytesPerRow;
-            scanlineData.destData = destRows;
-            scanlineData.destRowBytes = destBytesPerRow;
-
-            dispatch_apply_f(desth.unsafeGet(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), &scanlineData, unpremultitplyScanline);
-        } else {
+        if (unmultiplied)
+            unpremultiplyBufferData(src, dest);
+        else {
             // Swap pixel channels from BGRA to RGBA.
             const uint8_t map[4] = { 2, 1, 0, 3 };
             vImagePermuteChannels_ARGB8888(&src, &dest, map, kvImageNoFlags);
@@ -431,41 +394,32 @@
     if (!accelerateRendering) {
         destBytesPerRow = bytesPerRow.unsafeGet();
         destRows = reinterpret_cast<unsigned char*>(data) + (desty * destBytesPerRow + destx * 4).unsafeGet();
-        
-#if  USE(ACCELERATE)
+
+#if USE(ACCELERATE)
         if (unmultiplied) {
-#if USE_ARGB32
-            // FIXME: Are scanlineData.scanlineWidth and the number of iterations specified to dispatch_apply_f() correct?
-            ScanlineData scanlineData;
-            scanlineData.scanlineWidth = width.unsafeGet();
-            scanlineData.srcData = srcRows;
-            scanlineData.srcRowBytes = srcBytesPerRow;
-            scanlineData.destData = destRows;
-            scanlineData.destRowBytes = destBytesPerRow;
 
-            dispatch_apply_f(height.unsafeGet(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), &scanlineData, premultitplyScanline);
-#else
             vImage_Buffer src;
-            src.height = height.unsafeGet();
             src.width = width.unsafeGet();
+            src.height = height.unsafeGet();
             src.rowBytes = srcBytesPerRow;
             src.data = ""
-            
-            vImage_Buffer dst;
-            dst.height = desth.unsafeGet();
-            dst.width = destw.unsafeGet();
-            dst.rowBytes = destBytesPerRow;
-            dst.data = ""
 
+            vImage_Buffer dest;
+            dest.width = destw.unsafeGet();
+            dest.height = desth.unsafeGet();
+            dest.rowBytes = destBytesPerRow;
+            dest.data = ""
+
+#if USE_ARGB32
+            unpremultiplyBufferData(src, dest);
+#else
             if (resolutionScale != 1) {
-                vImage_AffineTransform scaleTransform = { resolutionScale, 0, 0, resolutionScale, 0, 0 }; // FIXME: Add subpixel translation.
-                Pixel_8888 backgroundColor;
-                vImageAffineWarp_ARGB8888(&src, &dst, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
+                affineWarpBufferData(src, dest, resolutionScale);
                 // The premultiplying will be done in-place.
-                src = ""
+                src = ""
             }
 
-            vImagePremultiplyData_RGBA8888(&src, &dst, kvImageNoFlags);
+            vImagePremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags);
 #endif
             return;
         }
@@ -479,6 +433,7 @@
             if (!unmultiplied)
                 return;
 
+            // The premultiplying will be done in-place.
             srcRows = destRows;
             srcBytesPerRow = destBytesPerRow;
             width = destw;
@@ -524,41 +479,26 @@
 
 #if USE(ACCELERATE)
         vImage_Buffer src;
-        src.height = height.unsafeGet();
         src.width = width.unsafeGet();
+        src.height = height.unsafeGet();
         src.rowBytes = srcBytesPerRow;
         src.data = ""
 
         vImage_Buffer dest;
-        dest.height = desth.unsafeGet();
         dest.width = destw.unsafeGet();
+        dest.height = desth.unsafeGet();
         dest.rowBytes = destBytesPerRow;
         dest.data = ""
 
         if (resolutionScale != 1) {
-            vImage_AffineTransform scaleTransform = { resolutionScale, 0, 0, resolutionScale, 0, 0 }; // FIXME: Add subpixel translation.
-            Pixel_8888 backgroundColor;
-            vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
+            affineWarpBufferData(src, dest, resolutionScale);
             // The unpremultiplying and channel-swapping will be done in-place.
-            if (unmultiplied) {
-                srcRows = destRows;
-                width = destw;
-                height = desth;
-                srcBytesPerRow = destBytesPerRow;
-            } else
-                src = ""
+            src = ""
         }
 
-        if (unmultiplied) {
-            ScanlineData scanlineData;
-            scanlineData.scanlineWidth = width.unsafeGet();
-            scanlineData.srcData = srcRows;
-            scanlineData.srcRowBytes = srcBytesPerRow;
-            scanlineData.destData = destRows;
-            scanlineData.destRowBytes = destBytesPerRow;
-
-            dispatch_apply_f(height.unsafeGet(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), &scanlineData, premultitplyScanline);
-        } else {
+        if (unmultiplied)
+            premultiplyBufferData(src, dest);
+        else {
             // Swap pixel channels from RGBA to BGRA.
             const uint8_t map[4] = { 2, 1, 0, 3 };
             vImagePermuteChannels_ARGB8888(&src, &dest, map, kvImageNoFlags);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to