Blurring an Image

Lately, I've been part of a team working on a utility library for iOS called Thundercats (on Github). We're about to start working on some changes and upgrades in preparation for a 2.0 release, so we figured it might be a good time to get some of our code publicly reviewed.

The library is written in Objective-C and is set up to work with Cocoapods, but we want to make sure we the library works well from iOS 6.0 and up and with both Swift and Objective-C.

This code is from the UIImage extension. The method is used for blurring an image. It is exceptionally long. At 161 lines, it may be the longest method in the library. It's fairly complex and relatively lacking in comments, but that doesn't inherently make it unreadable.

It's important that the code is efficient, and I know that refactoring into other methods can sometimes decrease execution speed in Objective-C. Perhaps some of this could be refactored into C-style functions?
UIImage+TCAdditions.m

```

• (UIImage *)tc_imageWithBlurUsingRadius:(CGFloat)blurRadius

tintColor:(UIColor *)tintColor
saturationDeltaFactor:(CGFloat)saturationDeltaFactor
maskImage:(UIImage *)maskImage
{
// Check pre-conditions.
if (self.size.width = 1: %@", self.size.width, self.size.height, self);
return nil;
}
if (!self.CGImage)
{
NSLog (@"*** error: image must be backed by a CGImage: %@", self);
return nil;
}
if (maskImage && !maskImage.CGImage)
{
NSLog (@"*** error: maskImage must be backed by a CGImage: %@", maskImage);
return nil;
}

CGRect imageRect = { CGPointZero, self.size };
UIImage *effectImage = self;

BOOL hasBlur = blurRadius > __FLT_EPSILON__;
BOOL hasSaturationChange = fabs(saturationDeltaFactor - 1.) > __FLT_EPSILON__;

if (hasBlur || hasSaturationChange)
{
UIGraphicsBeginImageContextWithOptions(self.size, NO, [[UIS

I'd say that this code is pretty straightforward and I don't find this method to be so long I can't understand it. That said, I think you're right that it could be a little more readable if you broke it up.

Break Things Into Separate Functions

I'd be surprised if breaking this into some smaller methods caused much of a slowdown. The bulk of the time will be spent doing the box blurs, probably. You should profile it to be sure, of course.

So I'd break each separate filter into a separate method. Something like:

- (void)applyBlurWithRadius:(CGFloat)blurRadius
              toInputBuffer:(vImage_Buffer&)inputBuffer
           withOutputBuffer:(vImage_Buffer&)outputBuffer;

- (void)applySaturation:(CGFloat)saturationDelta
          toInputBuffer:(vImage_Buffer&)inputBuffer
       withOutputBuffer:(vImage_Buffer&)outputBuffer;

- (void)drawImage:(CGImageRef)image
         withMask:(CGImageRef)mask
        toContext:(CGContextRef)context;

- (void)applyTint:(UIColor*)tint
      toCGContext:(CGContextRef)context;

Don't Do Work You Don't Need To

I also notice that you're always drawing the image, then drawing over it. That's necessary when there's a mask, but if there's no mask and you're blurring the image, you're doing an extra draw. I'd add a check around the first call to CGContextDrawImage(), like this:

// Draw base image if there's a mask or we didn't blur it
if ((maskImage) || (!hasBlur))
{
    CGContextDrawImage(outputContext, imageRect, self.CGImage);
}

// Draw effect image.
if (hasBlur)
{
    CGContextSaveGState(outputContext);
    // ...etc.

Actually, does the current code work correctly if there's only a saturation change and no blur? It looks to me like it doesn't, but I might be misunderstanding something. (If that's the case that the (!hasBlur) should be ((!hasBlur) && (!hasSaturationChange)) and the next if should be if (hasBlur || hasSaturationChange).)

Why Use the CPU When You Can Use the GPU?

Is there a reason you're doing this work on the CPU? It seems like it would be more efficient to do it on the GPU using CoreImage. They have built-in filters for Gaussian blur, Color Matrix, compositing, etc.

StackExchange Code Review Q#97987, answer score: 4