Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

If I create a brush like so

HBRUSH hBrush = CreateSolidBrush(RGB(33, 33, 33));

and then later on, I want to change the color of the brush, how would I do this?
I googled a bit and found out that creating a bunch of different brushes is not the way to go.

I was thinking that one possibility would be to just set hBrush to a new brush like so

hBrush = CreateSolidBrush(RGB(0, 0, 0));

everytime I wanted to use a different color, but this seems suspiciously easy, and I'm pretty sure that this would hog resources or something.

Another - presumably bad - idea of mine, was to use the above method, but always use

DeleteObject(hBrush);

immediately after drawing. I have no idea if this is the right way to do things or not. Any help would be greatly appreciated.

share|improve this question
    
Who says creating a bunch of different brushes is not the way to go? You should avoid creating more brushes than needed, but you cannot change their colors once created so you will need to create at least as many as colors you want to use... –  K-ballo Jan 5 '13 at 18:34
add comment

2 Answers

up vote 1 down vote accepted

Assume any Win32 call that returns a handle allocates a struct with malloc. And assume DeleteObject (DestroyWindow etc) frees allocated memory. you should call DeleteObject every time you creare a GDI object.

Calling CreateSolidBrush twice, and losing the first handle without calling Deleteobject on it causes a resource leak.

Since you are using C++ the correct way to do it is RAII. see http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

This site has a good tutorial with clear windows programming examples on the subject. http://www.relisoft.com/book/win/index.htm

This is a good open source library (MIT licence), that you can use in your project directly. Or you can look at gdi.h in the source. http://win32-framework.sourceforge.net/index.htm

share|improve this answer
    
Would HBRUSH hBrush = CreateSolidBrush(RGB(0, 0, 0)); HBRUSH hBrOld = hBrush; hBrush = CreateSolidBrush(RGB(255, 0, 0)); DeleteObject(hBrOld); cause a resource leak? –  user19953 Jan 6 '13 at 7:30
    
Also, couldn't I just create one HBRUSH, and then everytime I wanted to change the color, I just called DeleteObject() and then CreateSolidBrush()? –  user19953 Jan 6 '13 at 14:46
    
Like I said, code in the first comment is similar to this: char * p; p = (char*) malloc(SIZE); char* pOld = p; p = (char*) malloc(SIZE); free(pOld); which is equivalent to: char * p; p = (char*) malloc(SIZE); // CreateSolidBrush free(p); //DeleteObject p = (char*) malloc(SIZE); –  abuzittin gillifirca Jan 6 '13 at 15:47
    
Well.. yeah.. It is.. but does that mean I can use that method..? Also, in the second commment, I'm just allocating and freeing the memory, so there shouldn't be any memory leaks, right? So the second method is safe..? –  user19953 Jan 7 '13 at 14:20
    
If you are writing C++ without exceptions disabled your code will not be safe. Just use plain C, then you can safely do C style resource management. –  abuzittin gillifirca Jan 7 '13 at 15:29
add comment

Create a new brush for each color that you need, and delete them once you are done.

A BrushCache class could be created. It could have this public function: HBRUSH GetColorBrush(COLORREF Color);

And it could have a private std::set, storing the Color as the Key and the brush handle as the value.

And also a destructor, calling DeleteObject for each of the brushes in the std::set.

So you would create an instance of the BrushCache class, and call GetColorBrush everytime you need a color brush. Ex for blue: GetColorBrush(RGB(0, 0, 255));

GetColorBrush would then first check if the set contains an item of Color. If yes, return it, else, create a new brush and store it in the set, then return it.

The BrushCache destructor would destroy all your brushes.

With this you could get a brush using one call, and not worry about creating / destroying them all.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.