CODEWITHSUNDEEP
c#winforms
Lorenzo Berendsen
Lorenzo Berendsen

Reputation: 33

Why does painting in a timer cause application to become unresponsive

I include Application.DoEvents() in 3 different places. Don´t know why this simple app is freezing. Any idea?

It runs very well in my VM running Visual Studio 2010 in Windows 7. I move it to a Visual Studio 2019 in my host machine running Windows 10. It runs very well, but as soon I click inside it - to close the windows - it hangs the Windows control. Mouse is blocked in Visual Studio region, and I have to open Task Manager and kill the process to close the app.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace Kaledoscópio
{
    public partial class frmMain : Form
    {
        public frmMain()
        {
            InitializeComponent();

            tmrStep.Tick += tmrStep_Tick;
            tmrStep.Start();
            Application.DoEvents();
        }

        private void tmrStep_Tick(object sender, EventArgs e)
        {
            Graphics g = frmMain.ActiveForm.CreateGraphics();

            Random randomColor = new Random();
            Color defaultColor = Color.FromArgb(randomColor.Next(255), randomColor.Next(255), randomColor.Next(255));
            Pen defaultPen = new Pen(defaultColor, 2);

            int intHeight = (int)g.VisibleClipBounds.Height;
            int intWidth = (int)g.VisibleClipBounds.Width;

            int intMiddleHeight = intHeight / 2;
            int intMiddleWidth = intWidth / 2;

            Point posTopLeft = new Point(0, 0);
            Point posTopRight = new Point(intWidth, 0);
            Point posBottomLeft = new Point(0, intHeight);
            Point posBottomRight = new Point(intWidth, intHeight);
            Point posMiddle = new Point(intMiddleWidth, intMiddleHeight);
            Point posMiddleTop = new Point(intMiddleWidth, 0);
            Point posMiddleLeft = new Point(0, intMiddleHeight);
            Point posMiddleRight = new Point(intWidth, intMiddleHeight);
            Point posMiddleBottom = new Point(intMiddleWidth, intHeight);

            int defaultStep = randomColor.Next(3, 10);

            int b1;
            for(b1 = 0; b1<=intMiddleWidth; b1+=defaultStep)
            {
                g.DrawLine(defaultPen, posTopLeft.X + b1, posTopLeft.Y, posMiddle.X - b1, posMiddle.Y);
                g.DrawLine(defaultPen, posTopRight.X - b1, posTopRight.Y, posMiddle.X + b1, posMiddle.Y);
                g.DrawLine(defaultPen, posMiddle.X - b1, posMiddle.Y, posBottomLeft.X + b1, posBottomLeft.Y);
                g.DrawLine(defaultPen, posMiddle.X + b1, posMiddle.Y, posBottomRight.X - b1, posBottomRight.Y);
                Application.DoEvents();
            }

            int b2;
            for( b2=0; b2<=intMiddleHeight; b2+=defaultStep)
            {
                g.DrawLine(defaultPen, posMiddleTop.X, posMiddleTop.Y + b2, posMiddleLeft.X, posMiddleLeft.Y - b2);
                g.DrawLine(defaultPen, posMiddleTop.X, posMiddleTop.Y + b2, posMiddleRight.X, posMiddleRight.Y - b2);
                g.DrawLine(defaultPen, posMiddleRight.X, posMiddleRight.Y + b2, posMiddleBottom.X, posMiddleBottom.Y - b2);
                g.DrawLine(defaultPen, posMiddleLeft.X, posMiddleLeft.Y + b2, posMiddleBottom.X, posMiddleBottom.Y - b2);
                Application.DoEvents();
            }

            Application.DoEvents();
        }
    }
}

Upvotes: 2

Views: 323

Answers (1)

Peter Duniho
Peter Duniho

Reputation: 70671

For what it's worth, I was able to reproduce the hanging behavior using the code you posted. The program did eventually respond to closing the window, but it took a very long time. I did not spend any time trying to figure out exactly what caused the program to hang, because the code you have now is so horribly incorrect, so far from the right way to use the Windows Forms API, that I didn't see any point.

Problems in the code you posted include:

  • Calling CreateGraphics(). A correctly written Winforms program should never need this.
  • Calling DoEvents(). Ditto.
  • Creating a new Random object every time you want to pick a new random number. See e.g. Random number generator only generating one random number
  • Failing to dispose the Pen object you create. This can lead to exhausting of GDI handles, which might be the cause of the apparent hang (and would explain why the program eventually does come back…if and when the finalizer thread catches up with all the undisposed Pen objects, the handles can be reclaimed and the program can proceed normally, for a little while longer until it runs out of handles again).
  • Using the static Form.ActiveForm property to get the form to draw into. This is wrong for at least two reasons: 1) it could draw into a window you don't intend to, and 2) there might not even be an active form, causing the property to return null, which then will lead to an exception.

All of the above can be solved easily, just by following the correct guidelines for writing Windows Forms programs:

  • Only ever draw in a Paint event handler or in the OnPaint() override. Call Invalidate() to cause the Paint event to be raised.
  • DoEvents() often is a crutch used by people who put long-running code into the UI thread, when that long-running code belongs in a different thread. Your code doesn't run long enough to block the UI thread for significant amounts of time, so it's not clear why that's even in your code. It can just be removed.
  • Create a single Random object once, and reuse it each time you want a new random number.
  • Dispose the Pen object. Use the using statement to do this safely and reliably.
  • Just use the current Form instance to decide what form to affect (in a correct version of the code, this means calling Invalidate() on the current instance, rather than calling CreateGraphics(), but the same principle applies regardless).

Here is a version of the code you posted that is correct. As a happy benefit, it also runs a lot faster. It can actually keep up with the 100ms interval that's the default for the System.Windows.Forms.Timer class.

private readonly Random randomColor = new Random();

public Form1()
{
    InitializeComponent();
    tmrStep.Tick += tmrStep_Tick;
    tmrStep.Start();
}

private void tmrStep_Tick(object sender, EventArgs e)
{
    Invalidate();
}

protected override void OnPaint(PaintEventArgs e)
{
    base.OnPaint(e);

    Graphics g = e.Graphics;

    Color defaultColor = Color.FromArgb(randomColor.Next(255), randomColor.Next(255), randomColor.Next(255));
    using (Pen defaultPen = new Pen(defaultColor, 2))
    {
        int intHeight = this.ClientSize.Height;
        int intWidth = this.ClientSize.Width;

        int intMiddleHeight = intHeight / 2;
        int intMiddleWidth = intWidth / 2;

        Point posTopLeft = new Point(0, 0);
        Point posTopRight = new Point(intWidth, 0);
        Point posBottomLeft = new Point(0, intHeight);
        Point posBottomRight = new Point(intWidth, intHeight);
        Point posMiddle = new Point(intMiddleWidth, intMiddleHeight);
        Point posMiddleTop = new Point(intMiddleWidth, 0);
        Point posMiddleLeft = new Point(0, intMiddleHeight);
        Point posMiddleRight = new Point(intWidth, intMiddleHeight);
        Point posMiddleBottom = new Point(intMiddleWidth, intHeight);

        int defaultStep = randomColor.Next(3, 10);

        int b1;
        for (b1 = 0; b1 <= intMiddleWidth; b1 += defaultStep)
        {
            g.DrawLine(defaultPen, posTopLeft.X + b1, posTopLeft.Y, posMiddle.X - b1, posMiddle.Y);
            g.DrawLine(defaultPen, posTopRight.X - b1, posTopRight.Y, posMiddle.X + b1, posMiddle.Y);
            g.DrawLine(defaultPen, posMiddle.X - b1, posMiddle.Y, posBottomLeft.X + b1, posBottomLeft.Y);
            g.DrawLine(defaultPen, posMiddle.X + b1, posMiddle.Y, posBottomRight.X - b1, posBottomRight.Y);
        }

        int b2;
        for (b2 = 0; b2 <= intMiddleHeight; b2 += defaultStep)
        {
            g.DrawLine(defaultPen, posMiddleTop.X, posMiddleTop.Y + b2, posMiddleLeft.X, posMiddleLeft.Y - b2);
            g.DrawLine(defaultPen, posMiddleTop.X, posMiddleTop.Y + b2, posMiddleRight.X, posMiddleRight.Y - b2);
            g.DrawLine(defaultPen, posMiddleRight.X, posMiddleRight.Y + b2, posMiddleBottom.X, posMiddleBottom.Y - b2);
            g.DrawLine(defaultPen, posMiddleLeft.X, posMiddleLeft.Y + b2, posMiddleBottom.X, posMiddleBottom.Y - b2);
        }
    }
}

Upvotes: 4

Related Questions