Strange Problem

My goal is to bring up the Color Picker dialog and redraw the rectangle to the newly selected color. When I run this it will not work. However, if I set a breakpoint at the DrawRectangle function and step throught the code, it redraws the rectangle like it should.

With my limited knowledge of Win32 API/GDI I can't figure out why it would work stepping through the code and not work if I just execute it.

Here is the code:

Code:

LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{
    // Color picker dialog variables
    CHOOSECOLOR cc;
    static COLORREF customColor[16];
    HBRUSH hbrush;
    static DWORD rgbCurrent = RGB(255, 0, 0);

    // Initialize CHOOSECOLOR
    ZeroMemory(&cc, sizeof(cc));
    cc.lStructSize = sizeof(cc);
    cc.hwndOwner = hwnd;
    cc.lpCustColors = (LPDWORD) customColor;
    cc.rgbResult = rgbCurrent;
    cc.Flags = CC_FULLOPEN | CC_RGBINIT;

    switch (message)                  /* handle the messages */
    {

        case WM_PAINT:
            DrawRectangle(hwnd, rgbCurrent);
        break;

        case WM_COMMAND:
            switch(wParam)
            {
                case IDM_COLOR_PICK:
                    if (ChooseColor(&cc)==TRUE) {
                        hbrush = CreateSolidBrush(cc.rgbResult);
                        rgbCurrent = cc.rgbResult;
                        DrawRectangle(hwnd, rgbCurrent);
                    }
                break;
            }
        break;

        case WM_DESTROY:
            PostQuitMessage (0);       /* send a WM_QUIT to the message queue */
            break;
        default:                      /* for messages that we don't deal with */
            return DefWindowProc (hwnd, message, wParam, lParam);
    }

    return 0;
}

void DrawRectangle(HWND hwnd, DWORD rgbCurrent)
{
    // Varialbes for rectangle
    HDC hdc;
    PAINTSTRUCT ps;
    HBRUSH rectBrush;

    // color rectangle
    hdc = BeginPaint(hwnd, &ps);
    rectBrush = CreateSolidBrush(rgbCurrent);

    SelectObject(hdc, rectBrush);
    Rectangle(hdc, 20, 20, 100, 100);
    DeleteObject(rectBrush);

    EndPaint(hwnd, &ps);
}

As a guess, the DefWindowProc might be redrawing the entire window after you call DrawRectangle. Try calling ValidateRect for that rect.

Edit: If all you are doing is drawing the rectangle, I'm not sure you even need to call BeginPaint and EndPaint (maybe just use GetDC/ReleaseDC)

GetDC and ReleaseDC made the program freeze. (Assuming I used it properly)

Also I am not sure how ValidateRect is supposed to be used but I will look into it.

I just tried this again and here is exactly what happens:
- I click Debug, run the program, pick a color, and the color doesn't change.
- I put a breakpoint at the DrawRectangle function, click Debug, run through, pick a color, here the breakpoint catches, I click continure and the color has changed.

Why would a breakpoint make it work!?

Using the example from MSDN? I made a sample program following, close but not exact, the MSDN example. I found that when I use this line:

Code:

ZeroMemory(&cc, sizeof(cc));

It comes up with an error at run-time and says it must quit. If I take it out then it works as it should. I checked and that line of code is EXACTLY used as MSDN uses it, yet it causes an error? I'm using Dev-C++, are you Vicious?

EDIT:
If it is clearing the data in the address of cc, &cc, then why wouldn't we use the sizeof the address of cc, like so:

Code:

ZeroMemory(&cc,sizeof(&cc));

That line of code works without run-time errors. Is that an improper way to do it, it makes sense if we are clearing the memory of &cc, that we should use the sizeof that same element, sizeof(&cc)?

I use Code Blocks. It's a fairly new IDE.

That is the example off of MSDN. The dialog itself works but it just doesn't update the color of the rectangle. (Unless I put a breakpoint in)

I believe sizeof(&cc) would be the size of the address. (32 bit?)

Although, I wonder it there would be any difference with sizeof(CHOOSECOLOR)

Well, I have no idea why I'm getting an error at run-time with that ZeroMemory(&cc,sizeof(cc)); I mean I have used that function many many times, the same way too.

Oh well the reason the color isn't getting updated is because it is resetting the value each time the message loop runs through. See the WndProc loop? You are setting the rgbCurrent value to red every time your program runs through this loop. You need to set the initial color under WM_CREATE:. So something like this:

Code:

switch(msg)
{
     case WM_CREATE:
         rgbCurrent=RGB(255,0,0);
         return 0;
//All your other case statements
}

I didn't really look carefully before, but I notice now that you are calling DrawRectangle in the WM_COMMAND case. BeginPaint is only supposed to be called in response to a WM_PAINT command. Also, you have a GDI leak here:

Code:

   case IDM_COLOR_PICK:
                    if (ChooseColor(&cc)==TRUE) {
                        hbrush = CreateSolidBrush(cc.rgbResult);
                        rgbCurrent = cc.rgbResult;
                        DrawRectangle(hwnd, rgbCurrent);
                    }
                break;

And you are supposed to select the original object into memory after you are done drawing:

Code:

HBRUSH oldbrush = (HBRUSH)SelectObject(hdc, rectBrush);
//...
SelectObject(hdc,oldbrush);

Don't know if any of that will solve your problem, though

Originally Posted by jmd15

Oh well the reason the color isn't getting updated is because it is resetting the value each time the message loop runs through. See the WndProc loop? You are setting the rgbCurrent value to red every time your program runs through this loop. You need to set the initial color under WM_CREATE:. So something like this:

Code:

switch(msg)
{
     case WM_CREATE:
         rgbCurrent=RGB(255,0,0);
         return 0;
//All your other case statements
}

No. The variable in question is static. That means it's only initialized once. First time through WndProc.

@OP: Why do you assign the result from CreateSolidBrush to hBrush under case IDM_COLOR_PICK: when you dont even use it during that event? You're also not being a good boy with your resources. Free/Delete hbrush before exiting the Window Procedure - believe it or not, that could eventually be your problem.

[edit: OP: Try putting InvalidateRect(hwnd, NULL, TRUE) right before BeginPaint.]

Originally Posted by xeddiex

[edit: OP: Try putting InvalidateRect(hwnd, NULL, TRUE) right before BeginPaint.]

This is correct. Change:

Code:

                    if (ChooseColor(&cc)==TRUE) {
                        hbrush = CreateSolidBrush(cc.rgbResult);
                        rgbCurrent = cc.rgbResult;
                        DrawRectangle(hwnd, rgbCurrent);
                    }

to:

Code:

                    if (ChooseColor(&cc)==TRUE) {
                        rgbCurrent = cc.rgbResult;
                        InvalidateRect(hwnd, NULL, FALSE);
                    }

Windows will not update areas that are not invalid. By calling InvalidateRect, you mark the window as invalid and windows will post a WM_PAINT message.

When setting a breakpoint, you are seeing a side-effect of interacting with the debugger. When the debugger comes up, it will cover your window. This invalidates the window and when it is visible again Windows will send a WM_PAINT to repaint it.

Question: Why should you be extremely cautious about calling InvalidateRect from inside WM_PAINT?

Oh sorry, must of overlooked the static declaration.

Thank you for all your help. InvalidateRect works.

This is my first time messing with GDI so I had no idea about the memory leaks.


edit:
Actually I see the memory leak now. I have no clue why I was creating a brush there.

Originally Posted by Vicious

Thank you for all your help. InvalidateRect works.

This is my first time messing with GDI so I had no idea about the memory leaks.


edit:
Actually I see the memory leak now. I have no clue why I was creating a brush there.

Yeah. Memory leaks can lead to alot of problems in your application if you're not careful with your resources. I suggest you get a good book like Programming Windows, Fifth Edition by the Good'Ole Charles Petzold. May I also suggest that, you do not invoke BeginPaint anywhere else but in response to a WM_PAINT message. This is for many good reasons. It helps avoid problems like the one you've just now experienced. If you need to paint outside of WM_PAINT, call GetDC. This function will allow you to paint anywhere on your clien area - making a InvalidateRect call unnecessary before painting, like in this situation. If you want to control the clippping (where you're allowed to paint in the client area) you can set the initial clipping region instead, with GetDCEx.