Draw lines on a picturebox using mouse clicks in C#

Question

I'm trying to make a program that will draw lines over a picturebox using mouse clicks for the locations of where the line is to be drawn from and to. This is my current code:

public partial class Form1 : Form
{
    int Drawshape;

    private Point p1, p2;
    List<Point> p1List = new List<Point>();
    List<Point> p2List = new List<Point>();

    public Form1()
    {
        InitializeComponent();
        pictureBox1.Image = new Bitmap(pictureBox1.Width, pictureBox1.Height);
    }

    private void button1_Click(object sender, EventArgs e)
    {
        Drawshape = 1;
    }

    private void button2_Click(object sender, EventArgs e)
    {
        Drawshape = 2;
    }

    private void pictureBox1_MouseDown(object sender, MouseEventArgs e)
    {
        if (Drawshape == 1)
        {
            if (p1.X == 0)
            {
                p1.X = e.X;
                p1.Y = e.Y;
            }
            else
            {
                p2.X = e.X;
                p2.Y = e.Y;

                p1List.Add(p1);
                p2List.Add(p2);

                Invalidate();
                p1.X = 0;
            }
        }
    }

    private void pictureBox1_Paint(object sender, PaintEventArgs e)
    {
        Graphics G = Graphics.FromImage(pictureBox1.Image);
        if (Drawshape == 1)
        {
            using (var p = new Pen(Color.Blue, 4))
            {
                for (int x = 0; x < p1List.Count; x++)
                {
                    G.DrawLine(p, p1List[x], p2List[x]);
                }
            }
        }
    }

At the moment it doesn't allow me to draw on the picturebox at all. How would that be possible?

Solution

Change Invalidate(); to pictureBox1.Invalidate();

Comments

[Chris Bacon]

Have i not done the graphics.fromimage in my code, or have i written that wrong? Also where do i need to dispose the graphics in my code?

[SLaks]

You should dispose the graphics after you finish with it. You shouldn't draw on the image in the paint event; you should draw it once in advance. You'll probably also need to Refresh() the PictureBox. If you want to do it your way, you should draw directly on a Panel (in the Paint event) without a PictureBox or Bitmap.

[Chris Bacon]

The problem is i need to do it on a picturebox, so i need to make it work on that. Also where exactly in my current code should i put the picturebox1.refresh() & picturebox1.dispose()? thanks

[SLaks]

Why do you need to use a PictureBox? Also, do not dispose the PictureBox.

[Paul Sasik]

You just need to use the Graphics object passed to you in the paint handler. You don't need to create or dispose of one created in the paint handler.

[Paul Sasik]

And you also need to create a mouse move handler and call pictureBox1.Invalidate(); so that your drawing feedback is immediate.

[Chris Bacon]

I need it in a picturebox because i have other code which uses the picturebox to draw other graphics on

[Dark Knight]

@ SLaks: am using Refresh() to redraw...but when mouse button goes up,the drawn line disappears...how to fix this?