Call to GetDIBits() succeeds, but program terminates

Question

When I call the following function in a Windows program, the program abruptly terminates.

The purpose of ScanRect() is to copy a rectangle at specified coordinates on the screen and load the pixel values into a memory buffer.

Every function call within ScanRect() succeeds, including both calls to GetDIBits(). The first call, with lpvBits set to NULL, causes it to fill the BITMAPINFOHEADER of bmInfo with information about the pixel data, reporting a value of 32 bits per pixel. The second call to GetDIBits() copies 80 lines of the rectangle into memory buffer pMem, returning the value 80 for the number of lines copied.

Everything seems to succeed, but then the program suddenly terminates. I inserted the line Sleep(8192) after the second call to GetDIBits(), and the program terminates after the 8 seconds have elapsed.

What is causing the program to terminate?

EDIT: the original code is revised per suggestions in this thread. No errors are detected when the function is run, but the program still terminates unexpectedly. I realize the memory buffer size is hard coded, but it is way bigger than needed for the rectangle used in the testing. That should not cause an error. Of course I will have the program compute the necessary buffer size after I find out why the program is terminating.

VOID ScanRect(int x, int y, int iWidth, int iHeight) // 992, 96, 64, 80
{   HDC hDC = GetDC(NULL);
    if (!hDC)
    {
      cout << "!hDC" << endl;        // error handling ...
    }
    else
    {   HBITMAP hBitmap = CreateCompatibleBitmap(hDC, iWidth, iHeight);
        if (!hBitmap)
        {
           cout << "!hBitmap" << endl;        // error handling ...
        }
        else
        {   HDC hCDC = CreateCompatibleDC(hDC); // compatible with screen DC
            if (!hCDC)
            {
              cout << "!hCDC" << endl;        // error handling ...
            }
            else
            {   HBITMAP hOldBitmap = (HBITMAP) SelectObject(hCDC, hBitmap);
                BitBlt(hCDC, 0, 0, iWidth, iHeight, hDC, x, y, SRCCOPY);
                BITMAPINFO bmInfo = {0};
                bmInfo.bmiHeader.biSize = sizeof(bmInfo.bmiHeader);
                if (!GetDIBits(hCDC, hBitmap, 0, iHeight, NULL, &bmInfo, DIB_RGB_COLORS))
                {
                  cout << "!GetDIBits" << endl; // error handling ...
                }
                else
                {   HANDLE hHeap = GetProcessHeap();
                    LPVOID pMem = HeapAlloc(hHeap, HEAP_ZERO_MEMORY, 65536); // TODO: calculate a proper size based on bmInfo's pixel information ...
                    if (!pMem)
                    {
                      cout << "!pMem" << endl;
                    }
                    else
                    {   int i = GetDIBits(hCDC, hBitmap, 0, iHeight, pMem, &bmInfo, DIB_RGB_COLORS);
                        cout << "i returned by GetDIBits() " << i << endl;
                        HeapFree(hHeap, NULL, pMem);
                    }
                }
                SelectObject(hCDC, hOldBitmap);
                DeleteDC(hCDC);
            }
            DeleteObject(hBitmap);
        }
        ReleaseDC(NULL, hDC);
    }
}

Use a debugger rather than sleep for figuring out where the problem is. — SoronelHaetir
– SoronelHaetir, Commented May 10, 2021 at 21:09
You should always undo your SelectObject before deleting either the HDC or the HGDIOBJ. — Ben Voigt
– Ben Voigt, Commented May 10, 2021 at 21:27
You don't know whether any function succeeds or not because you do no error checking — David Heffernan
– David Heffernan, Commented May 10, 2021 at 21:28
David Heffernan, I did error checking by using cout to a console created by AllocConsole, but I omitted that from the code example for brevity. Trust me, I checked the return values of all the functions called, and they succeed. If you copy and paste the function into a Windows program, you can check the return values. The value of i is 80 after the second call to GetDIBits(). Per the suggestion by Ben Voight, I will attempt to undo SelectObject() after I figure out how to do that. But note that when I comment out the last four lines of the function, the program still terminates. — Steven Brown
– Steven Brown, Commented May 10, 2021 at 22:01
You didn't reserve space for the color table, so this probably overflowed the stack buffer, which is bad news. — Raymond Chen
– Raymond Chen, Commented May 11, 2021 at 0:14

YangXiaoPo-MSFT · Accepted Answer · 2021-05-11 09:59:07Z

1

The biCompression value is returned by first GetDIBits is BI_BITFIELDS and before you call second GetDIBits, you need to call bmInfo.bmiHeader.biCompression = BI_RGB;. According to c++ read pixels with GetDIBits(), Setting it to BI_RGB is essential in order to avoid extra 3 DWORDs to be written at the end of structure.
More details

answered May 11, 2021 at 9:59

YangXiaoPo-MSFT

2,1931 gold badge9 silver badges32 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Steven Brown Over a year ago

Thank you to YangXiaPo for posting the definitive solution to the problem. After the biCompression field of the BITMAPINOHEADER member of the BITMAPINFO structure is set to BI_RGB, the ScanRect() function works as expected, and the program does not unexpectedly terminate. The "solutions" I posted got around the problem and hinted at the cause, a buffer overrun causing a stack overflow.

Remy Lebeau · Accepted Answer · 2021-05-11 01:08:25Z

0

Like @BenVoigt said in comments, you need to restore the old HBITMAP that you replaced with SelectObject() before you destroy the HDC that owns it. You are selecting hBitmap into hCDC, and then destroying hCDC before destroying hBitmap.

https://learn.microsoft.com/en-us/windows/win32/gdi/operations-on-graphic-objects

Each of these functions returns a handle identifying a new object. After an application retrieves a handle, it must call the SelectObject() function to replace the default object. However, the application should save the handle identifying the default object and use this handle to replace the new object when it is no longer needed. When the application finishes drawing with the new object, it must restore the default object by calling the SelectObject() function and then delete the new object by calling the DeleteObject() function. Failing to delete objects causes serious performance problems.

Also, you should free the GDI objects in the reverse order that you create them.

And, don't forget error handling.

Try something more like this instead:

VOID ScanRect(int x, int y, int iWidth, int iHeight) // 992, 96, 64, 80
{ 
    HDC hDC = GetDC(NULL);
    if (!hDC)
    {
        // error handling ...
    }
    else
    {
        HBITMAP hBitmap = CreateCompatibleBitmap(hDC, iWidth, iHeight);
        if (!hBitmap)
        {
            // error handling ...
        }
        else
        {
            HDC hCDC = CreateCompatibleDC(hDC); // compatible with screen DC
            if (!hCDC)
            {
                // error handling ...
            }
            else
            {
                HBITMAP hOldBitmap = (HBITMAP) SelectObject(hCDC, hBitmap);

                BitBlt(hCDC, 0, 0, iWidth, iHeight, hDC, x, y, SRCCOPY);

                SelectObject(hCDC, hOldBitmap);

                BITMAPINFO bmInfo = {0};
                bmInfo.bmiHeader.biSize = sizeof(bmInfo.bmiHeader);

                if (!GetDIBits(hCDC, hBitmap, 0, iHeight, NULL, &bmInfo, DIB_RGB_COLORS))
                {
                    // error handling ...
                }
                else
                {
                    HANDLE hHeap = GetProcessHeap();
                    LPVOID pMem = HeapAlloc(hHeap, HEAP_ZERO_MEMORY, 65536); // TODO: calculate a proper size based on bmInfo's pixel information ...
                    if (!pMem)
                    {
                        // error handling ...
                    }
                    else
                    {
                        int i = GetDIBits(hCDC, hBitmap, 0, iHeight, pMem, &bmInfo, DIB_RGB_COLORS);
                        HeapFree(hHeap, NULL, pMem);
                    }
                }

                DeleteDC(hCDC);
            }

            DeleteObject(hBitmap);
        }

        ReleaseDC(NULL, hDC);
    }
}

Note the TODO on the call to HeapAlloc(). You really should be calculating the buffer size based on the bitmap's actual width, height, pixel depth, scanline padding size, etc. Don't use a hard-coded buffer size. I will leave this as an exercise for you to figure out. Although, in this particular example, 64K should be large enough for a 64x80 32bpp bitmap, it will just waste 45K of unused memory.

edited May 11, 2021 at 1:08

answered May 10, 2021 at 22:08

Remy Lebeau

609k36 gold badges516 silver badges875 bronze badges

10 Comments

Steven Brown Over a year ago

I copied and pasted your code example into my program. It complies but when I ran it, the program still terminates unexpectedly. And now I will check the return values.

Steven Brown Over a year ago

I checked the return values and there are no errors. However, the program still terminates.

Remy Lebeau Over a year ago

Then the problem is likely not in this code. Run your code under a debugger and see exactly what is crashing and where.

Steven Brown Over a year ago

I will give that a try. You have been helpful.

Steven Brown Over a year ago

By "fixed the problem" I meant the program stopped terminating, and that was a clue to the nature of the problem. The eventual solution was to set the biCompression field of the BITMAPINFOHEADER to BI_RGB, preventing the BITMPAPINFO structure exceeding bounds.

|

Collectives™ on Stack Overflow

Call to GetDIBits() succeeds, but program terminates

2 Answers 2

1 Comment

10 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

10 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related