Should I create new Pens/Brushes per Paint request

Question

I have an application that does a lot of drawing, let's pretend it's a Viso-like application. It has objects that have multiple sub-objects that are drawn, things can be connected, resized etc. Currently when I call paint on a particular sub-object or object, I do the following:

using(var pen = new Pen(this.ForeColor))
{
    // Paint for this object.
}

I've read conflicting answers that this should be done for an application that is constantly drawing the same thing (maybe just resized, moved etc). Should I store the Pen/Brush with the object and then dispose them all when the application is disposed of, or are they efficient enough to be created/disposed for each paint call (remembering that this is a pretty graphics intense application).

EDIT: There are already two answers that have conflicting answers and this is where I'm not sure to make the switch. Does anyone have any stats on the differences?

Answer 1

You could of course use the Pens and Brushes classes that provide you with objects that are already created by the runtime.

For example, if you want one of the standard colour Pens, you can do this:

var pen = Pens.Red;

Likewise you can do the same with Brushes, if you just want standard solid brush colours:

var brush = Brushes.Red

Using these, you don't need to worry about cleaning them up, disposing it or otherwise.

If you want different colours that you create yourself, for example with a different alpha component, or a gradient brush perhaps, then you still need to create these yourself and clean them up appropriately.

EDIT:

To create and dispose of an array of 100,000 new pens took approximately half a second on my ancient old XP machine, running a test app in Debug mode.

That equates to approximately 5 microseconds per pen. Only you can decide if that is fast enough for you. I would hazard a guess that this time may be largely insignificant with regard to the rest of your operations.

Answer 2

You'll only know the performance impact by testing on your particular application, but the framework doesn't seem to have a problem keeping a few pens around for the life of the application. The first time you call Pens.Black, it creates a black pen and caches it. You get the same object back for future calls, and it's never explicitly disposed (Pens.Black.Dispose() will actually throw an exception). However, you don't want to blindly create pens and leave them to be disposed when the application ends, because you'll be leaking unmanaged memory. A couple of options spring to mind depending on the pattern of usage in your app.

Give each object a private Pen that's created when ForeColor is set and reused for all painting. You should make your object IDisposable so that it can dispose of that Pen properly.

If you're using relatively few distinct colours but lots of objects use each colour, you may not want each object to hold on to its own Pen. Create some cache class that keeps a Dictionary<Color,Pen> and hands them out through a PenCache.GetPen(ForeColor). Like using Pens.Black, you can now forget about disposing them. A problem arises if you use a colour briefly, then don't need it again. The Pen got cached, so you're stuck with it in memory forever. You could keep a Dictionary<Color,WeakReference<Pen>> instead, allowing cached pens to be eventually garbage collected if they're no longer needed.

That last option may be the best general purpose solution, avoiding gratuitous creation and disposal while letting the garbage collector make sure that orphaned pens don't cause too much memory trouble. It may, of course, not be any better in your particular case.

Answer 3

Yeah best to test like @fantius suggests, you'll probably find it doesn't really matter - although I'd be temped to keep them open as if I recall correctly they're unmanaged resources and could use up your memory. If you were recreating them every time, you'd need to be especially careful of disposing them properly to avoid memory leaks.

Answer 4

Okay this is an old question, it is probably new for some and has a simple and intuitive solution followed by a detailed explanation.

"Should I hold on to paint resources, like pens and brushes, longer than what is required to preform my painting operation", question is rephrased? The answer is no, you should not in that very context; but why, what does that mean?

When you are painting to a graphics object you are using memory intensive resources for each paint object you create, be it a pen, brush, path, matrix and so on.

Yes, do create pens, create brushes etc. for painting and do myPen.dispose() and immediately following do release all object references to the disposed object by setting that object tree to null such as (myPen = null;) this allows the garbage collector to release unmanaged memory held by these objects without having to wait for the call to object finalize(). See: Garbage collection for more information about how garbage collection works in C#.

Creating too many of these IDisposable class objects and not releasing these objects when finished "Using" them can have serious consequences to your program's operation such as causing possible stack and heap memory exceptions, causing increased CPU usage by having to sort through unnecessary quantities of objects potentially causing slow performance and even a runtime crash if left unchecked. See about the Stack and Heap for more information.

Moral of the story release resources you no longer require; if you must retain resources, bench test the potential effects on performance when you "hold on to resources" hopefully avoiding negative consequences.

Rule of thumb: At the out most, ensure all resources are released upon exiting your "paint event" routine. Preferably, When each of your paint event methods finish so too should any resources implicitly created by that method. Penalty, any object references *passed to those methods like pens and brushes etc. will be held until base object finalize is called which is longer than necessary and can be considered a memory leak in the general terms of the definition. *Pass too many references equals unusable memory for a longer period than what probably is expected or assumed.

Note: Cautiously pass object references like pens and brushes to methods and if doing so implement an IDisposable interface at a class level to perform your painting. Your homework just got more expensive and recommend checking out the IDisposable interface.

Happy painting!

Answer 5

Reusing the same Pen and Brush in operations that will require it many times over is going to be a lot faster than disposing of them each time with using.

I would say for sure that it's a good idea to reuse these as much as possible (and I've seen code comparisons before, but I can't source them right now) but do remember to dispose of them when you are truly finished with them.