[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 bugzilla-daemon at webkit.org
Thu Apr 14 16:56:40 PDT 2011

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

Attachment 89636: Patch

------- 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

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 *

Cast again.

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

And again.

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

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

More information about the webkit-reviews mailing list