Thread safety for taking screenshots on Windows with C++ Builder

Question

Isn't taking a screenshot on Windows thread-safe?

My following code sometimes takes some shots, but in most cases, the imgScreenshot (which is just a TImage) keeps being just plain white...

Am I missing something?

void __fastcall TCaptureThread::Execute()
{
    int CurWidth = 1600;
    int CurHeight = 900;

    std::unique_ptr<TCanvas> Canvas(new TCanvas);
    Canvas->Handle = GetDC(0);    

    FBMP = new TBitmap;                     // private class field
    FBMP->Width     = CurWidth;
    FBMP->Height    = CurHeight;    

    FR = Rect(0, 0, CurWidth, CurHeight);   // private class field

    while(!Terminated)
    {
        FBMP->Canvas->CopyRect(FR, Canvas, FR);       
        Synchronize(&UpdatePicture);

        Sleep(100);                 
    }

    delete FBMP;
    FBMP = NULL;
}

void __fastcall TCaptureThread::UpdatePicture()
{
    FMainForm->imgScreenshot->Canvas->CopyRect(FR, FBMP->Canvas, FR);
}

Environment is C++ Builder 10.1.2 Berlin

Answer 1

Isn't taking a screenshot on Windows thread-safe?

Not when using VCL wrapper classes that are not thread-safe by default. If you were using plain Win32 API functions directly, then yes, it would be possible to write thread-safe code.

The main reason your code fails is because the VCL is designed to share GDI resources between multiple objects, and the main UI thread frequently frees unused/dormant GDI resources. So your worker thread's TBitmap image data is likely to get destroyed before you can call Synchronize() to copy it to your TImage.

That being said, what you are attempting can be done if you call Lock()/Unlock() on the Canvas objects in your worker thread, eg:

struct CanvasLocker
{
    TCanvas *mCanvas;
    CanvasLocker(TCanvas *C) : mCanvas(C) { mCanvas->Lock(); }
    ~CanvasLocker() { mCanvas->Unlock(); }
};

void __fastcall TCaptureThread::Execute()
{
    int CurWidth = 1600;
    int CurHeight = 900;

    std::unique_ptr<TCanvas> Canvas(new TCanvas);
    std::unique_ptr<TBitmap> BMP(new TBitmap);
    FBMP = BMP.get();

    {
    CanvasLocker lock(Canvas); // <-- add this!
    Canvas->Handle = GetDC(0);    
    }

    {
    CanvasLocker lock(BMP->Canvas); // <-- add this!
    BMP->Width = CurWidth;
    BMP->Height = CurHeight;    
    }

    FR = Rect(0, 0, CurWidth, CurHeight);

    while (!Terminated)
    {
        {
        CanvasLocker lock1(Canvas); // <-- add this!
        CanvasLocker lock2(BMP->Canvas); // <-- add this!
        BMP->Canvas->CopyRect(FR, Canvas.get(), FR);
        }

        Synchronize(&UpdatePicture);

        Sleep(100);                 
    }
}

void __fastcall TCaptureThread::UpdatePicture()
{
    CanvasLocker lock1(FBMP->Canvas); // <-- add this!
    CanvasLocker lock2(FMainForm->imgScreenshot->Canvas); // <-- add this!

    FMainForm->imgScreenshot->Canvas->CopyRect(FR, FBMP->Canvas, FR);
    // or: FMainForm->imgScreenshot->Picture->Bitmap->Assign(FBMP);
}

That being said, because TCanvas is lockable, you might be able to get away with removing Synchronize() altogether:

void __fastcall TCaptureThread::Execute()
{
    int CurWidth = 1600;
    int CurHeight = 900;

    std::unique_ptr<TCanvas> Canvas(new TCanvas);
    std::unique_ptr<TBitmap> BMP(new TBitmap);

    {
    CanvasLocker lock(Canvas);
    Canvas->Handle = GetDC(0);    
    }

    {
    CanvasLocker lock(BMP->Canvas);
    BMP->Width = CurWidth;
    BMP->Height = CurHeight;    
    }

    TRect r = Rect(0, 0, CurWidth, CurHeight);

    while (!Terminated)
    {
        {
        CanvasLocker lock1(BMP->Canvas);

        {
        CanvasLocker lock2(Canvas);
        BMP->Canvas->CopyRect(r, Canvas.get(), r);
        }

        CanvasLocker lock3(FMainForm->imgScreenshot->Canvas);
        FMainForm->imgScreenshot->Canvas->CopyRect(r, BMP->Canvas, r);
        // or: FMainForm->imgScreenshot->Picture->Bitmap->Assign(BMP);
        }

        Sleep(100);                 
    }
}