Hello Triangle in Win32 and OpenGL

Question

At the moment, I'm trying to get some experience writing C++, Win32 and OpenGL, so I decided to implement a "Hello World" of sorts. However, instead of printing Hello World! to the console, I render a colorful triangle.

#include <Windows.h>
#include <gl/GL.h>
#include <gl/GLU.h>
#include <gl/glext.h>
#include <gl/wglext.h>
#include <gl/glcorearb.h>
#include "SWOGLL.h"

struct Window
{
    HINSTANCE hInstance;
    HWND hWnd;
    HDC hdc;
    HGLRC hglrc;
    int width;
    int height;
};
Window currentWindow;

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR szCmdLine, int iCmdShow);
LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam);

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR szCmdLine, int iCmdShow)
{
    WNDCLASS wc;
    wc.cbClsExtra = 0;
    wc.cbWndExtra = 0;
    wc.hbrBackground = (HBRUSH)GetStockObject(BLACK_BRUSH);
    wc.hCursor = LoadCursor(NULL, IDC_ARROW);
    wc.hIcon = LoadIcon(NULL, IDI_APPLICATION);
    wc.hInstance = hInstance;
    wc.lpfnWndProc = WndProc;
    wc.lpszClassName = TEXT("W32OGL");
    wc.lpszMenuName = 0;
    wc.style = CS_HREDRAW | CS_VREDRAW | CS_OWNDC;
    RegisterClass(&wc);

    currentWindow.width = 800;
    currentWindow.height = 600;
    currentWindow.hInstance = hInstance;
    currentWindow.hWnd = CreateWindow(TEXT("W32OGL"), TEXT("W32OGL"), WS_OVERLAPPEDWINDOW, 50, 50, 800, 600, NULL, NULL, hInstance, NULL);
    currentWindow.hdc = GetDC(currentWindow.hWnd);
    ShowWindow(currentWindow.hWnd, iCmdShow);

    PIXELFORMATDESCRIPTOR pfd;
    memset(&pfd, 0, sizeof(pfd));
    pfd.nSize - sizeof(PIXELFORMATDESCRIPTOR);
    pfd.nVersion = 1;
    pfd.dwFlags = PFD_SUPPORT_OPENGL | PFD_DOUBLEBUFFER | PFD_DRAW_TO_WINDOW;
    pfd.iPixelType = PFD_TYPE_RGBA;
    pfd.cColorBits = 24;
    pfd.cDepthBits = 32;

    int chosenPixelFormat = ChoosePixelFormat(currentWindow.hdc, &pfd);
    SetPixelFormat(currentWindow.hdc, chosenPixelFormat, &pfd);
    currentWindow.hglrc = wglCreateContext(currentWindow.hdc);
    wglMakeCurrent(currentWindow.hdc, currentWindow.hglrc);

    MSG msg;
    while(true)
    {
        if(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
        {
            if(msg.message == WM_QUIT)
            {
                break;
            }

            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
        else
        {
            glViewport(0, 0, currentWindow.width, currentWindow.height);
            glMatrixMode(GL_PROJECTION);
            glLoadIdentity();
            gluPerspective(70, (float)currentWindow.width / (float)currentWindow.height, 0.1f, 1000);
            glMatrixMode(GL_MODELVIEW);
            glLoadIdentity();
            gluLookAt(0, 0, 10, 0, 0, 0, 0, 1, 0);
            glClearColor(0, 0, 0, 0);
            glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
            glBegin(GL_TRIANGLES);
            glColor3f(1, 0, 0);
            glVertex3f(-5, -4, 0);
            glColor3f(0, 1, 0);
            glVertex3f(5, -4, 0);
            glColor3f(0, 0, 1);
            glVertex3f(0, 3.5f, 0);
            glEnd();

            SwapBuffers(currentWindow.hdc);
        }
    }

    wglMakeCurrent(NULL, NULL);
    wglDeleteContext(currentWindow.hglrc);
    ReleaseDC(currentWindow.hWnd, currentWindow.hdc);

    return msg.wParam;
}

LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
    switch(message)
    {
        case WM_DESTROY:
        {
            PostQuitMessage(0);
            return 0;
            break;
        }

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

The "SWOGLL.h" include is simply my own custom OpenGL function loader, which is too long to include in this question. It can be found here.

I'd (preferably) like to know a few things:

• Is there anything I can do to reduce CPU usage?
• Am I writing succinct and correct Win32 code? This is my first time using Win32.
• Is it okay to use memset in C++, or is there a more modern and/or safer alternative?
• Is it a good idea to use a while(true) loop here?
• Is the format of my main application loop appropriate?
• Is there anything in general that needs to be improved?

Answer 1

If you have a cpu usage of around 16-20% for that simple example, then you draw you scene more often then required. As forsvarir already said, you should only redraw your scene if something changed and you might want to limit the redraws so that the wont exceed to the refresh rate of the display (e.g. 30, 60, 140 fps)

If an object is std::is_trivial then there should be no problem using memset.

I always prefer something like while(running) in combination with running=false over a while(true) in combination with a break, because it is more verbose about what it is doing.

In c++ you should not use c style castings but static_cast, dynamic_cast or reinterpret_cast What is the difference between static_cast<> and C style casting?

If you don't need to target devices that only support OpenGL with immediate mode, then you should think over using modern OpenGL this will not only reduce the API calls, but will also increase the probability that the driver and gpu can optimise your drawing. This will for sure not have great effect in your simple hello world example, but will be noticeable in real world examples.

Answer 2

memset

memset is generally fine in C++, if you're dealing with structs of basic types / pointers. You need to be careful though if you're start dealing with more complex types (for example if you memset something after it's constructor has been run, you can undo the construction and leave the object in an inconsistent state). If you're dealing with complex types you're better off adding an appropriate constructor to the type (if possible), or assigning values if not.

CPU usage

I've not used open GL and it's been a while since I used core WIN32 APIs, but this looks suspect to me:

while(true)
{
    if(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
    {
        // break on quit
    }
    else
    {
        // Draw
    }
}

The reason it looks suspect is that you're redrawing ever iteration through the loop where there isn't a message to deal with. Typically, you would only redraw either when what you're drawing changes (so if you're doing an animation you redraw the next frame after a given period of time), or when your window has been invalidated and you get a repaint request. As it stands, it seems like you're going to be continually spinning redrawing the image over and over again unnecessarily (which is going to use a lot of CPU).

Rather than using PeekMessage you might find it easier to either split your functionality (have a thread for dealing with the message pump and a thread for dealing with animating your shape/ drawing it at a given interval) or using something like GetMessage with a timeout.