Cache HDC used for painting for the entire duration of WM_PAINT processing.
authorVadim Zeitlin <vadim@wxwidgets.org>
Sat, 10 Nov 2012 00:52:54 +0000 (00:52 +0000)
committerVadim Zeitlin <vadim@wxwidgets.org>
Sat, 10 Nov 2012 00:52:54 +0000 (00:52 +0000)
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
src/msw/dcclient.cpp
src/msw/window.cpp

index b57b5edbada887f0f6d683a36226998a426aafa1..9fb1cca4e7e4a15a703eb193bbaca33fd401fa0e 100644 (file)
 #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);
index 4564a189d1569c5eed1cd7eda41c4040e4890049..112cf781f5993ecf35984c8ba91086dd08c133d2 100644 (file)
@@ -29,6 +29,7 @@
 
 #ifndef WX_PRECOMP
     #include "wx/string.h"
+    #include "wx/hashmap.h"
     #include "wx/log.h"
     #include "wx/window.h"
 #endif
 #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;
+}
index 782d8c442e183891dd76096e86b7f50a59fd4917..1289c4fcb845651b37b493dc0251667a06276c64 100644 (file)
@@ -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;
 }