views:

483

answers:

3

I'm trying to build a drawing app with redo and undo functionality. My idea is to draw lines in a layer in "touchMoved", then saving the layer in "touchEnded".

I'm not shure that I am drawing to the layer correct, everything works fine though, until I clear the image I'm drawing in on and try to redraw the layers in the array.

- (void)touchesMoved:(NSSet *)touches withEvent:(UIEvent *)event {

    UITouch *touch = [touches anyObject];   
    CGPoint currentPoint = [touch locationInView:self.view];

    UIGraphicsBeginImageContext(self.imageView.frame.size);
    [self.imageView.image drawInRect:self.imageView.frame];

    CGContextRef context = UIGraphicsGetCurrentContext();
    CGContextRef myContext;

    layerRef = CGLayerCreateWithContext(context, self.imageView.frame.size, NULL);

    if (self.layer == nil) {
        myContext =CGLayerGetContext(layerRef);

        CGContextSetLineCap(myContext, kCGLineCapRound);
        CGContextSetLineWidth(myContext, 5.0);
        CGContextSetLineJoin(myContext,  kCGLineJoinRound);
        CGContextSetRGBStrokeColor(myContext, 1.0, 0.0, 0.0, 1.0);

        CGContextBeginPath(myContext);
        CGContextMoveToPoint(myContext, lastPoint.x, lastPoint.y);
        CGContextAddLineToPoint(myContext, currentPoint.x, currentPoint.y);
        CGContextStrokePath(myContext);

        CGContextDrawLayerAtPoint(context, CGPointMake(00, 00),layerRef); 
        self.imageView.image = UIGraphicsGetImageFromCurrentImageContext();

                UIGraphicsEndImageContext();
        lastPoint = currentPoint;       
    }   
}

- (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event {
    if (self.layerArray != nil) {
        NSLog(@"Saving layer");
        [self.layerArray addObject:[[NSValue alloc] initWithBytes:layerRef objCType:@encode(CGLayerRef)]];
        CGLayerRelease(layerRef);
    }
    NSLog(@"%d",[layerArray count]);
}

Here is the method I'm trying to redraw the layer in. The app crashes when it reaches the the CGContextDrawLayerAtPoint()

- (IBAction)redrawViewButton:(id)sender {
    UIGraphicsBeginImageContext(self.imageView.frame.size);
    [self.imageView.image drawInRect:self.imageView.frame];

    NSValue *val = [layerArray objectAtIndex:0];
    CGLayerRef layerToShow;
    [val getValue:&layerToShow];    

    CGContextRef context = CGLayerGetContext(layerToShow);
    CGContextDrawLayerAtPoint(context, CGPointMake(00, 00),layerToShow);

    self.imageView.image = UIGraphicsGetImageFromCurrentImageContext();
    UIGraphicsEndImageContext();
}

+2  A: 

I take it that layerRef is an ivar that you've mapped to self.layer? You seem to be moving between accessors and direct ivar access, which is very confusing and error-prone. Make sure to always access your ivars through accessors. This will go a long way towards saving you memory management troubles. You'd implement the layerproperty something like this:

@property (nonatomic, readwrite, retain) CGLayerRef layer;

@synthesize layer = _layer;

- (void)setLayer:(CGLayer)aLayer
{
    CGLayerRetain(aLayer);
    CGLayerRelease(_layer);
    _layer = aLayer;
}

...

CGLayerRef layer = CGLayerCreateWithContext(context, self.imageView.frame.size, NULL);
self.layer = layer;
CGLayerRelease(layer);

The point of this is to put all of your memory management of the ivar inside of setLayer:. The most common cause of crashes on ivar access is that you have mismanaged the memory management on it. Accessors protect you from that.

A copule of other noteworthy points:

  • Never release something without immediately setting it to nil if it's staying in context. In your case you're releasing layerRef, but you don't clear the ivar. That means if you get touchesEnded: again before you get another touchesMoved:, you'll double-release the layer. That's probably the actual cause of your problem. Accessors protect you from this.

  • Your touchesMoved: code seems very wrong. You're creating a new layer every time you get a move. You can get dozens of touchesMoved: for a single touchesEnd:. Or you could get no touchesMoved: at all. I think you meant to put this code in touchesBegan:?

Rob Napier
+2  A: 

Some random things:

There is a memory leak in touchedEnded:withEvent:, you are adding a retained object to self.llayerArray but never release it after the array has also retained it. Try this instead:

- (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event {
    if (self.layerArray != nil) {
        NSLog(@"Saving layer");
        [self.layerArray addObject: [NSValue valueWithPointer: layerRef]];
        CGLayerRelease(layerRef);
    }
    NSLog(@"%d",[layerArray count]);
}

A CGLayerRef is a pointer. This means that in redrawViewButton: you can simpy do this:

CGLayerRef* layerToShow = (CGLayerRef) [[layerArray objectAtIndex: 0] pointerValue];
St3fan
+1  A: 

The simplest explanation is that either the layer or the context is not formed properly. You test both for nil before using. IIRC, the debugger can display values for Core Graphic structures if you use the "Print Description to Console" contextual menu.

Probably unrelated but I would recommend changing...

CGPointMake(00, 00)

...to:

CGPointMake(0.0f, 0.0f)

Just to make sure.

In any case, I think you need to abandon this method of implementing undo. It looks simple and neat but in reality it will grow cumbersome, complex and unreliable.

Undo and redo are properly functions of the data model and not a view or it's controller. Instead of saving the results of the user's inputs i.e. the drawings, you should be saving the users inputs and then drawing from that data.

In this case you store the points of the touches, the time/sequences of touches and whatever operations are relevant. The view and the view controller would have no "memory" at all. They would simply draw whatever the data model indicated needed to drawn at the moment. You would implement undo and redo in the data model. To undo you would draw all the data up to the undo point. To redo you draw up to the last data.

Core Data is very good for this although the learning curve is steep. It will implement undo and redo for you automatically. If your data model is relatively simple you could implement it with just an array that stores a custom class designed to store the data for a single drawing event.

If you try to do this all in the view or the view controller, you end up with a monster ball of fragile code.

TechZen