Implement redo functionality for painting app

Question

0.00/5 (No votes)

See more:

So drawing on a picture box does not make the drawings disappear when the application window is minimized. I have implemented an Undo functionality for my drawing application where I have a stack that adds a new bitmap when a new drawing is made on the picture box. It works as each drawing step is undone by replacing the current bitmap displayed in the picture box with the previous bitmap held in the stack. I have no idea how to implement the Redo functionality, I had an idea to add the bitmaps that are being removed from the undo stack to a new redo stack and then peek when the user presses Ctrl + Y, the problem is that it only does that once and then it stops redoing. Check my code below for more details.

What I have tried:

C#

class MyForm{
    //stack object for holding the redo bitmap states
     private Stack<Bitmap> redoStates = new Stack<Bitmap>();
    //stack object for holding the undo bitmap states
    private Stack<Bitmap> paintStates = new Stack<Bitmap>();
    //ctor
    public MyForm(){
        //this form will receive keyboard events before any other control does
        KeyPreview = true;
        //override on key down event to listen for undo, redo, paste and copy events
        KeyDown += OnKeyDown;
    }
     private void OnKeyDown(object? sender, KeyEventArgs e)
    {

     //check if the user is trying to undo
      if (e.Control && e.KeyCode == Keys.Z)
        {
            //the user is trying to undo the last paint action
            if (paintStates.Count > 0)
            {
                //add this bitmap to the redo stack before removing it
                redoStates.Push(paintStates.Peek());
                //remove the last added bitmap
                paintStates.Pop();
                drawingPanel.Image = paintStates.Peek();
            }
        }
      //check if the user is trying to redo
       if(e.Control && e.KeyCode == Keys.Y)
        {
            //the user is trying to redo the paint events
            //get the object pushed out and display it
            if (redoStates.Count > 1)
            {
                //reverse the redo states
                drawingPanel.Image = redoStates.Peek();
            }
        }
    }
}

Posted 3-Apr-23 20:40pm

Tim the Gamer

Updated 3-Apr-23 21:13pm

Add a Solution

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

OriginalGriff · Answer 1 · 2023-04-03T21:13:00

Solution 1

Tim, don't use bitmaps as a "undo"/"redo" method: it's really, really inefficient in memory terms as you are storing potentially huge amounts of unchanging data!

Store the actions you take to move from one picture to the next: add those to a queue as you change things - by all means perform them on a bitmap for speed - but when the user uses undo, clear the bitmap and repeat all action up to that point.

Redo isn't difficult: when you undo just take the last action off the stack, and push it onto a second stack. Then redo is just a pop from the redo stack, and a push back onto the undo stack followed by a rebuild of your picture.
When the user adds a new action to the undo stack, clear the redo stack.

Posted 3-Apr-23 21:13pm

OriginalGriff

Comments

Tim the Gamer 4-Apr-23 3:58am

I define a data structure that will hold the drawing commands and then when the user performs a draw action such as drawing a line we add that command to the structure up to the point where he starts undoing, this makes sense. I will use this for better memory managdment as I also fear that the bitmap might make the program memeroy inefficient.

OriginalGriff 4-Apr-23 4:53am

Don't define a data structure that holds commands, define a data structure that is the command - then create a Stack<DrawCommand> of those to hold the "undo" list.

When you redraw the whole bitmap, use the Stack.ToArray Method[^] to get the commands in a "for-able" order (it returns them in pop order, so the latest action is at index 0)

Tim the Gamer 4-Apr-23 4:58am

Okay thanks let me implement that into the program.

Tim the Gamer 4-Apr-23 7:08am

This is clearing all the drawn steps at once without redrawing the items till the end of the stack

Tim the Gamer 4-Apr-23 7:15am

I created a DrawItem class with a constructor that accepts the item shape and list points of that item. Created a new Stack of the DrawItem objects and then pushed the shape type and the points whenever I draw a shape then in the undo I popped out the recently added command and looped through the remaining items to draw the items up to the current state of the stack but it removes all of them at once despite calling invalidate only once.

OriginalGriff 4-Apr-23 7:24am

:sigh:
What do you think Pop does?
That's why I suggested Stack.ToArray ...

using System;
using System.Collections.Generic;
public class Program
{
	public static void Main()
	{
		Stack<int> s = new Stack<int>();
		for (int i = 0; i < 10; i++)
		{
			s.Push(i);
		}
		int[] arr = s.ToArray();
		// Forward
		for (int i = arr.Length - 1; i >= 0; i--)
		{
			Console.WriteLine(arr[i]);
		}
		// Backward
		while (s.Count > 0)
		{
			Console.WriteLine(s.Pop());
		}
	}
}

OriginalGriff 4-Apr-23 7:53am

And how many times did you call Pop?

Tim the Gamer 4-Apr-23 7:48am

Pop removes the item at the top of the stack. So I should call it if I want to remove the last draw action performed. Then with the remaining events, wait let me check the code you provided for looping through the stack.

OriginalGriff 4-Apr-23 8:25am

Correct - but if you want to draw all the items, then you need to start with the first so if they overlap you end up with the same result.
Pop is not the method to use there, as it permanently removes the last item so you'd have to store them all up somewhere, then process them in the correct order while putting them back on the stack as you go. Hence ToArray: it returns all the items (in reverse order) without affecting the original stack.

Tim the Gamer 4-Apr-23 8:48am

I call Pop once to remove the last item before calling peek to show the current item at the top of the stack. It should not overlap as each draw command has the same shape ie Line with different List of Points. It should remove the command at the top of the stack and then redraw the rest but its not working, it removes all once.

OriginalGriff 4-Apr-23 9:21am

Show the code - use pre tags to preserve formatting:
<pre lang="cs">your code here
More code
</pre>

Tim the Gamer 4-Apr-23 9:51am

class MyForm{
  //class level declaration of the stack to hold the draw commands
  private Stack<DrawItem> drawCommands = new Stack<DrawItem>();
  //event handler when the user clicks two points on the picture box to draw a line
  private void DrawingPanelOnMouseClick(object? sender, MouseEventArgs e)
    {
        //increase the indexer if the app is listening
        if (listening)
        {
            indexer += 1;
        }
        //get the point clicked
        var point = new Point(e.X, e.Y);
        pointsClicked.Add(point);
        //add the point to a list
        if (indexer == 2 && drawItem == "Line")
        {
            //push object and points to stack
            drawCommands.Push(new DrawItem("Line",pointsClicked));
            //get the context and draw the line
            drawingPanel.CreateGraphics().DrawLine(drawingPen,pointsClicked[0],pointsClicked[1]);
            //reset the indexer
            indexer = 0;
            //clear the list of points clicked
            pointsClicked.Clear();
            
        }
     //draw as many lines as possible and then try undoing with this method
       private void OnKeyDown(object? sender, KeyEventArgs e)
    {
     if (e.Control && e.KeyCode == Keys.Z)
        {
            //the user is trying to undo the last paint action
            //if the user has performed only one draw command  clear the pane
            if (drawCommands.Count == 1)
            {
                SetPixel(drawingPanel);
            }
            if (drawCommands.Count > 1)
            {
                //push the command at the top of the stack to the redo command list before deleting it
                 redo.Push(drawCommands.Peek());
                //remove the command at the top of the stack
                drawCommands.Pop();
                //loop through the remaining commands and redraw them on the panel
                foreach(var command in drawCommands){
                  //get the command text
                  switch(command.Text){
                    case "Line":
                     drawingPanel.CreateGraphics().DrawLine(drawingPen,command.Points[0],command.Points[1]);
                     break;
                        }
                }
            }
        }
    }
}
//implementation of my drawitem class
public  class DrawItem
{
    private string _itemShape;
    private List<Point> shape = new List<Point>();
    //require the two inside the constructor
    public DrawItem(string itemShape, List<Point> points)
    {
        _itemShape = itemShape;
        shape = points;
    }
    //define the getters and setters for the properties
    public string Item
    {
        get
        {
            return _itemShape;
        }
        
    }

    public List<Point> Points
    {
        get
        {
            return shape;
        }
    }
}

OriginalGriff 4-Apr-23 10:09am

*cough*

drawCommands.Pop();
           ^
           |
...
foreach(var command in drawCommand){
                                 ^
                                 |

:cough:

Tim the Gamer 4-Apr-23 10:10am

They are typos, updating the code

Tim the Gamer 4-Apr-23 10:12am

The code should now compile.

OriginalGriff 4-Apr-23 10:35am

Don't show me code that "should compile" - show me code that runs and exhibits the problem. I.e. copy'n'paste the relevant code fragments, don't type up "stuff that's similar".
That's like taking an identical make and model car to the garage because yours won't start ...

Tim the Gamer 4-Apr-23 10:55am

The actual code in the Form1.cs file is very long and includes sections that I would consider irrelevant to this that is why I typed to produce a minimum reproducible example that you an replicate the issue with on your machine.

OriginalGriff 4-Apr-23 11:24am

Don't give us something you *think* will replicate the problem - give us something that *does* replicate the problem. If it doesn't compile, it's not tested - so it doesn't replicate the problem!

Just typing up code that doesn't work wastes your time and mine - yours more than mine unless you are a seriously fast typist ... :D

Tim the Gamer 4-Apr-23 11:27am

Okay I will paste the whole thing, I was just trying to spare you time you may spend going through the whole file looking for specific code

OriginalGriff 4-Apr-23 11:59am

No, post the relevant fragments, or sample code that shows the actual problem. The letter is better because getting that to fail may show you what the problem is!