From ddc8faa9e1f9d7dc99ef11e2294ad33915092024 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 10 Nov 2012 00:52:54 +0000 Subject: [PATCH] Cache HDC used for painting for the entire duration of WM_PAINT processing. This fixes a long standing problem with 2 wxPaintDC created one after another (and not with nested lifetimes, which was handled by the caching mechanism previously used) not working correctly. And as this was exactly what happened when handling wxEVT_PAINT for wxScrolled, it also fixes drawing artefacts when using scrolled windows. Closes #14757. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72938 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/msw/dcclient.h | 20 +-- src/msw/dcclient.cpp | 261 +++++++++++++++++++------------------- src/msw/window.cpp | 2 + 3 files changed, 140 insertions(+), 143 deletions(-) diff --git a/include/wx/msw/dcclient.h b/include/wx/msw/dcclient.h index b57b5edbad..9fb1cca4e7 100644 --- a/include/wx/msw/dcclient.h +++ b/include/wx/msw/dcclient.h @@ -19,16 +19,8 @@ #include "wx/dc.h" #include "wx/msw/dc.h" #include "wx/dcclient.h" -#include "wx/dynarray.h" -// ---------------------------------------------------------------------------- -// array types -// ---------------------------------------------------------------------------- - -// this one if used by wxPaintDC only -struct WXDLLIMPEXP_FWD_CORE wxPaintDCInfo; - -WX_DECLARE_EXPORTED_OBJARRAY(wxPaintDCInfo, wxArrayDCInfo); +class wxPaintDCInfo; // ---------------------------------------------------------------------------- // DC classes @@ -86,11 +78,13 @@ public: // find the entry for this DC in the cache (keyed by the window) static WXHDC FindDCInCache(wxWindow* win); -protected: - static wxArrayDCInfo ms_cache; + // This must be called by the code handling WM_PAINT to remove the DC + // cached for this window for the duration of this message processing. + static void EndPaint(wxWindow *win); - // find the entry for this DC in the cache (keyed by the window) - wxPaintDCInfo *FindInCache(size_t *index = NULL) const; +protected: + // Find the DC for this window in the cache, return NULL if not found. + static wxPaintDCInfo *FindInCache(wxWindow* win); DECLARE_CLASS(wxPaintDCImpl) wxDECLARE_NO_COPY_CLASS(wxPaintDCImpl); diff --git a/src/msw/dcclient.cpp b/src/msw/dcclient.cpp index 4564a189d1..112cf781f5 100644 --- a/src/msw/dcclient.cpp +++ b/src/msw/dcclient.cpp @@ -29,6 +29,7 @@ #ifndef WX_PRECOMP #include "wx/string.h" + #include "wx/hashmap.h" #include "wx/log.h" #include "wx/window.h" #endif @@ -36,37 +37,110 @@ #include "wx/msw/private.h" // ---------------------------------------------------------------------------- -// array/list types +// local data structures // ---------------------------------------------------------------------------- -struct WXDLLEXPORT wxPaintDCInfo +// This is a base class for two concrete subclasses below and contains HDC +// cached for the duration of the WM_PAINT processing together with some +// bookkeeping information. +class wxPaintDCInfo { - wxPaintDCInfo(wxWindow *win, wxPaintDCImpl *dc) +public: + wxPaintDCInfo(HDC hdc) + : m_hdc(hdc) { - hwnd = win->GetHWND(); - hdc = dc->GetHDC(); - count = 1; } - WXHWND hwnd; // window for this DC - WXHDC hdc; // the DC handle - size_t count; // usage count + // The derived class must perform some cleanup. + virtual ~wxPaintDCInfo() = 0; + + WXHDC GetHDC() const { return (WXHDC)m_hdc; } + +protected: + const HDC m_hdc; + + wxDECLARE_NO_COPY_CLASS(wxPaintDCInfo); }; -#include "wx/arrimpl.cpp" +namespace +{ + +// This subclass contains information for the HDCs we create ourselves, i.e. +// those for which we call BeginPaint() -- and hence need to call EndPaint(). +class wxPaintDCInfoOur : public wxPaintDCInfo +{ +public: + wxPaintDCInfoOur(wxWindow* win) + : wxPaintDCInfo(::BeginPaint(GetHwndOf(win), GetPaintStructPtr(m_ps))), + m_hwnd(GetHwndOf(win)) + { + } + + virtual ~wxPaintDCInfoOur() + { + ::EndPaint(m_hwnd, &m_ps); + } -WX_DEFINE_OBJARRAY(wxArrayDCInfo) +private: + // This helper is only needed in order to call it from the ctor initializer + // list. + static PAINTSTRUCT* GetPaintStructPtr(PAINTSTRUCT& ps) + { + wxZeroMemory(ps); + return &ps; + } -// ---------------------------------------------------------------------------- -// macros -// ---------------------------------------------------------------------------- + const HWND m_hwnd; + PAINTSTRUCT m_ps; + + wxDECLARE_NO_COPY_CLASS(wxPaintDCInfoOur); +}; + +// This subclass contains information for the HDCs we receive from outside, as +// WPARAM of WM_PAINT itself. +class wxPaintDCInfoExternal : public wxPaintDCInfo +{ +public: + wxPaintDCInfoExternal(HDC hdc) + : wxPaintDCInfo(hdc), + m_state(::SaveDC(hdc)) + { + } + + virtual ~wxPaintDCInfoExternal() + { + ::RestoreDC(m_hdc, m_state); + } + +private: + const int m_state; + + wxDECLARE_NO_COPY_CLASS(wxPaintDCInfoExternal); +}; + +// The global map containing HDC to use for the given window. The entries in +// this map only exist during WM_PAINT processing and are destroyed when it is +// over. +// +// It is needed because in some circumstances it can happen that more than one +// wxPaintDC is created for the same window during its WM_PAINT handling (and +// as this can happen implicitly, e.g. by calling a function in some library, +// this can be quite difficult to find) but we need to reuse the same HDC for +// all of them because we can't call BeginPaint() more than once. So we cache +// the first HDC created for the window in this map and then reuse it later if +// needed. And, of course, remove it from the map when the painting is done. +WX_DECLARE_HASH_MAP(wxWindow *, wxPaintDCInfo *, + wxPointerHash, wxPointerEqual, + PaintDCInfos); + +PaintDCInfos gs_PaintDCInfos; + +} // anonymous namespace // ---------------------------------------------------------------------------- // global variables // ---------------------------------------------------------------------------- -static PAINTSTRUCT g_paintStruct; - #ifdef wxHAS_PAINT_DEBUG // a global variable which we check to verify that wxPaintDC are only // created in response to WM_PAINT message - doing this from elsewhere is a @@ -186,26 +260,8 @@ void wxClientDCImpl::DoGetSize(int *width, int *height) const // wxPaintDCImpl // ---------------------------------------------------------------------------- -// VZ: initial implementation (by JACS) only remembered the last wxPaintDC -// created and tried to reuse it - this was supposed to take care of a -// situation when a derived class OnPaint() calls base class OnPaint() -// because in this case ::BeginPaint() shouldn't be called second time. -// -// I'm not sure how useful this is, however we must remember the HWND -// associated with the last HDC as well - otherwise we may (and will!) try -// to reuse the HDC for another HWND which is a nice recipe for disaster. -// -// So we store a list of windows for which we already have the DC and not -// just one single hDC. This seems to work, but I'm really not sure about -// the usefullness of the whole idea - IMHO it's much better to not call -// base class OnPaint() at all, or, if we really want to allow it, add a -// "wxPaintDC *" parameter to wxPaintEvent which should be used if it's -// !NULL instead of creating a new DC. - IMPLEMENT_ABSTRACT_CLASS(wxPaintDCImpl, wxClientDCImpl) -wxArrayDCInfo wxPaintDCImpl::ms_cache; - wxPaintDCImpl::wxPaintDCImpl( wxDC *owner ) : wxClientDCImpl( owner ) { @@ -234,17 +290,13 @@ wxPaintDCImpl::wxPaintDCImpl( wxDC *owner, wxWindow *window ) : m_window = window; // do we have a DC for this window in the cache? - wxPaintDCInfo *info = FindInCache(); - if ( info ) + m_hDC = FindDCInCache(m_window); + if ( !m_hDC ) { - m_hDC = info->hdc; - info->count++; - } - else // not in cache, create a new one - { - m_hDC = (WXHDC)::BeginPaint(GetHwndOf(m_window), &g_paintStruct); - if (m_hDC) - ms_cache.Add(new wxPaintDCInfo(m_window, this)); + // not in cache, create a new one + wxPaintDCInfoOur* const info = new wxPaintDCInfoOur(m_window); + gs_PaintDCInfos[m_window] = info; + m_hDC = info->GetHDC(); } // Note: at this point m_hDC can be NULL under MicroWindows, when dragging. @@ -264,77 +316,51 @@ wxPaintDCImpl::~wxPaintDCImpl() if ( m_hDC ) { SelectOldObjects(m_hDC); + m_hDC = 0; + } +} - size_t index; - wxPaintDCInfo *info = FindInCache(&index); - - wxCHECK_RET( info, wxT("existing DC should have a cache entry") ); - if ( --info->count == 0 ) - { - ::EndPaint(GetHwndOf(m_window), &g_paintStruct); +/* static */ +wxPaintDCInfo *wxPaintDCImpl::FindInCache(wxWindow *win) +{ + PaintDCInfos::const_iterator it = gs_PaintDCInfos.find( win ); - ms_cache.RemoveAt(index); + return it != gs_PaintDCInfos.end() ? it->second : NULL; +} - // Reduce the number of bogus reports of non-freed memory - // at app exit - if (ms_cache.IsEmpty()) - ms_cache.Clear(); - } - //else: cached DC entry is still in use +/* static */ +WXHDC wxPaintDCImpl::FindDCInCache(wxWindow* win) +{ + wxPaintDCInfo* const info = FindInCache(win); - // prevent the base class dtor from ReleaseDC()ing it again - m_hDC = 0; - } + return info ? info->GetHDC() : 0; } -wxPaintDCInfo *wxPaintDCImpl::FindInCache(size_t *index) const +/* static */ +void wxPaintDCImpl::EndPaint(wxWindow *win) { - wxPaintDCInfo *info = NULL; - size_t nCache = ms_cache.GetCount(); - for ( size_t n = 0; n < nCache; n++ ) + wxPaintDCInfo *info = FindInCache(win); + if ( info ) { - wxPaintDCInfo *info1 = &ms_cache[n]; - if ( info1->hwnd == m_window->GetHWND() ) - { - info = info1; - if ( index ) - *index = n; - break; - } + gs_PaintDCInfos.erase(win); + delete info; } - - return info; } -// find the entry for this DC in the cache (keyed by the window) -WXHDC wxPaintDCImpl::FindDCInCache(wxWindow* win) +wxPaintDCInfo::~wxPaintDCInfo() { - size_t nCache = ms_cache.GetCount(); - for ( size_t n = 0; n < nCache; n++ ) - { - wxPaintDCInfo *info = &ms_cache[n]; - if ( info->hwnd == win->GetHWND() ) - { - return info->hdc; - } - } - return 0; } /* * wxPaintDCEx */ -// TODO: don't duplicate wxPaintDCImpl code here!! - class wxPaintDCExImpl: public wxPaintDCImpl { public: wxPaintDCExImpl( wxDC *owner, wxWindow *window, WXHDC dc ); ~wxPaintDCExImpl(); - - int m_saveState; }; @@ -348,47 +374,22 @@ wxPaintDCEx::wxPaintDCEx(wxWindow *window, WXHDC dc) wxPaintDCExImpl::wxPaintDCExImpl(wxDC *owner, wxWindow *window, WXHDC dc) : wxPaintDCImpl( owner ) { - wxCHECK_RET( dc, wxT("wxPaintDCEx requires an existing device context") ); - - m_saveState = 0; - - m_window = window; - - wxPaintDCInfo *info = FindInCache(); - if ( info ) - { - m_hDC = info->hdc; - info->count++; - } - else // not in cache, create a new one - { - m_hDC = dc; - ms_cache.Add(new wxPaintDCInfo(m_window, this)); - m_saveState = SaveDC((HDC) dc); - } -} - -wxPaintDCExImpl::~wxPaintDCExImpl() -{ - size_t index; - wxPaintDCInfo *info = FindInCache(&index); + wxCHECK_RET( dc, wxT("wxPaintDCEx requires an existing device context") ); - wxCHECK_RET( info, wxT("existing DC should have a cache entry") ); - - if ( --info->count == 0 ) - { - RestoreDC((HDC) m_hDC, m_saveState); - ms_cache.RemoveAt(index); + m_window = window; - // Reduce the number of bogus reports of non-freed memory - // at app exit - if (ms_cache.IsEmpty()) - ms_cache.Clear(); - } - //else: cached DC entry is still in use + m_hDC = FindDCInCache(m_window); + if ( !m_hDC ) + { + // not in cache, record it there + gs_PaintDCInfos[m_window] = new wxPaintDCInfoExternal(dc); - // prevent the base class dtor from ReleaseDC()ing it again - m_hDC = 0; + m_hDC = dc; + } } - +wxPaintDCExImpl::~wxPaintDCExImpl() +{ + // prevent the base class dtor from ReleaseDC()ing it again + m_hDC = 0; +} diff --git a/src/msw/window.cpp b/src/msw/window.cpp index 782d8c442e..1289c4fcb8 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -4816,6 +4816,8 @@ bool wxWindowMSW::HandlePaint() // be called from inside the event handlers called above) m_updateRegion.Clear(); + wxPaintDCImpl::EndPaint((wxWindow *)this); + return processed; } -- 2.45.2