Optimization possible? - Screenshot

Hi, I have this function for taking a screenshot and extracting the pixels, it is working but I have a feeling it could be optimized and I don't really know what all the commands do. Can you spot any superflous steps or any other issues?

Code:

---

bool GetScreenshot()
{
        int screenWidth = GetSystemMetrics(SM_CXSCREEN);
        int screenHeight = GetSystemMetrics(SM_CYSCREEN);

        HDC hdcScreen = CreateDC(TEXT("DISPLAY"), NULL, NULL, NULL);
        HDC hdcShot = CreateCompatibleDC(hdcScreen);
        HBITMAP hbmap = CreateCompatibleBitmap(hdcScreen, screenWidth, screenHeight);
        HBITMAP hBitmapOld = (HBITMAP) SelectObject(hdcShot, hbmap);
        BitBlt(hdcShot, 0, 0, screenWidth, screenHeight, hdcScreen, 0, 0, SRCCOPY);

        BITMAP bm;
        if (!GetObject(hbmap, sizeof(BITMAP), (LPSTR)&bm)) return false;
        int bitsPerPixel = bm.bmBitsPixel;
        int bwidth = bm.bmWidth;
        int bheight = bm.bmHeight;

        if (bitsPerPixel != 32 || bm.bmPlanes != 1) return false;

        BITMAPINFO bmi;
        bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
        bmi.bmiHeader.biWidth = bwidth;
        bmi.bmiHeader.biHeight = bheight;
        bmi.bmiHeader.biPlanes = 1;
        bmi.bmiHeader.biBitCount = bitsPerPixel;
        bmi.bmiHeader.biCompression = BI_RGB;
        bmi.bmiHeader.biSizeImage = 0;

        BYTE* bits = new BYTE[bwidth * bheight * 4];
        if (!bits) return false;

        HDC hdc = GetDC(GetDesktopWindow());
        if (!GetDIBits(hdc, hbmap, 0, bheight, bits, &bmi, DIB_RGB_COLORS))
        {
                ReleaseDC(GetDesktopWindow(), hdc);
                delete [] bits;
                return false;
        }
        ReleaseDC(GetDesktopWindow(), hdc);

        //Do things with bits here

        SelectObject(hdcShot, hBitmapOld);
        DeleteObject(hbmap);
        DeleteDC(hdcShot);
        DeleteDC(hdcScreen);

        return true;
}

---

Thanks!:)

How long does it take to call it once?
If it's less than say 20mS, then it's already faster than what you can perceive as being "instantaneous" and "taking some time".

How many times per second are you planning to call it?
If (X times per second) * (time per grab) is still less than one second, do you still need to do anything?

What other useful function can you be getting on with coding, rather than focussing on optimisation?

Since you are not using nothrow new, this is superfluous:

Code:

---

if (!bits) return false;

---

Rather, std::bad_alloc might be thrown. I suggest that you replace this:

Code:

---

BYTE* bits = new BYTE[bwidth * bheight * 4];
if (!bits) return false;

---

with:

Code:

---

std::vector<BYTE> bits(bwidth * bheight * 4);

---

and then change this:

Code:

---

if (!GetDIBits(hdc, hbmap, 0, bheight, bits, &bmi, DIB_RGB_COLORS))
{
    ReleaseDC(GetDesktopWindow(), hdc);
    delete [] bits;
    return false;
}

---

to:

Code:

---

if (!GetDIBits(hdc, hbmap, 0, bheight, &bits[0], &bmi, DIB_RGB_COLORS))
{
    ReleaseDC(GetDesktopWindow(), hdc);
    return false;
}

---

This could well fix a potential memory leak in your code.

I second the earlier changes.

If you're desperate to make it faster then several of those things could be precalculated in a seperate function, and passed into this function inside an object, each time it is called. However rather than being a useful optimisation, this would probably simply demonstrate that it's not worth trying to make it faster than it is already because there's very little gain left to be made.