0
votes

I have a program that is supposed to display a count that increments every second. The counter is in a separate thread. I call WindowUpdate when a change has occurred but the count in the window does not update unless I hover the mouse pointer over the window or resize the window. I have tried InvalidateRect and RedrawWindow but they don't work either.

Why won't the counter updates display?

    #ifndef WIN32_LEAN_AND_MEAN
    #define WIN32_LEAN_AND_MEAN
    #endif
    #include <sdkddkver.h>
    #include <Windows.h>

    #include <stdio.h>
    #include <stdlib.h>
    #include <tchar.h>
    #include <strsafe.h>

    typedef struct DataTransfer {
        BOOL Updated;
        int Counter;
    } TRANSFER, *PTRANSFER;

    TRANSFER Payload;
    PTRANSFER pPayload;
    DWORD dwThreadId;
    HANDLE hThread;

    LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
    void DisplayData(HDC hDC);
    void ErrorHandler(LPTSTR lpszFunction);
    DWORD WINAPI Counter(LPVOID lpParam);

    int
    APIENTRY
    wWinMain(
        HINSTANCE hInstance,
        HINSTANCE hPrevInstance,
        LPWSTR lpCmdLine,
        int nShowCmd)
    {
        MSG        msg;  

        WNDCLASSEX wcex;
        ZeroMemory(&wcex, sizeof(wcex));

        wcex.cbSize = sizeof(wcex); 
        wcex.style = CS_HREDRAW | CS_VREDRAW; 
        wcex.lpszClassName = TEXT("MYFIRSTWINDOWCLASS"); 
        wcex.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
        wcex.hCursor = LoadCursor(hInstance, IDC_ARROW); 
        wcex.lpfnWndProc = WndProc;
        wcex.hInstance = hInstance; 

        if (!RegisterClassEx(&wcex))
            return 1;

        CREATESTRUCT cs;
        ZeroMemory(&cs, sizeof(cs));

        cs.x = 0; 
        cs.y = 0;
        cs.cx = 200;
        cs.cy = 300;
        cs.hInstance = hInstance; 
        cs.lpszClass = wcex.lpszClassName; 
        cs.lpszName = TEXT("Test");
        cs.style = WS_OVERLAPPEDWINDOW;


        HWND hWnd = ::CreateWindowEx(
            cs.dwExStyle,
            cs.lpszClass,
            cs.lpszName,
            cs.style,
            cs.x,
            cs.y,
            cs.cx,
            cs.cy,
            cs.hwndParent,
            cs.hMenu,
            cs.hInstance,
            cs.lpCreateParams);

        if (!hWnd)
            return 1;

        DWORD dwThreadId;
        HANDLE hThread;

        pPayload = (PTRANSFER)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(TRANSFER));
        if (pPayload == NULL)
            ExitProcess(2);
        pPayload->Updated = FALSE;
        pPayload->Counter = 0;

        // Display the window.
    ShowWindow(hWnd, SW_SHOWDEFAULT);
    UpdateWindow(hWnd);

    hThread = CreateThread(
        NULL,   
        0,     
        Counter,     
        pPayload,  
        0,       
        &dwThreadId); 
        if (hThread == NULL)
             ExitProcess(2);

        while (1)
        {
            if (pPayload->Updated == TRUE)
            {
    //          InvalidateRect(hWnd, NULL, FALSE);
    //          RedrawWindow(hWnd, NULL, NULL, RDW_INVALIDATE | RDW_UPDATENOW);
                UpdateWindow(hWnd);
            }
            if (GetMessage(&msg, NULL, 0, 0) > 0)
            {
                TranslateMessage(&msg);
                DispatchMessage(&msg);
            }
       }

        ::UnregisterClass(wcex.lpszClassName, hInstance);

        return (int)msg.wParam;
    }

    LRESULT
    CALLBACK
    WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
    {
        PAINTSTRUCT paintStruct;
        HDC hDC;

        switch (uMsg)
        {
        case WM_DESTROY:
            ::PostQuitMessage(0);
            break;
        case WM_PAINT:
            hDC = BeginPaint(hWnd, &paintStruct);
            DisplayData(hDC);
            EndPaint(hWnd, &paintStruct);
            return 0;
            break;
        default:
            return ::DefWindowProc(hWnd, uMsg, wParam, lParam);
        }

        return 0;
    }


    void DisplayData(HDC hDC)
    {
        char OutputStr[32];

        sprintf_s(OutputStr, sizeof(OutputStr) - 1, "%d", pPayload->Counter);
        TextOut(hDC, 100, 100, OutputStr, strlen(OutputStr));
    }

    DWORD WINAPI Counter(LPVOID lpParam)
    {
        PTRANSFER pTransfer;

        pTransfer = (PTRANSFER)lpParam;

        while (1)
        {
            pTransfer->Counter++;
            pTransfer->Updated = TRUE;
            Sleep(1000);
        }
    }

    void ErrorHandler(LPTSTR lpszFunction)
    {
        LPVOID lpMsgBuf;
        LPVOID lpDisplayBuf;
        DWORD dw = GetLastError();

        FormatMessage(
            FORMAT_MESSAGE_ALLOCATE_BUFFER |
            FORMAT_MESSAGE_FROM_SYSTEM |
            FORMAT_MESSAGE_IGNORE_INSERTS,
            NULL,
            dw,
            MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
            (LPTSTR)&lpMsgBuf,
            0, NULL);

        lpDisplayBuf = (LPVOID)LocalAlloc(LMEM_ZEROINIT,
            (lstrlen((LPCTSTR)lpMsgBuf) + lstrlen((LPCTSTR)lpszFunction) + 40) * sizeof(TCHAR));
        StringCchPrintf((LPTSTR)lpDisplayBuf,
            LocalSize(lpDisplayBuf) / sizeof(TCHAR),
            TEXT("%s failed with error %d: %s"),
            lpszFunction, dw, lpMsgBuf);
        MessageBox(NULL, (LPCTSTR)lpDisplayBuf, TEXT("Error"), MB_OK);

        LocalFree(lpMsgBuf);
        LocalFree(lpDisplayBuf);
    }
3
Calling Sleep(1000) will not implement a timer, that ticks at a steady rate. Sleep may expire sooner or later than the requested time period. Instead, implement a single-threaded application and set up a timer (see SetTimer). Also, you never seem to reset pPayload->Updated. The main issue, however, is, that you only ever check Updated after a message has been posted.IInspectable
UpdateWindow by itself won't do anything, you need to invalidate first (which it seems like you have commented out for some reason).Jonathan Potter
You are also not synchronizing accesses to the counter thread's data.Sleep() by itself won't cut it.andlabs

3 Answers

2
votes

(Disclaimer: This answer was written by the seat of my pants. Please correct me if I made an error somewhere.)

You are not doing what you are trying to do properly.

First, as Jonathan Potter has noted, UpdateWindow() by itself will not update the window unless it has an invalid region. The InvalidateRect() call will invalidate a rectangle. So you need both...

...but in reality you don't really want to call UpdateWindow() directly, because it bypasses the Windows drawing model. After calling InvalidateRect(), if there are no pending messages next time GetMessage() is called, and Windows itself decides it's time to refresh the screen contents with new data, you'll get a WM_PAINT message. The system knows when it's best to paint; you'll make your life easier by using it. Only use UpdateWindow() when it is vital that you want the window to redraw right now.

(The point of the invalid region is also due to the Windows drawing model: drawing can be very expensive, so Windows tries to optimize drawing by only redrawing what is needed. Invalidate only the part of your window that needs to be updated, and you'll get better performance.)

But there is a deeper issue: you do not implement multithreading properly.

Your worker thread is generating data every second and overwriting a shared structure. Your window is reading from that shared structure. There is nothing in place to ensure that only one thread is accessing that shared structure at a time. As a result, you'll wind up with ptential mixed state if your worker thread ever grows from a simple integer to a large and complex data structure.

You need to synchronize your data accesses. Communicate between threads.

How? The obvious method is to use a synchronization object, like a mutex. And you can totally do that.

But there's a better way: since one of your threads has a window, just use a window message! Window messages sent using SendMessage() will be received on the window's thread (with the calling thread blocked); messages posted using PostMessage() will be placed on the window's thread's message queue.

Plus you can pass not one but two pieces of information in that message! Both wParam and lParam are pointer-sized integers, so just stuff a pointer in one of them and use SendMessage() or PostMessage()!

But what message do you use? Every* message in the range [WM_USER, WM_APP) is available to the window class to decide what to use it for, and every message in the range [WM_APP, 0xC000) is for the application to decide what to use it for. So just pick one and use it!

Just remember how SendMessage() and PostMessage() work. If the data is allocated on the heap, and each piece of data is allocated separately, then it doesn't matter; if the data is a pointer to a local variable, SendMessage() is the correct answer; if the data is a numeric value that can fit in a pointer-sized integer, either will work. If you need a response, your worker thread will either need a window of its own or the use of SendMessage() to get a value back from the window.

There are lots of ways to do this. It all depends on what data you need to communicate. As the Go programming language's designers say (altered somewhat): don't communicate by sharing data; share data by communicating.


*until you call IsDialogMessage(), at which point you lose WM_USER and WM_USER + 1 (and possibly WM_USER + 2 as well?). But that's only three messages out of over 32,000.

2
votes

Your problem is in your message loop:

    while (1)
    {
        if (pPayload->Updated == TRUE) {
            UpdateWindow(hWnd);
        }
        if (GetMessage(&msg, NULL, 0, 0) > 0) {
            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
    }

GetMessage is synchronous, and doesn't return until a message is available for retrieval. The code sits there, waiting for a message to arrive most of the time, and will not evaluate pPayload->Updated. Consequently, no updates happen, until a message arrives. But even then, nothing happens, since UpdateWindow sends "a WM_PAINT message to the window if the window's update region is not empty." UpdateWindow by itself doesn't do anything useful.1)

You could solve this by reworking your message loop. The following issues need to be addressed:

  • A call to InvalidateRect must precede the call to UpdateWindow.
  • Access to the shared data must be synchronized.
  • The message loop needs to terminate, when GetMessage returns 0 or -1.

None of this is necessary, though. You don't need a background thread. A single-threaded application will do just fine. Just use a Timer to trigger updates to the stored value, and the visual representation. Here is a rough sketch for a window procedure implementing a timer-based solution:

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) {
    static unsigned __int64 startTime = ::GetTickCount64();
    const UINT_PTR timerId = 1;

    switch (uMsg)
    {
    case WM_CREATE:
        // Start timer when window is created
        ::SetTimer(hWnd, timerId, 1000, nullptr);
        return ::DefWindowProc(hWnd, uMsg, wParam, lParam);
    case WM_TIMER: {
        unsigned __int64 currTime = ::GetTickCount64();
        Counter = (currTime - startTime) / 1000;
        // Value changed -> initiate window update
        ::InvalidateRect(hWnd, nullptr, FALSE);
        // Re-start timer
        ::SetTimer(hWnd, timerId, 1000, nullptr);
        }
        break;
    case WM_DESTROY:
        ::KillTimer(hWnd, timerId);
        ::PostQuitMessage(0);
        break;
    case WM_PAINT: {
        PAINTSTRUCT paintStruct = {0};
        HDC hDC = BeginPaint(hWnd, &paintStruct);
        DisplayData(hDC);
        EndPaint(hWnd, &paintStruct);
        }
        return 0;
    default:
        return ::DefWindowProc(hWnd, uMsg, wParam, lParam);
    }

    return 0;
}

A few notes on the implementation:

  • The timer uses a fixed time-out value of 1000 milliseconds. This will occasionally skip an update. The error doesn't accumulate, though, as the Counter is re-evaluated whenever the timer expires. If you more steady updates, calculate the remaining time-out value based on the Counter, startTime, and currTime.
  • Counter is the variable that holds the current time in seconds. It is a replacement for TRANSFER::Counter, that needs to be accessible from both the window procedure and the rendering code.


1)There is another issue with your message loop: It never terminates, and neither does your process. The GUI may disappear, but the process will still show up in Task Manager.
-1
votes

i marked my alterations with comment <<updated instead of setting flag Updated, i changed it to be hWnd, and then update the window instantly and directly from the thread

#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
#include <sdkddkver.h>
#include <Windows.h>

#include <stdio.h>
#include <stdlib.h>
#include <tchar.h>
#include <strsafe.h>

typedef struct DataTransfer {
    //BOOL Updated;   // << updated
    int Counter;
    HWND hWnd;        // << updated
} TRANSFER, *PTRANSFER;

TRANSFER Payload;
PTRANSFER pPayload;
DWORD dwThreadId;
HANDLE hThread;

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
void DisplayData(HDC hDC);
void ErrorHandler(LPTSTR lpszFunction);
DWORD WINAPI Counter(LPVOID lpParam);

int
APIENTRY
wWinMain(
    HINSTANCE hInstance,
    HINSTANCE hPrevInstance,
    LPWSTR lpCmdLine,
    int nShowCmd)
{
    MSG        msg;  

    WNDCLASSEX wcex;
    ZeroMemory(&wcex, sizeof(wcex));

    wcex.cbSize = sizeof(wcex); 
    wcex.style = CS_HREDRAW | CS_VREDRAW; 
    wcex.lpszClassName = TEXT("MYFIRSTWINDOWCLASS"); 
    wcex.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
    wcex.hCursor = LoadCursor(NULL, IDC_ARROW); //<< updated
    wcex.lpfnWndProc = WndProc;
    wcex.hInstance = hInstance; 

    if (!RegisterClassEx(&wcex))
        return 1;

    CREATESTRUCT cs;
    ZeroMemory(&cs, sizeof(cs));

    cs.x = 0; 
    cs.y = 0;
    cs.cx = 200;
    cs.cy = 300;
    cs.hInstance = hInstance; 
    cs.lpszClass = wcex.lpszClassName; 
    cs.lpszName = TEXT("Test");

    cs.style = WS_OVERLAPPEDWINDOW;


    HWND hWnd = CreateWindowEx(
        cs.dwExStyle,
        cs.lpszClass,
        cs.lpszName,
        cs.style,
        cs.x,
        cs.y,
        cs.cx,
        cs.cy,
        cs.hwndParent,
        cs.hMenu,
        cs.hInstance,
        cs.lpCreateParams);

    if (!hWnd)
        return 1;

    DWORD dwThreadId;
    HANDLE hThread;

    pPayload = (PTRANSFER)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(TRANSFER));
    if (pPayload == NULL)
        ExitProcess(2);
    //pPayload->Updated = FALSE; //<< updated
    pPayload->hWnd=hWnd;

    pPayload->Counter = 0;

    // Display the window.
    ShowWindow(hWnd, SW_SHOWDEFAULT);
    UpdateWindow(hWnd);

    hThread = CreateThread( NULL,  0, Counter, pPayload,  0, &dwThreadId); 
    if (hThread == NULL)
         ExitProcess(2);


    while (1)
    {
        //    ____[ updated ]_____
         /*
        if (pPayload->Updated == TRUE)
        {

            UpdateWindow(hWnd);
            pPayload->Updated=FALSE;
        }
        */
        int ret=GetMessage(&msg, NULL, 0, 0);//<< updated
        if(ret==0 || ret==-1)
            break;

        TranslateMessage(&msg);
        DispatchMessage(&msg);

   }

    UnregisterClass(wcex.lpszClassName, hInstance);

    return (int)msg.wParam;
}

LRESULT
CALLBACK
WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    PAINTSTRUCT paintStruct;
    HDC hDC;

    switch (uMsg)
    {
    case WM_DESTROY:
        PostQuitMessage(0);
        break;
    case WM_PAINT:
        hDC = BeginPaint(hWnd, &paintStruct);
        DisplayData(hDC);
        EndPaint(hWnd, &paintStruct);

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

    return 0;
}


void DisplayData(HDC hDC)
{
    char OutputStr[32];

    sprintf_s(OutputStr, sizeof(OutputStr) - 1, "%d", pPayload->Counter);
    TextOut(hDC, 100, 100, OutputStr, strlen(OutputStr));
}

DWORD WINAPI Counter(LPVOID lpParam)
{
    PTRANSFER pTransfer;

    pTransfer = (PTRANSFER)lpParam;

    while (1)
    {
        pTransfer->Counter++;
        InvalidateRect(pTransfer->hWnd,NULL,1);
        Sleep(1000);
    }
}

void ErrorHandler(LPTSTR lpszFunction)
{
    LPVOID lpMsgBuf;
    LPVOID lpDisplayBuf;
    DWORD dw = GetLastError();

    FormatMessage(
        FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_SYSTEM |
        FORMAT_MESSAGE_IGNORE_INSERTS,
        NULL,
        dw,
        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
        (LPTSTR)&lpMsgBuf,
        0, NULL);

    lpDisplayBuf = (LPVOID)LocalAlloc(LMEM_ZEROINIT,
        (lstrlen((LPCTSTR)lpMsgBuf) + lstrlen((LPCTSTR)lpszFunction) + 40) * sizeof(TCHAR));
    StringCchPrintf((LPTSTR)lpDisplayBuf,
        LocalSize(lpDisplayBuf) / sizeof(TCHAR),
        TEXT("%s failed with error %d: %s"),
        lpszFunction, dw, lpMsgBuf);
    MessageBox(NULL, (LPCTSTR)lpDisplayBuf, TEXT("Error"), MB_OK);

    LocalFree(lpMsgBuf);
    LocalFree(lpDisplayBuf);
}