Bug 147391 – Remove dispatch_apply_f and instead use vImage more directly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dean Jackson]

Remove dispatch_apply_f and instead use vImage more directly

[Dean Jackson]

Created attachment 257711 [details]
Patch

[Dean Jackson]

<rdar://problem/21893047>

[Simon Fraser (smfr)]

Comment on attachment 257711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257711&action=review

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:60
> +static void premultiplyBufferData(vImage_Buffer* src, vImage_Buffer* dest)

Could use const vImage_Buffer&, right?

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:65
> +    if (kvImageNoError != vImagePremultiplyData_RGBA8888(src, dest, kvImageDoNotTile))

Is there any reason to use kvImageDoNotTile, or should we just let vImage use threads if it wants to?

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:70
> +    vImagePermuteChannels_ARGB8888(dest, dest, map, kvImageDoNotTile);

Ditto.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:73
> +static void unpremultiplyBufferData(vImage_Buffer* src, vImage_Buffer* dest)

const vImage_Buffer&

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:83
> +    vImagePermuteChannels_ARGB8888(dest, dest, map, kvImageDoNotTile);

kvImageNoFlags?

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:270
> +            // The unpremultiplying and channel-swapping will be done in-place ooooo.

oooo?

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:420
>              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);
> +                vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
>                  // The premultiplying will be done in-place.
> -                src = dst;
> +                src = dest;
>              }
>  
> -            vImagePremultiplyData_RGBA8888(&src, &dst, kvImageNoFlags);
> +            vImagePremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags);

Odd that this code isn't in its own function.

[Dean Jackson]

(In reply to comment #3)
> Comment on attachment 257711 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257711&action=review
> 
> > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:60
> > +static void premultiplyBufferData(vImage_Buffer* src, vImage_Buffer* dest)
> 
> Could use const vImage_Buffer&, right?

Yes, fixed.

> 
> > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:65
> > +    if (kvImageNoError != vImagePremultiplyData_RGBA8888(src, dest, kvImageDoNotTile))
> 
> Is there any reason to use kvImageDoNotTile, or should we just let vImage
> use threads if it wants to?

Correct, we want kvImageNoFlags.

> 
> > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:420
> >              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);
> > +                vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
> >                  // The premultiplying will be done in-place.
> > -                src = dst;
> > +                src = dest;
> >              }
> >  
> > -            vImagePremultiplyData_RGBA8888(&src, &dst, kvImageNoFlags);
> > +            vImagePremultiplyData_RGBA8888(&src, &dest, kvImageNoFlags);
> 
> Odd that this code isn't in its own function.

Yeah, I made this:

static void affineWarpBufferData(const vImage_Buffer& src, const vImage_Buffer& dest, float scale)
{
    vImage_AffineTransform scaleTransform = { scale, 0, 0, scale, 0, 0 }; // FIXME: Add subpixel translation.
    Pixel_8888 backgroundColor;
    vImageAffineWarp_ARGB8888(&src, &dest, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend);
}

[Dean Jackson]

Committed r187534: <http://trac.webkit.org/changeset/187534>