Command Pattern and OnPaint Event issue

I am trying to adapt Command Pattern to simple paint application with undo functionality. And I've stuck with OnPaint Event on Undo operations. This is the code:

[SOLVED] details at the end of the post

interface ICommand {
    void Execute();
    void UnExecute();
}

class DrawLineCommand : ICommand {
    private SimpleImage simpleImage;
    private Image prevImage;
    public DrawLineCommand(SimpleImage simpleImage) {
        this.simpleImage = simpleImage;
        this.prevImage = simpleImage.Image;
    }
    public void Execute() {
        simpleImage.DrawLine();
    }
    public void UnExecute() {
        simpleImage.Image = prevImage;
    }
}

class CommandManager {
    private Stack undoStack = new Stack();
    public void ExecuteCommand(ICommand command) {
        command.Execute();
        undoStack.Push(command);
    }
    public void UnExecuteCommand() {
        if (undoStack.Count > 0) {
            ICommand command = (ICommand)undoStack.Pop();
            command.UnExecute();
        }
    }
}

class SimpleImage {
    private Point startPoint;
    private Point endPoint;
    private PictureBox pictureBox;
    public SimpleImage(PictureBox pictureBox) {
        this.pictureBox = pictureBox;
        pictureBox.Paint += new PaintEventHandler(pictureBox_Paint);
    }
    void pictureBox_Paint(object sender, PaintEventArgs e) {
        // this code shows the line during drawing
        // this code is under "if operation == drawLine" block
        Graphics graphics = e.Graphics;
        graphics.DrawLine(Pens.Red, startPoint, endPoint);

        // how can i refresh picturebox after undo operation?
        // "if operation == undo" then ??
    }
    public void DrawLine() {
        // this code actually saves finally drawn line
        Image img = Image;
        Graphics graphics = Graphics.FromImage(img);
        graphics.DrawLine(Pens.Red, startPoint, endPoint);
        Image = img;
    }

    public void Invalidate() {
        pictureBox.Invalidate();
    }
    public Image Image {
        get { return pictureBox.Image; }
        set { pictureBox.Image = value; }
    }
    public Point StartPoint {
        get { return startPoint; }
        set { startPoint = value; }
    }
    public Point EndPoint {
        get { return endPoint; }
        set { endPoint = value; }
    }
}

public partial class FormMain : Form {
    private PictureBox pictureBox;
    private SimpleImage simpleImage;
    private CommandManager commandManager;
    public FormMain() {
        InitializeComponent();
        simpleImage = new SimpleImage(this.pictureBox);
        commandManager = new CommandManager();
    }
    void pictureBox_MouseDown(object sender, MouseEventArgs e) {
        if (e.Button != MouseButtons.Left)
            return;

        simpleImage.StartPoint = e.Locati开发者_Python百科on;
    }
    void pictureBox_MouseMove(object sender, MouseEventArgs e) {
        if (e.Button != MouseButtons.Left)
            return;

        simpleImage.EndPoint = e.Location;
        simpleImage.Invalidate();
    }
    void pictureBox_MouseUp(object sender, MouseEventArgs e) {
        simpleImage.Invalidate();
        commandManager.ExecuteCommand(new DrawLineCommand(simpleImage));
    }
}

It actually draws a line, it executes command and push it on the stack. I cannot achieve working UNDO. I mean. Step-by-step debugging I see the object pops from the stack, and then OnPaint executes. But no 'previous' image is actually shown.

I have read many sites, and I have also sample app from one of codeproject site's / articles. It presents the same approach with TextBox and Bold / Italicize operations. It works like hell. The only difference is this cruel OnPaint method..

Thanks in advance for any advices!

[EDIT] in a rush i forgot that assigning one reference type to another is not copying it (creating independent object), changing this:

this.prevImage = simpleImage.Image;

in few places solved the problem. Everything works now..

---

The point here would be not to paint directly on the canvas, but rather have a data structure that represents your painting. You would then add a line to this painting object, and the main loop of the canvas would draw the appropriate graphic from the data structure. Then your do/undo methods would simply need to manipulate the data structure, not do the painting.

You would need something like this:

interface IPaintable // intarface for Lines, Text, Circles, ...
{
    void OnPaint(Image i); // does the painting
}

interface IPaintableCommand // interface for commands 
{
    void Do(ICollection<IPaintable> painting); // adds line/text/circle to painting
    void Undo(ICollection<IPaintable> painting);  // removes line/text/circle from painting    
}

Your main application would simply keep a List, and repaint the canvas when a command changes the painting collection.

---

It looks like you have an aliasing problem. In your DrawLineCommand you pull in a reference to the image before the operation and store it as such:

this.prevImage = simpleImage.Image;

You now have two references to the same object. The draw line operation happens you operate on that same image:

Image img = Image; // Now a third reference to the same image object
Graphics graphics = Graphics.FromImage(img);
graphics.DrawLine(Pens.Red, startPoint, endPoint);
Image = img; // and you set the Image reference back to the same object

The above makes img an unnecessary reference to Image. But, you still have another reference to Image in your command. After the garbage collector runs, you're back down to to references to the same image object. Undo then executes the following:

simpleImage.Image = prevImage;

Here you haven't changed Image, only made Image reference the same object it was alreafy referencing.

Although I strongly agree with m0sa, the fix, in this case, is to make prevImage a COPY of the original image at the time you create your command. For the following I assume that Image.Clone() is implemented, though I've never tried it myself:

this.prevImage = simpleImage.Image.Clone();

NOTE: you may quickly run out of memory with large images or many commands if you use this approach.