[webkit-reviews] review denied: [Bug 58088] [CG] Use vImage (un)premultiplyImageData functions for get/putImageData with IOSurfaces : [Attachment 89636] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Matthew Delaney <mdelaney at apple.com>'s
request for review:
Bug 58088: [CG] Use vImage (un)premultiplyImageData functions for
get/putImageData with IOSurfaces
https://bugs.webkit.org/show_bug.cgi?id=58088

Attachment 89636: Patch
https://bugs.webkit.org/attachment.cgi?id=89636&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89636&action=review

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:44
> +typedef struct vImageBufferPair {
> +    vImage_Buffer src;
> +    vImage_Buffer dest;
> +} vImageBufferPair;

This is c++, so the typedef is not necessary.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:66
> +#if USE(IOSURFACE_CANVAS_BACKING_STORE)

Is this optimization only worth while for IOSurface backed canvas, or would
anyone with vImage want it? Maybe it should be in the USE(ACCELLERATE).

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:69
> +    vImageBufferPair* bufferPair = (vImageBufferPair*) data;

Please use c++ style casts and don't put a space after the cast.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:72
> +    src.data = (char*) bufferPair->src.data + tileNumber *
bufferPair->src.rowBytes;

Cast again.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:78
> +    dest.data = (char*) bufferPair->dest.data + tileNumber *
bufferPair->dest.rowBytes;

And again.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:216
> +	       dispatch_apply_f(height,
dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), &buffPair,
unpremultitplyScanline);

Why is that queue the right queue to use?  Will it be a concurrent queue?