HPEN object declaration

[Powerpuncher]

Hey,

I had a problem that I fixed, but probably not properly.

My problem was, that after a few seconds, the window just stopped redrawing and responding to my inputs.

I commented out parts of the code to nail it down and the HPEN's I created seemed to be the culprit of my problem. So I did some research and found out that this problem is caused by undeleted objects. I tried deleting the HPEN objects in the WM_PAINT case, but the problem was only fixed by deleting them in the default case.

I imagined that this is because these objects get created everytime the WinProc is called, so I tried to declare them in the WM_PAINT case, but that caused errors:

jump to case label [-fpermissive]|
crosses initialization of 'HPEN__* hPenD5Red'|

After a quick search I put the WM_PAINT case in an explicit block and that fixed it. But I can't help but feel like this is not the proper way to handle this. Is it better to declare these objects globally and not deleting them? Or should I use a class to handle this?

  LRESULT CALLBACK WindowProcedure(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam){
    ...

    HDC hdc;                     // This was the original position of the declaration
    PAINTSTRUCT ps;
    ...
    HPEN hPen = NULL,
         hPenS1Blue = CreatePen(PS_SOLID, 1, RGB(0, 0, 255)),
         hPenD5Red  = CreatePen(PS_DOT,   5, RGB(255, 0, 0));
    ...

    switch (message){
        // ______________________ WM_PAINT ______________________
        case WM_PAINT:{
            HDC hdc;             // New position
            PAINTSTRUCT ps;
            ...
            HPEN hPen = NULL,
                 hPenS1Blue = CreatePen(PS_SOLID, 1, RGB(0, 0, 255)),
                 hPenD5Red  = CreatePen(PS_DOT,   5, RGB(255, 0, 0));
            ...

            hdc = BeginPaint(hwnd, &ps);
                ...

                hPen = (HPEN)SelectObject(hdc, hPenS1Blue);
                Rectangle(hdc, 50, 100, 100, 250);
                SelectObject(hdc, hPen);

                hPen = (HPEN)SelectObject(hdc, hPenD5Red);
                Ellipse(hdc, 120, 100, 170, 250);
                SelectObject(hdc, hPen);

                ...
            EndPaint(hwnd, &ps);

            DeleteObject(hPenS1Blue);
            DeleteObject(hPenD5Red);
            DeleteObject(hPen);
            break;}
        ...
      return 0;
}

[andywestken]

Is it better to declare these objects globally and not deleting them? Or should I use a class to handle this?

Your solution looks OK to me.

It's pretty standard practice to create and delete pens on each WM_PAINT message. For a normal app this should not be a problem, as it won't be repainting itself that often. But if your apps is updating itself frequently then it might be better to hold onto the pens.

But if I read your code fragment correctly your pens were not global -- they were declared in the scope of you WindowProc but outside the while loop. If this was the case it's not surprising you ran out of GDI handles as you were creating new pens for each and every Windows message you were sent, which is a lot!

By the way, you should not be deleting the pen you did not create. I see three calls to DeleteObject (for pens) but only two to CreatePen. The one you got from the device context when you called SelectObject is not yours to delete. Esp. as I see you correctly restore it.

Using classes would be more C++. But it you're going that way you might want to look at using a OO windowing library/toolkit rather than raw WinAPI.

Andy

Choice #1 -- like your solution

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    switch (uMsg)
    {
        case WM_CREATE:
        {
        }
        break;

        case WM_DESTROY:
        {
            PostQuitMessage(0);
        }
        break;

        case WM_PAINT:
        {
            HPEN hPenS1Blue = CreatePen(PS_SOLID, 1, RGB(0, 0, 255));
            HPEN hPenD5Red  = CreatePen(PS_DOT,   5, RGB(255, 0, 0));

            PAINTSTRUCT ps = {0};
            HDC hdc = BeginPaint(hWnd, &ps);

            HPEN hPenOld = (HPEN)SelectObject(hdc, hPenS1Blue);
            Rectangle(hdc, 50, 100, 100, 250);
            // don't bother to restore old pen yet

            SelectObject(hdc, hPenD5Red);
            // already have old pen
            Ellipse(hdc, 120, 100, 170, 250);

            SelectObject(hdc, hPenOld);
            // now restore it

            EndPaint(hWnd, &ps);

            DeleteObject(hPenS1Blue);
            DeleteObject(hPenD5Red);
            // only delete the pens you created
        }
        break;

        // Etc

        default:
            return DefWindowProc(hWnd, uMsg, wParam, lParam);
    }

    return 0;
}

Choice #2 -- with statics

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    static HPEN s_hPenS1Blue = NULL; // I mark statics with an s_ prefix
    static HPEN s_hPenD5Red  = NULL; // could use globals for pens, too.

    switch (uMsg)
    {
        case WM_CREATE:
        {
            s_hPenS1Blue = CreatePen(PS_SOLID, 1, RGB(0, 0, 255));
            s_hPenD5Red  = CreatePen(PS_DOT,   5, RGB(255, 0, 0));
        }
        break;

        case WM_DESTROY:
        {
            DeleteObject(s_hPenS1Blue);
            s_hPenS1Blue = NULL;
            DeleteObject(s_hPenD5Red);
            s_hPenD5Red = NULL;
            // only delete the pens you created

            PostQuitMessage(0);
        }
        break;

        case WM_PAINT:
        {
            PAINTSTRUCT ps = {0};
            HDC hdc = BeginPaint(hWnd, &ps);

            HPEN hPenOld = (HPEN)SelectObject(hdc, s_hPenS1Blue);
            Rectangle(hdc, 50, 100, 100, 250);
            // don't bother to restore old pen yet

            SelectObject(hdc, s_hPenD5Red);
            // already have old pen
            Ellipse(hdc, 120, 100, 170, 250);

            SelectObject(hdc, hPenOld);
            // now restore it

            EndPaint(hWnd, &ps);
        }
        break;

        // Etc

        default:
            return DefWindowProc(hWnd, uMsg, wParam, lParam);
    }
    return 0;
}

[Powerpuncher]

Thank you very much for clearing all the questionmarks out of my head!
I'm still very new to all of this, so things like creating and destroying objects often goes over my head, because I don't have a clear image of them, yet.

Also thanks for the hint on restoring and destroying pens! I didn't realize simply declaring an HPEN object doesn't create a pen.