Drawing bitmaps efficiently

I'm working on makin some sort of image editor that will resize bitmaps for me. I am currently drawing bitmaps to a "static" window just to make sure i'm loading the bitmaps correctly. So here comes a couple questions.

1) Is there a way to draw a bitmap to the screen all at once, instead of setting every pixel manually? This obviously takes some time when I'm dealing with images 1280x800 or higher. I found some stuff about HBITMAP but i'm not quite sure if that's what I'm in need of yet as I can't find a definite answer on its use. I think drawing the bitmap this way is also the reason why my CPU usage jumps to 50%+ so it has to be fixed.

2) The bmp loader I made works well for certain bitmap images. I've used it to load textures for opengl for quite a while and have never had a problem. But now that I'm throwing the image to a static screen, about 1 out of 3 images will end up being slanted with the colors screwed up. This never happened in opengl, and I have tested this on 3 computers and they all do the same thing. What gives?

Here is the code I'm using to draw the bitmap to the static screen, along with the example of the slanted bitmap.

Code:

---

        case WM_PAINT:
        {
            if(UpdateCanvasImage){
                UINT error;
                ///set up for displaying picture to the screen
                InvalidateRect(hCanvas, 0, true); //hCanvas is the static window
                PAINTSTRUCT ps;
                HDC winDC = BeginPaint(hCanvas, &ps);
               
                for(int y = 0; y < lParam; y++) {
                    for(unsigned int x = 0; x < wParam; x++) {    //g_Canvas is a class defining the bmp image
                        BYTE R = g_Canvas.GetPixel(x, y, PIXEL_R, error);
                        BYTE G = g_Canvas.GetPixel(x, y, PIXEL_G, error);
                        BYTE B = g_Canvas.GetPixel(x, y, PIXEL_B, error);
                        SetPixel(winDC, x, lParam - y, RGB(R, G, B));
                    }
                }
                EndPaint(hCanvas, &ps);
                ///End set up for displaying picture to screen
               
                PostMessage(hwnd, RECEIVE_MSG, MSG_SETREADY, 0);
                UpdateCanvasImage = false;
                //SetWindowPos(hCanvas, HWND_TOP, 0, 0, wParam, lParam, SWP_SHOWWINDOW);
            }
        }
        break;

---

Load it into a MemDC, do whatever proccessing you need to do, then blast it to the screen with bitblt. Also any of the other methods will draw it pixel by pixel just like you are doing manually, except with a back buffer approach you can avoid the tearing. Also instead of blitting manually like that, MFC probably wraps functionallity around the static window in the CStatic class to do it. Check out CStatic::SetBitmap.

CStatic::SetBitmap

Inside of Windows the fastest way to do it is with GDI or GDI+. Any of your methods are going to be considerably slower. Know that GDI and GDI+ are not the fastest graphics APIs out there and they were not built for speed.

Quote:

---

2) The bmp loader I made works well for certain bitmap images. I've used it to load textures for opengl for quite a while and have never had a problem. But now that I'm throwing the image to a static screen, about 1 out of 3 images will end up being slanted with the colors screwed up. This never happened in opengl, and I have tested this on 3 computers and they all do the same thing. What gives?

---

Show us your code for your bitmap loader. Do you account for things such as color depths being 16/8 bit and RLE encoding? (Google for a spec on the BMP format "BMP file format" on Google, or just visit Wotsit.)

For fast drawing….

Your WM_PAINT should be a single BitBlt() inside the BeginPaint() / EndPaint() and nothing else.
All other drawing should be done prior to calling for a WM_PAINT.
You must allow for WM_PAINT msgs to be generated external to your app (ie by the OS in response to other activities on the PC) and not redraw everything. Your WM_PAINT should only draw the invalidated rectangle and not the whole screen each time.
Keep a copy of the last screen (in a mem DC) while you construct the next screen.


I use two mem DCs.
One holds the current screen image [ScreenDC]
One holds the image I am constructing from live data etc [FrameDC].
Both are created the same size as the screen and compatible with it (when the app starts).
Both are cleaned up on close.

When you need a new screen;
Draw the new screen to the FrameDC.
BitBlt the whole FrameDC to the ScreenDC
Call for a WM_PAINT msg covering the whole client area.

When you get a WM_PAINT msg;
BitBlt the ScreenDC to the HDC in the PAINTSTRUCT using the RECT in the PAINTSTRUCT.

NOTE: For fast drawing use BOTH
InvalidateRect() [generates a WM_PAINT msg]
and
UpdateWindow() [sends the WM_PAINT directly to the callback, not via the OS msg queue].
NEVER call InvalidateRect() (or UpdateWindow() ) inside the WM_PAINT msg, it causes an infinte loop (negated in your code by your drawing flag).

Do a search on 'double buffering' here for more code.

Rows of bitmap data are 4-byte aligned. It's slanting because your DIB decoder is reading the padding, ignoring it's presence. It's actually an easy fix.

Sorry I did not get back to this right away, been pretty busy.

After getting the header and info from the bmp file this is how I do the checking/byte reading. Right now I do not take in to account anything but 24 bit bmp files.
@ Yarin, how is it an easy fix? I've always read bmp files in with this function for OpenGL and it rendered the textures correctly every time.

@novacain, I always like your ideas, I'll work on trying that.

One other thing I can't quite get. The way my WM_PAINT message is in the first post, the program takes up 50% cpu power, basically one of two cores. If I comment everything out of the message, but leave the message there so it is essentially empty, it still uses 50%. If I comment the message out, then it idles at 0-2% like it should. So... could repeated meaningless Post/SendMessage calls do this, or is there something else that can cause it?

EDIT: I should mentioned that I replace WM_PAINT with WM_DRAWSCREEN (my definition), and the cpu % went back down to 0. All that I changed was the message name, so is the WM_PAINT message being over used somehow?

Code:

---

    if(fileheader.bfType != 0x4D42){
        printf("***BMP load error: bad file format***\n");
        return false;
    }

    if(infoheader.biBitCount != 24){
        printf("***BMP load error: invalid color depth***\n");
        return false;
    }

    if(infoheader.biCompression != BI_RGB){
        printf("BMP image is compressed\n");
        return false;
    }
   
    width = infoheader.biWidth; //assign width value
    height = infoheader.biHeight; //assign height value
   
    (*pixelmap) = (unsigned char*)malloc(width*height*3);
    //ZeroMemory(*pixelmap, width*height*3);
    unsigned char alpha;

    for (int i = 0; i < infoheader.biWidth*infoheader.biHeight; i++)
    {
        fread(&rgbt, sizeof(RGBTRIPLE), 1, l_file);

        (*pixelmap)[j+0] = rgbt.rgbtRed; // Red component
        (*pixelmap)[j+1] = rgbt.rgbtGreen; // Green component
        (*pixelmap)[j+2] = rgbt.rgbtBlue; // Blue component
        j += 3; // Go to the next position
    }

    fclose(l_file); //closes the file stream

    return true; //returns true if it was successful
}

---

See the Wikipedia page on BMP. In that table, there is a row which reads "Padding for 4 byte alignment (Could be a value other than zero)" - look at the corresponding data too. That is what Yarin mentions.

And I thought OpenGL provided a BMP file loader?

Quote:

---

Originally Posted by Cactus_Hugger

And I thought OpenGL provided a BMP file loader?

---

It probably does. I never found anything for one, but I didn't look very hard either (and by not very hard i mean maybe once). Once I made a working bmp loader I did away with looking for a loader.

Alright... I got the alignment down, but came up with this messed up problem that I can't trace down. I resaved the file as I loaded it to make sure I am properly loading the file, and when I view the resaved file it looks exactly as the original so I know I am loading it correctly. Here's what's happening, at some width, that doesn't change, the image drops off, then after that same width the image comes back and it repeats in this manner. There's a couple pictures to show what I am talking about. As you can see the static window in grey has the correct width/height so I know it is not a problem with that. Also, the code below is how I am loading the bitmap, and how it is being displayed to the screen.

Code:

---

//After doing the header/info reading...
    width = infoheader.biWidth; //assign width value
    height = infoheader.biHeight; //assign height value
   
    (*pixelmap) = (unsigned char*)malloc(width*height*3);

    //ZeroMemory(*pixelmap, width*height*3);
    unsigned char padding;

    for (int i = 0; i < infoheader.biWidth*infoheader.biHeight; i++)
    {
        fread(&rgbt, sizeof(RGBTRIPLE), 1, l_file);
       
        (*pixelmap)[j+0] = rgbt.rgbtRed; // Red component
        (*pixelmap)[j+1] = rgbt.rgbtGreen; // Green component
        (*pixelmap)[j+2] = rgbt.rgbtBlue; // Blue component
        j += 3; // Go to the next position
       
        if(i % infoheader.biWidth == 0){
            for(int n = 0; n < infoheader.biWidth % 4; n++)
                fread(&padding, 1, 1, l_file);
        }
    }
   
    fclose(l_file); //closes the file stream



//Drawing proceedure, now it's not in WM_PAINT
//All the 'error' calls return ERR_SUCCESS, so it's not hitting out of bounds

        case WM_DRAWCANVAS:
        {
            if(UpdateCanvasImage){
                printf("UpdateCanvasImage = true\n");
                UINT error;
                ///set up for displaying picture to the screen
                InvalidateRect(hCanvas, 0, true);
               
                PAINTSTRUCT ps;
                HDC winDC = BeginPaint(hCanvas, &ps);
               
                for(int y = 0; y < g_Canvas.Height(); y++){
                    for(int x = 0; x < g_Canvas.Width(); x++){
                       
                        BYTE R = g_Canvas.GetPixel(x, y, PIXEL_R, error);
                        if(error != ERR_SUCCESS) cout<< error <<endl;
                       
                        BYTE G = g_Canvas.GetPixel(x, y, PIXEL_G, error);
                        if(error != ERR_SUCCESS) cout<< error <<endl;
                       
                        BYTE B = g_Canvas.GetPixel(x, y, PIXEL_B, error);
                        if(error != ERR_SUCCESS) cout<< error <<endl;
                       
                        SetPixel(winDC, x, lParam - y, RGB(R, G, B));
                    }
                }
                EndPaint(hCanvas, &ps);
                ///End set up for displaying picture to screen
               
                PostMessage(hwnd, RECEIVE_MSG, MSG_SETREADY, 0);
                UpdateCanvasImage = false;
            }
        }
        break;

---

EDIT: sorry for the size, I don't know how to make the images stacked.

Not exactly sure, but why are you using 'lParam - y'? Also, you're still using InvalidateRect incorrectly. Remember, the purpose of the function is to mark a certain region as 'invalid' and, if the last parameter is TRUE, it generates a WM_PAINT message (so it obviously doesn't make sense to call it during the *painting* process). Logically, it should be called whenever you want to update the screen (such as after loading the image). Finally, BeginPaint/EndPaint should be handled exclusively in response to the WM_PAINT message.

The bitmap is stored in the file upside down, so my array that has the pixel information is upside down and needs to be swapped when drawing it to the screen, so basically 'lParam - y' is 'Height - current height' since I'm passing the width and height through the wParam and lParam arguments to cut back on global variables. (yes I know my class is global and can call the width and height).

I tried using the WM_PAINT message, and for some reason (with or without InvalidateRect() ) just the fact that I'm using that message jumps the cpu % to 50. If I use the message I have in my last post, the cpu stays down.

I've found a few things out, but none of it is making sense
-the loader is loading the files properly
-the drawing may have something to do with it, not sure yet
-the open file does not have to do with this (since all it does is call LoadBMP and post a message)
-the width, height, and image are being set correctly
-the static window initializes to the correct size

This is not making any sense to me.

>> so basically 'lParam - y' is 'Height - current height'

OK, got it.

I would suspect then that the problem might be in the implementation of GetPixel (whatever the type of g_Canvas is). An easy way to test this would be to do something like:

Code:

---

SetPixel(winDC, x, lParam - y, RGB(0, 0, 255));

---

If you get a solid, unbroken rectangle, then the implementation is somehow broken. Otherwise, I would recommend posting more code so that we can get a better idea of what else could be the problem.

Quote:

---

Originally Posted by Sebastiani

If you get a solid, unbroken rectangle, then the implementation is somehow broken. Otherwise, I would recommend posting more code so that we can get a better idea of what else could be the problem.

---

That was a good idea, but I still get the broken image in the exact same format as shown above. The code for my GetPixel function is there. I'm almost certain it's not that since I use to have a global array of the same thing and got the pixel in the same fashion and it worked correctly. This use to work, even with my CANVAS class, then I changed a few things and didn't remember what I changed, and it didn't work :(

I'm working on using novacain's idea on the BitBlt, hoping my problem goes away somewhere in the process. I'm still completely confused on why my exe file creates 50% cpu with just the WM_PAINT message being present. I tested this by creating a new project, changed nothing except adding case WM_PAINT: {} break; and boom, cpu shoots up. I have no idea why. So until that also gets figured out my paint message will be replaced by my own defined message that acts as WM_PAINT.

Code:

---

unsigned char CANVAS::GetPixel(int col /*x*/, int row /*y*/, UINT color, UINT &error)
{
    if(m_empty){
        error = ERR_NO_CANVAS;
        return 0;
    }
    else if(col > m_width){
        error = ERR_WIDTH_TOO_LARGE;
        return 0;
    }
    else if(row > m_height){
        error = ERR_HEIGHT_TOO_LARGE;
        return 0;
    }
   
    //PIXEL_R = 9
    //PIXEL_G = 8
    //PIXEL_B = 7
    error = ERR_SUCCESS;
    return m_im_rgb[(col + row*m_width)*3 + PIXEL_R - color];
}

---

Except for an out-of-bounds bug (if col == m_width or row == m_height you're going to overstep the array), the function appears OK. It isn't very efficient though since you have to call it three times to get the color values. I would recommend returning the entire pixel, personally. But I still don't see how you're getting that output (although if you call InvalidateRect/BeginPaint/EndPaint in the wrong manner you could certainly run into this problem).

>> I'm still completely confused on why my exe file creates 50% cpu with just the WM_PAINT message being present.

Without seeing more code (which would also help with pinpointing the first problem), it's impossible to say.

The easiest way to draw a bitmap in a static control is to let someone else do it.

Code:

---

case WM_CREATE: // or wherever else the control is created
{
    // you can set the control height and width to 0 as it'll resize automatically
    HWND hStatic = CreateWindowEx(0, WC_STATIC, TEXT(""), SS_BITMAP | WS_CHILD, ...);
    if(hStatic)
    {
        HBITMAP hBm = // load your hbitmap
        SendMessage(hStatic, STM_SETIMAGE, IMAGE_BITMAP, (LPARAM)hBm);
        ShowWindow(hStatic, SW_SHOW);
    }
}
break;

---

You could also create a pattern brush with CreatePatternBrush using your HBITMAP and return the brush in reponse to WM_CTLCOLORSTATIC for largely the same effect.

I don't know if it would help (and I'm a little rusty at Win32, so it may not be 100% correct), but here's a simple test harness that you can use to compare with:

Code:

---

#include <windows.h>
#include <stdio.h>

void cleanup( void );
LRESULT callback( HWND handle, UINT code, WPARAM wparam, LPARAM lparam );

HWND
        window = 0;
HDC
        foreground = 0;
HBITMAP
        bitmap = 0,
        saved = 0;

int main( int argc, char** argv )
{
        if( argc != 2 )
        {
                fprintf( stderr, "- Bitmap Test -\n" );
                fprintf( stderr, "Usage: %s <FILE_NAME>\n", argv[ 0 ] );
                cleanup( );
                return 1;
        }
        bitmap = HBITMAP( LoadImage( 0, argv[ 1 ], IMAGE_BITMAP, 0, 0, LR_LOADTRANSPARENT | LR_LOADFROMFILE ) );
        if( bitmap == 0 )
        {
                fprintf( stderr, "Error: could not load file '%s'\n", argv[ 1 ] );
                cleanup( );
                return 2;
        }
        WNDCLASS
                settings;       
        memset( &settings, 0, sizeof( settings ) );
        settings.lpfnWndProc = WNDPROC( callback );
        settings.hInstance = GetModuleHandle( 0 );
        settings.hIcon = LoadIcon( 0, IDI_APPLICATION );
        settings.hCursor = LoadCursor( 0, IDC_ARROW );
        settings.lpszClassName = __FILE__;
        if( !RegisterClass( &settings ) )
        {
                fprintf( stderr, "Error: could not register window class '%s'\n", settings.lpszClassName );
                cleanup( );
                return 3;
        }
        RECT
                size;       
        SystemParametersInfo( SPI_GETWORKAREA, 0, &size, 0 );       
        window = CreateWindow
        (
                settings.lpszClassName,
                "Bitmap Test",
                WS_OVERLAPPEDWINDOW,
                0,
                0,
                size.right - size.left,
                size.bottom - size.top,
                0,
                0,
                settings.hInstance,
                0
        );
        if( window == 0 )
        {
                fprintf( stderr, "Error: could not create window" );
                cleanup( );
                return 4;
        }
        HDC
                canvas = GetDC( window );
        foreground = CreateCompatibleDC( canvas );
        ReleaseDC( window, canvas );
        saved = HBITMAP( SelectObject( foreground, bitmap ) );       
        ShowWindow( window, SW_SHOW );
        UpdateWindow( window );
        MSG
                message;       
        while( GetMessage( &message, 0, 0, 0 ) > 0 )
        {       
                TranslateMessage( &message );
                DispatchMessage( &message );
        }
        cleanup( );       
        return message.wParam;               
}       

LRESULT callback( HWND handle, UINT code, WPARAM wparam, LPARAM lparam )
{
        if( code == WM_CLOSE )
        {               
                PostQuitMessage( 0 );
                return 0;
        }
        else if( code == WM_ERASEBKGND )
        {
        /*
                We'll redraw the background in the WM_PAINT code, instead
        */       
                return 1;
        }
        else if( code == WM_SIZE || code == WM_MOVE )
        {
                InvalidateRect( window, 0, 0 );
        }
        else if( code == WM_PAINT )
        {               
                PAINTSTRUCT
                        info;
                HDC
                        canvas = BeginPaint( window, &info );
        /*
                We could actually use PAINTSTRUCT::rcPaint to calculate the
                dimensions, but I opted to go this route, for the sake of clarity.
        */
                RECT
                        bounds;
                GetClientRect( window, &bounds );
                int
                        width = bounds.right - bounds.left,
                        height = bounds.bottom - bounds.top;
                HBRUSH
                        saved = HBRUSH( SelectObject( canvas, GetStockObject( LTGRAY_BRUSH ) ) );       
                PatBlt( canvas, 0, 0, width, height, PATCOPY );
                BitBlt( canvas, 0, 0, width, height, foreground, 0, 0, SRCCOPY );
                SelectObject( canvas, saved );
                EndPaint( window, &info );
        }
        return DefWindowProc( handle, code, wparam, lparam );               
}

void cleanup( void )
{
        SelectObject( foreground, saved );
        DeleteObject( bitmap );
        DeleteDC( foreground );
        DestroyWindow( window );
}

---

How did you create/cast the lParam?

Does it draw a rectangle if you replace lParam with the height?

SetPixel(winDC, x, g_Canvas.Height() - y, RGB(R, G, B));


My guesses....

Quote:

---

Originally Posted by scwizzo

One other thing I can't quite get. The way my WM_PAINT message is in the first post, the program takes up 50% cpu power, basically one of two cores.

---

100% of one the one core it is using?

Quote:

---

Originally Posted by scwizzo

If I comment everything out of the message, but leave the message there so it is essentially empty, it still uses 50%.

---

Something is sending a paint msg as fast as it can or not clearing the paint rect?

Quote:

---

Originally Posted by scwizzo

If I comment the message out, then it idles at 0-2% like it should.

---

Sounds like 'essentially empty' means no Begin/EndPaint call to validate the invalid paint rect,

but a return that says you have processed the msg?

Commenting out the handler calls the default processing, which validates the paint rect?

I replaced the lParam with Canvas.Height(), which returns the height of the bitmap, and it still draws the same as if lParam was left there. When I call the WM_PAINT message I post a message like the following.

Code:

---

PostMessage(hwnd, WM_PAINT, g_Canvas.Width(), g_Canvas.Height() );

---

I also found out why it was sucking up the cpu, which yes was basically 100% of one core on my dual core cpu. It was from calling BeginPaint() on another window other than the one WM_PAINT belongs to so when I called something like the following I got lots of CPU drain. If I did the latter version it was fine yet after the image was drawn to the static window it'd be deleted (obviously since it's painting to a window that doesn't have a WM_PAINT message). I think the reason it would still drain 50% even if the body of WM_PAINT was commented came from a stray InvalidateRect() I had forgotten about elsewhere in the messages I created.

Code:

---

//CPU drained from this
case WM_PAINT:
{
    PAINTSTRUCT ps;
    HDC hdc = BeginPaint(hCanvas /*other window*/, &ps);
    EndPaint(hCanvas, &ps);
}
break;

//CPU not drained from this
case WM_PAINT:
{
    PAINTSTRUCT ps;
    HDC hdc = BeginPaint(hwnd /*main window*/, &ps);
    EndPaint(hwnd, &ps);
}
break;

---

So, a recap, I still need to figure out a way to effectively draw the entire bitmap to a window/canvas/rectangle of some kind. Is there another type of window I should be looking in to instead of drawing my bitmap to a static window? I'm still working on BitBlt-ing the image.

>> PostMessage(hwnd, WM_PAINT, g_Canvas.Width(), g_Canvas.Height() );

Don't do that. That's what InvalidateRect is for.

>> So, a recap, I still need to figure out a way to effectively draw the entire bitmap to a window/canvas/rectangle of some kind. Is there another type of window I should be looking in to instead of drawing my bitmap to a static window? I'm still working on BitBlt-ing the image.

Post a small working program that demonstrates the problem. Did you look over the example I posted earlier?

Quote:

---

Originally Posted by Sebastiani

Post a small working program that demonstrates the problem. Did you look over the example I posted earlier?

---

Yes I did, it gave me a better idea, thank you. I've been pretty busy so I've been doing more pondering then doing on my project lately. Here are the snippets of code that are involved in getting the bitmap from a file, to the screen. I hacked it up since the project file is a little too large to post, but it is in order of calling/use with the exception of the bmp loading function first.

Code:

---

bool LoadBitmapFile(string filename, unsigned char **pixelmap, int &width, int &height)
{
    if(filename.empty()) return false;
    int j=0; //Index variable
    FILE *l_file; //File pointer

    // windows.h gives us these types to work with the Bitmap files
    BITMAPFILEHEADER fileheader;
    BITMAPINFOHEADER infoheader;
    RGBTRIPLE rgbt; //store one pixel at a time in here

    l_file = fopen(filename.c_str(), "rb");
    if(!l_file) return false; // Open the file for reading

    fread(&fileheader, sizeof(fileheader), 1, l_file); // Read the fileheader

    fseek(l_file, sizeof(fileheader), SEEK_SET);
    fread(&infoheader, sizeof(infoheader), 1, l_file);
   
    if(fileheader.bfType != 0x4D42){
        printf("***BMP load error: bad file format***\n");
        return false;
    }

    if(infoheader.biBitCount != 24){
        printf("***BMP load error: invalid color depth***\n");
        return false;
    }
   
    if(infoheader.biCompression != BI_RGB){
        printf("BMP image is compressed\n");
        return false;
    }
   
    width = infoheader.biWidth; //assign width value
    height = infoheader.biHeight; //assign height value
   
    (*pixelmap) = (unsigned char*)malloc(width*height*3);

    //ZeroMemory(*pixelmap, width*height*3);
    unsigned char padding;

    for (int i = 0; i < infoheader.biWidth*infoheader.biHeight; i++)
    {
        fread(&rgbt, sizeof(RGBTRIPLE), 1, l_file);
       
        (*pixelmap)[j+0] = rgbt.rgbtRed; // Red component
        (*pixelmap)[j+1] = rgbt.rgbtGreen; // Green component
        (*pixelmap)[j+2] = rgbt.rgbtBlue; // Blue component
        j += 3; // Go to the next position
       
        if(i % infoheader.biWidth == 0){
            for(int n = 0; n < infoheader.biWidth % 4; n++)
                fread(&padding, 1, 1, l_file);
        }
    }
   
    fclose(l_file); //closes the file stream

    return true; //returns true if it was successful
}

//This is the message posted when the user says file open... obviously...
                case CTRL_FILE_OPEN:
                {
                    if(hCanvas != NULL) DestroyWindow(hCanvas); //destroy any canvas window
                   
                    OPENFILENAME ofn;
                    char szFileName[100] = "";

                    ZeroMemory(&ofn, sizeof(ofn));

                    ofn.lStructSize = sizeof(ofn);
                    ofn.hwndOwner = hwnd;
                    ofn.lpstrFilter = "24-bit Bitmap (*.bmp)\0*.bmp\0All Files (*.*)\0*.*\0";
                    ofn.lpstrFile = szFileName;
                    ofn.nMaxFile = MAX_PATH;
                    ofn.Flags = OFN_EXPLORER | OFN_FILEMUSTEXIST | OFN_HIDEREADONLY | OFN_ENABLESIZING;
                    ofn.lpstrDefExt = "bmp";
                                        //ofn.lpstrInitialDir = "C:\\"; //might use this later on

                    if(GetOpenFileName(&ofn))
                    {
                        SendMessage(hStatus,SB_SETTEXT,0,(LPARAM)"Loading bitmap");
                        //filename is stored in lpstrFile
                        unsigned char* rgb;
                        int l_width, l_height;
                       
                        if(hCanvas != NULL) DestroyWindow(hCanvas); //destroy any previous window
                       
                        if(!LoadBitmapFile(ofn.lpstrFile, &rgb, l_width, l_height))
                            printf("Error: cannot open image file");
                        else
                            printf("Image pixel map loaded\n");
                       
                        hCanvas = CreateWindowEx(0,"STATIC",NULL,WS_CHILD|WS_VISIBLE,0,0,l_width,l_height,
                            hwnd,(HMENU)HCANVAS,GetModuleHandle(NULL),NULL);
                       
                        char temp[25];
                        sprintf_s(temp, 25, "(%i, %i)", l_width, l_height); //set up width and height to display to status bar
                       
                        SendMessage(hStatus,SB_SETTEXT,1,(LPARAM)temp);
                        SendMessage(hStatus,SB_SETTEXT,0,(LPARAM)"Loading successful");
                       
                        g_Canvas.SetCanvas(rgb, l_width, l_height);
                       
                        if(g_Canvas.empty()) printf("Error: canvas is still empty\n");
                        else printf("Canvas is set\n");

                                                SetWindowPos(g_MDIChildWindow, HWND_TOP, 0, 0, g_Canvas.Width(), g_Canvas.Height(), SWP_SHOWWINDOW);
                       
                        UpdateCanvasImage = true;
                                                InvalidateRect(hwnd, 0, true);
                        //PostMessage(hwnd, WM_PAINT, (UINT)g_Canvas.Width(), (UINT)g_Canvas.Height() );
                       
                        delete [] rgb;
                    }
                }
                break;


//This part didn't indent entirely right, but you can get the idea
                case WM_PAINT:
                        //hdc = BeginPaint(hwnd, &ps);
                        //EndPaint(hwnd, &ps);
                        //InvalidateRect(hCanvas, 0, false);
                       
                        if(UpdateCanvasImage){
                                printf("UpdateCanvasImage = true\n");
                                unsigned int error;
                                //InvalidateRect(hCanvas, 0, true);

                                PAINTSTRUCT ps;
                                HDC hdc = BeginPaint(hwnd, &ps);
                                //hdc = GetDC(hCanvas);
                               
                                                for(int y = 0; y < g_Canvas.Height(); y++){
                                                      for(int x = 0; x < g_Canvas.Width(); x++){

                                                            unsigned char RGB[3];
                       
                                                            error = g_Canvas.GetPixel(x, y, PIXEL_R, RGB);
                                                            if(error != ERR_SUCCESS) printf("Error obtaining pixel: %i\n", error);

                                                            SetPixel(hdc, x, g_Canvas.Height() - y, RGB(RGB[0], RGB[1], RGB[2]));
                                                        }
                                                  }

                                    EndPaint(hwnd, &ps);
                                    //ReleaseDC(hwnd, hdc);
               
                                    PostMessage(hwnd, RECEIVE_MSG, MSG_SETREADY, 0);
                                    UpdateCanvasImage = false;
                            }
                  break;

---

>> height = infoheader.biHeight; //assign height value

You need to check that infoheader.biHeight isn't negative. If it is, you'll need to negate height and set a flag (for your drawing routine, so it'll know which direction to copy the data).

The rest of you're snippet looks OK from the outset, but I'd still recommend posting a complete working example. To simplify things, you might even consider using LoadImage till you have all of the other problems worked out.

I am concerned that the drawing does not draw a simple rectangle.

Is there something wrong with your DCs or paint area?

Try changing the paint to

Code:

---

RECT rcClient;
GetClientRect(hCanvas,&rcClient);
HBRUSH hRed=CreateSolidBrush(RGB(255,0,0));
HBRUSH hBlack=CreateSolidBrush(RGB(0,0,0));
PAINTSTRUCT ps;

HDC winDC = BeginPaint(hCanvas, &ps);
FillRect(winDC,rcClient,hBlack);//draw the client area black
FillRect(winDC,ps.rcPaint,hRed);//draw the update area red
EndPaint(hCanvas, &ps);

DeleteObject(hRed);
DeleteObject(hBlack);

---

You should see only a red rectangle covering the whole client area.


I notice you delete the memory for 'rgb'.
SetCanvas() allocs more memory and fills it from 'rgb', not just copying the 'rgb' pointer? (dangling pointer)

Code:

---

g_Canvas.SetCanvas(rgb, l_width, l_height);

---

I also suggest you look at

AdjustWindowRectEx()

for creating a window with the client area exactly the size of the newly loaded image.

Quote:

---

Originally Posted by Sebastiani

>> height = infoheader.biHeight; //assign height value

You need to check that infoheader.biHeight isn't negative.

---

LPARAM and WPARAM are DWORDs, an unsigned data type. So a small -ive number will be converted into a very large +ive number if the casting fails.

You are passing them without a cast, try using the macros

MAKELPARAM / MAKEWPARAM to create the DWORDs and LOWORD / HIWORD to 'decode' them.



EDIT:
I see you are Destroy'ing the window each image load.
DestroyWindow() generates msgs, not sure if it completes the destroy before returning (with a new HWND value but the same ID, not sure if this could cause issues, prob not but....).

I would only create the 'canvas' window once, with DCs the size of the whole screen (so I do not have to recreate them as well).
Resize (with the returned rect from AdjustWindowRect) when a new image is loaded && hCanvas!=NULL.

Quote:

---

Originally Posted by scwizzo

OPENFILENAME ofn;
char szFileName[100] = "";

ofn.lpstrFile = szFileName;
ofn.nMaxFile = MAX_PATH;

---

That's not the most effective lie you'll ever tell :)

@novacain
I'll give you're suggestions a try, and see what happens with my window. And you are correct on the rgb pointer. The class allocates it's own memory in a member pointer. I'm thinking passing the class to the load bmp function and setting it that way would be a better route, but right now it's not my main concern. Also, the destroy window is there for a "safety measure". I blocked the user from having more than one cavnas/window open at the moment until I get the drawing under control. The window gets completely destroyed before any CTRL_FILE_OPEN call is ever made.

@adeyblue
I never noticed that, thank you.

EDIT: The entire client area is one big red block now.

First WTF - not using LoadImage or similar API.
Second WTF - using SetPixel instead of bitblt or similar API.

Quote:

---

First WTF - not using LoadImage or similar API.

---

I have future reasons of loading each pixel in to an array. I want to deal with certain image algorithms which required me to deal with each pixel individually.

Quote:

---

Second WTF - using SetPixel instead of bitblt or similar API.

---

Code:

---

case WM_PAINT:
{
    //hdc = BeginPaint(hwnd, &ps);
    //EndPaint(hwnd, &ps);
    //InvalidateRect(hCanvas, 0, false);
    PAINTSTRUCT ps;
    HDC hdc, img;
    HBITMAP hbmp;
    HANDLE hOld;
    int width, height;
    unsigned int error;

    BeginPaint(hwnd, &ps);
    hdc = GetDC(hCanvas);
    width = g_Canvas.Width(); //width of the canvas
    height = g_Canvas.Height(); //height of the canvas
    img = CreateCompatibleDC(hdc); //this will be the resulting image
    hbmp = CreateCompatibleBitmap(hdc, width, height); //gdi object
    hOld = SelectObject(img, hbmp); //select the gdi object

    if (UpdateCanvasImage)
    {
        printf("UpdateCanvasImage = true\n");

        for (int y = 0; y < g_Canvas.Height(); y++)
        {
            for (int x = 0; x < g_Canvas.Width(); x++)
            {
                unsigned char RGB[3];

                error = g_Canvas.GetPixel(x, y, PIXEL_R, RGB);
                if (error != ERR_SUCCESS)
                {
                    printf("Error obtaining pixel: %i\n", error);
                }

                SetPixel(img , x, g_Canvas.Height() - y, RGB(RGB[0], RGB[1], RGB[2]) );
            }
        }

        //InvalidateRect(hCanvas, 0, true);
        //

        BitBlt(hdc, 0, 0, width, height, img, 0, 0, SRCCOPY);

        PostMessage(hwnd, RECEIVE_MSG, MSG_SETREADY, 0);
        UpdateCanvasImage = false;
    }
    SelectObject(img, hOld);
    DeleteObject(hbmp);
    DeleteDC(img);

    EndPaint(hwnd, &ps);
    ReleaseDC(hCanvas, hdc);
    UpdateWindow(hwnd);
    //ReleaseDC(hwnd, hdc);
}
break;

---

There might be some redundant stuff in there.

Code:

---

case WM_PAINT:
{
    PAINTSTRUCT ps;
    HDC hdc, img;
    HBITMAP hbmp;
    HANDLE hOld;
    int width, height;
    unsigned int error;

    hdc = BeginPaint(hwnd, &ps);//use this DC as we will draw to it later

    width = g_Canvas.Width(); //width of the canvas
    height = g_Canvas.Height(); //height of the canvas
    img = CreateCompatibleDC(hdc); //this will be the resulting image
    hbmp = CreateCompatibleBitmap(hdc, width, height); //gdi object
    hOld = SelectObject(img, hbmp); //select the gdi object

    if (UpdateCanvasImage)
    {
        printf("UpdateCanvasImage = true\n");

        for (int y = 0; y < g_Canvas.Height(); y++)
        {
            for (int x = 0; x < g_Canvas.Width(); x++)
            {
                unsigned char RGB[3];

                error = g_Canvas.GetPixel(x, y, PIXEL_R, RGB);
                if (error != ERR_SUCCESS)
                {
                    printf("Error obtaining pixel: %i\n", error);
                    //set the pixel to white / black?
                }
                else//dont set the pixel to some random value if the GetPixel call fails!
                {
                    SetPixel(img , x, g_Canvas.Height() - y, RGB(RGB[0], RGB[1], RGB[2]) );
                }
            }
        }
        BitBlt(hdc, 0, 0, width, height, img, 0, 0, SRCCOPY);

        PostMessage(hwnd, RECEIVE_MSG, MSG_SETREADY, 0);
        UpdateCanvasImage = false;
    }
    SelectObject(img, hOld);
    DeleteObject(hbmp);
    DeleteDC(img);

    EndPaint(hwnd, &ps);
    //ReleaseDC(hwnd, hdc);//EndPaint clears up our DC now
    //UpdateWindow(hwnd);//this generates another paint msg in the window's queue, if there is an invalid area
    //should only be paired with an earlier call to InvalidateRect
    //NEVER USE EITHER INSIDE A WM_PAINT (infinite loop)
 }
break;

---

Creating the DC, BMP, SetPixel takes a LONG time, not good each time we get a paint msg.

I would do this more like..

Code:

---

case WM_CREATE://or WM_INITDIALOG
{
    //GetDC
    //get screen size 
    //create compatible DC and BMP as members of g_Canvas (and the original BMP)

    //NOTE: in this code g_Canvas.m_ScreenDC is our created DC.
}
case WM_DESTORY://clean up
{
    //select the origianl BMP back
    //delete GDIs (ie g_Canvas.m_ScreenDC etc)
}
case WM_PAINT:
{
    PAINTSTRUCT ps;

    hdc = BeginPaint(hwnd, &ps);
    BitBlt(hdc,
              ps.rcPaint.left, ps.rcPaint.top, ps.rcPaint.right - ps.rcPaint.left, ps.rcPaint.bottom -  ps.rcPaint.top, //only redraw the invalid area, not the whole screen.
              g_Canvas.m_ScreenDC, //from our global object's compatible DC
              ps.rcPaint.left, ps.rcPaint.top, //assuming no offset for the image
              SRCCOPY);

    EndPaint(hwnd, &ps);
}
break;

case IDC_LOAD_IMAGE:
//when new image loads
    //load image into temp location
    //if load fails
        //inform user
        //clean up
        //exit (leave old image loaded?)
  //else successful load
        //set the g_Canvas member vars to the new image size
        //free old member image array
        //alloc and set member image pixel array from temp image array
        //clean up temp image vars

        //NOTE do not free DC or BMP, just redraw over them

        //reset the DC to a base colour
        FillRect( g_Canvas.m_ScreenDC, g_Canvas.m_rcArea, GetStockObject( BLACK_BRUSH)); //EDIT: This is the max DC size, not new image size (could be smaller than last image and leave remnants)

        //set pixels
        for (int y = 0; y < g_Canvas.Height(); y++)
        {
            for (int x = 0; x < g_Canvas.Width(); x++)
            {
                unsigned char RGB[3];

                error = g_Canvas.GetPixel(x, y, PIXEL_R, RGB);
                if (error != ERR_SUCCESS)
                {
                    printf("Error obtaining pixel: %i\n", error);
                    //error pixels will remain BLACK
                }
                else SetPixel(g_Canvas.m_ScreenDC , x, g_Canvas.Height() - y, RGB(RGB[0], RGB[1], RGB[2]) );
            }
        }
        //drawing finished on our buffer
        // need to send to the screen
        InvalidateRect(hwnd, NULL, FALSE);//redraw whole client
        UpdateWindow(hwnd); //bypass OS msg queue and post to the window msg queue for quick response

---

Thank you novacain, once again. I've moved things around in a relatively similar manner you suggested and things draw about as fast as they're ever going to, which is instantly in 99% of cases.