From 452c8dcfa3c8864edaa594520c6f1a1780014072 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 10 Feb 2020 13:39:08 +0100 Subject: [PATCH] Check that wxPaintDC is created correctly in code using wxMSW Creating wxPaintDC for a window outside of any wxEVT_PAINT handler already resulted in assert failures and crash due to using the empty wxDidCreatePaintDC stack, but the assert message was not really clear, so improve it by stating explicitly that wxPaintDC can only be created from wxEVT_PAINT handlers. Also check that wxPaintDC is being created for the correct window: this wasn't detected at all before, but could still result in a lot of grief, so check for this too. Finally, create a new private header with the paint data stack variable declaration instead of using "extern" to declare it manually in wxDC code. --- include/wx/msw/private/paint.h | 39 ++++++++++++++++++++++++++++++++++ src/msw/dcclient.cpp | 11 ++++++---- src/msw/window.cpp | 20 +++++++++-------- 3 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 include/wx/msw/private/paint.h diff --git a/include/wx/msw/private/paint.h b/include/wx/msw/private/paint.h new file mode 100644 index 0000000000..abd0c44cd1 --- /dev/null +++ b/include/wx/msw/private/paint.h @@ -0,0 +1,39 @@ +/////////////////////////////////////////////////////////////////////////////// +// Name: wx/msw/private/paint.h +// Purpose: Helpers for handling repainting +// Author: Vadim Zeitlin +// Created: 2020-02-10 +// Copyright: (c) 2020 Vadim Zeitlin +// Licence: wxWindows licence +/////////////////////////////////////////////////////////////////////////////// + +#ifndef _WX_MSW_PRIVATE_PAINT_H_ +#define _WX_MSW_PRIVATE_PAINT_H_ + +#include "wx/stack.h" + +namespace wxMSWImpl +{ + +// Data used by WM_PAINT handler +struct PaintData +{ + explicit PaintData(wxWindowMSW* window_) + : window(window_), + createdPaintDC(false) + { + } + + // The window being repainted (never null). + wxWindowMSW* const window; + + // True if the user-defined paint handler created wxPaintDC. + bool createdPaintDC; +}; + +// Stack storing data for the possibly nested WM_PAINT handlers. +extern wxStack paintStack; + +} // namespace wxMSWImpl + +#endif // _WX_MSW_PRIVATE_PAINT_H_ diff --git a/src/msw/dcclient.cpp b/src/msw/dcclient.cpp index 2b628ef97d..d178e0e7ed 100644 --- a/src/msw/dcclient.cpp +++ b/src/msw/dcclient.cpp @@ -36,6 +36,7 @@ #include "wx/stack.h" #include "wx/msw/private.h" +#include "wx/msw/private/paint.h" // ---------------------------------------------------------------------------- // local data structures @@ -257,11 +258,13 @@ wxPaintDCImpl::wxPaintDCImpl( wxDC *owner, wxWindow *window ) : { wxCHECK_RET( window, wxT("NULL canvas in wxPaintDCImpl ctor") ); - // see comments in src/msw/window.cpp where this is defined - extern wxStack wxDidCreatePaintDC; - - wxDidCreatePaintDC.top() = true; + using namespace wxMSWImpl; + wxCHECK_RET( !paintStack.empty(), + "wxPaintDC can't be created outside wxEVT_PAINT handler" ); + wxCHECK_RET( paintStack.top().window == window, + "wxPaintDC must be associated with the window being repainted" ); + paintStack.top().createdPaintDC = true; m_window = window; diff --git a/src/msw/window.cpp b/src/msw/window.cpp index f207a8afd7..0d06fcf9a5 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -63,7 +63,6 @@ #include "wx/popupwin.h" #include "wx/power.h" #include "wx/scopeguard.h" -#include "wx/stack.h" #include "wx/sysopt.h" #if wxUSE_DRAG_AND_DROP @@ -81,6 +80,7 @@ #include "wx/msw/private.h" #include "wx/msw/private/keyboard.h" +#include "wx/msw/private/paint.h" #include "wx/msw/private/winstyle.h" #include "wx/msw/dcclient.h" #include "wx/msw/seh.h" @@ -5240,8 +5240,7 @@ wxColour wxWindowMSW::MSWGetThemeColour(const wchar_t *themeName, // endless stream of WM_PAINT messages for this window resulting in a lot of // difficult to debug problems (e.g. impossibility to repaint other windows, // lack of timer and idle events and so on) -extern wxStack wxDidCreatePaintDC; -wxStack wxDidCreatePaintDC; +wxStack wxMSWImpl::paintStack; bool wxWindowMSW::HandlePaint() { @@ -5257,14 +5256,17 @@ bool wxWindowMSW::HandlePaint() m_updateRegion = wxRegion((WXHRGN) hRegion); - wxDidCreatePaintDC.push(false); + using namespace wxMSWImpl; + + paintStack.push(PaintData(this)); wxPaintEvent event(m_windowId); event.SetEventObject(this); bool processed = HandleWindowEvent(event); - if ( wxDidCreatePaintDC.top() && !processed ) + const bool createdPaintDC = paintStack.top().createdPaintDC; + if ( createdPaintDC && !processed ) { // Event handler did paint something as wxPaintDC object was created // but then it must have skipped the event to indicate that default @@ -5288,16 +5290,16 @@ bool wxWindowMSW::HandlePaint() wxPaintDCImpl::EndPaint((wxWindow *)this); + paintStack.pop(); + // It doesn't matter whether the event was actually processed or not here, // what matters is whether we already painted, and hence validated, the // window or not. If we did, either the event was processed or we called // OnPaint() above, so we should return true. If we did not, even the event // was processed, we must still call MSWDefWindowProc() to ensure that the // window is validated, i.e. to avoid the problem described in the comment - // before wxDidCreatePaintDC definition above. - const bool ret = wxDidCreatePaintDC.top(); - wxDidCreatePaintDC.pop(); - return ret; + // before paintStack definition above. + return createdPaintDC; } // Can be called from an application's OnPaint handler