From 261e7eac73ffcb2e1f2a1ca566f7273bc4a912e8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 11 Nov 2014 01:02:40 +0000 Subject: [PATCH] Add RAII AutoIconInfo class wrapping ICONINFO Windows struct. This ensures that we never forget to delete the handles returned by GetIconInfo() and also centralizes the error message given if it fails in a single place. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@78132 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/msw/private.h | 27 +++++++++++++++++++++++++++ src/common/image.cpp | 12 +++++++----- src/msw/bitmap.cpp | 17 +++++++---------- src/msw/cursor.cpp | 24 ++++++++---------------- src/msw/gdiimage.cpp | 12 ++---------- src/msw/graphics.cpp | 7 ++----- 6 files changed, 53 insertions(+), 46 deletions(-) diff --git a/include/wx/msw/private.h b/include/wx/msw/private.h index de77f43125..c4bf30e918 100644 --- a/include/wx/msw/private.h +++ b/include/wx/msw/private.h @@ -613,6 +613,33 @@ public: operator HRGN() const { return (HRGN)GetObject(); } }; +// Class automatically freeing ICONINFO struct fields after retrieving it using +// GetIconInfo(). +class AutoIconInfo : public ICONINFO +{ +public: + AutoIconInfo() { wxZeroMemory(*this); } + + bool GetFrom(HICON hIcon) + { + if ( !::GetIconInfo(hIcon, this) ) + { + wxLogLastError(wxT("GetIconInfo")); + return false; + } + + return true; + } + + ~AutoIconInfo() + { + if ( hbmColor ) + ::DeleteObject(hbmColor); + if ( hbmMask ) + ::DeleteObject(hbmMask); + } +}; + // class sets the specified clipping region during its life time class HDCClipper { diff --git a/src/common/image.cpp b/src/common/image.cpp index 8cb549b2d9..8e4547aa74 100644 --- a/src/common/image.cpp +++ b/src/common/image.cpp @@ -2375,15 +2375,17 @@ static wxImage LoadImageFromResource(const wxString &name, wxBitmapType type) } else { - ICONINFO info; - if ( !::GetIconInfo(hIcon, &info) ) - { - wxLogLastError(wxT("GetIconInfo")); + AutoIconInfo info; + if ( !info.GetFrom(hIcon) ) return wxImage(); - } hBitmap.Init(info.hbmColor); hMask.Init(info.hbmMask); + + // Reset the fields to prevent them from being destroyed, we took + // ownership of them. + info.hbmColor = + info.hbmMask = 0; } } else if ( type == wxBITMAP_TYPE_CUR_RESOURCE ) diff --git a/src/msw/bitmap.cpp b/src/msw/bitmap.cpp index f5fe48e148..67d4677658 100644 --- a/src/msw/bitmap.cpp +++ b/src/msw/bitmap.cpp @@ -488,13 +488,9 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon, // it may be either HICON or HCURSOR HICON hicon = (HICON)icon.GetHandle(); - ICONINFO iconInfo; - if ( !::GetIconInfo(hicon, &iconInfo) ) - { - wxLogLastError(wxT("GetIconInfo")); - + AutoIconInfo iconInfo; + if ( !iconInfo.GetFrom(hicon) ) return false; - } wxBitmapRefData *refData = new wxBitmapRefData; m_refData = refData; @@ -508,6 +504,10 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon, refData->m_height = h; refData->m_depth = wxDisplayDepth(); refData->m_hBitmap = (WXHBITMAP)iconInfo.hbmColor; + + // Reset this field to prevent it from being destroyed by AutoIconInfo, + // we took ownership of it. + iconInfo.hbmColor = 0; } else // we only have monochrome icon/cursor { @@ -568,7 +568,7 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon, // alpha, so check for this. { HBITMAP hdib = 0; - if ( CheckAlpha(iconInfo.hbmColor, &hdib) ) + if ( CheckAlpha(refData->m_hBitmap, &hdib) ) refData->Set32bppHDIB(hdib); } break; @@ -586,9 +586,6 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon, refData->SetMask(wxInvertMask(iconInfo.hbmMask, w, h)); } - // delete the old one now as we don't need it any more - ::DeleteObject(iconInfo.hbmMask); - return true; #else // __WXMICROWIN__ || __WXWINCE__ wxUnusedVar(icon); diff --git a/src/msw/cursor.cpp b/src/msw/cursor.cpp index de73d96368..5e399713c4 100644 --- a/src/msw/cursor.cpp +++ b/src/msw/cursor.cpp @@ -288,28 +288,20 @@ void ReverseBitmap(HBITMAP bitmap, int width, int height) HCURSOR CreateReverseCursor(HCURSOR cursor) { - ICONINFO info; - if ( !::GetIconInfo(cursor, &info) ) + AutoIconInfo info; + if ( !info.GetFrom(cursor) ) return NULL; - HCURSOR cursorRev = NULL; - BITMAP bmp; - if ( ::GetObject(info.hbmMask, sizeof(bmp), &bmp) ) - { - ReverseBitmap(info.hbmMask, bmp.bmWidth, bmp.bmHeight); - if ( info.hbmColor ) - ReverseBitmap(info.hbmColor, bmp.bmWidth, bmp.bmHeight); - info.xHotspot = (DWORD)bmp.bmWidth - 1 - info.xHotspot; + if ( !::GetObject(info.hbmMask, sizeof(bmp), &bmp) ) + return NULL; - cursorRev = ::CreateIconIndirect(&info); - } - - ::DeleteObject(info.hbmMask); + ReverseBitmap(info.hbmMask, bmp.bmWidth, bmp.bmHeight); if ( info.hbmColor ) - ::DeleteObject(info.hbmColor); + ReverseBitmap(info.hbmColor, bmp.bmWidth, bmp.bmHeight); + info.xHotspot = (DWORD)bmp.bmWidth - 1 - info.xHotspot; - return cursorRev; + return ::CreateIconIndirect(&info); } } // anonymous namespace diff --git a/src/msw/gdiimage.cpp b/src/msw/gdiimage.cpp index cc9c9de4c0..7fe0731c3c 100644 --- a/src/msw/gdiimage.cpp +++ b/src/msw/gdiimage.cpp @@ -663,12 +663,8 @@ wxSize wxGetHiconSize(HICON WXUNUSED_IN_WINCE(hicon)) #ifndef __WXWINCE__ if ( hicon ) { - ICONINFO info; - if ( !::GetIconInfo(hicon, &info) ) - { - wxLogLastError(wxT("GetIconInfo")); - } - else + AutoIconInfo info; + if ( info.GetFrom(hicon) ) { HBITMAP hbmp = info.hbmMask; if ( hbmp ) @@ -678,11 +674,7 @@ wxSize wxGetHiconSize(HICON WXUNUSED_IN_WINCE(hicon)) { size = wxSize(bm.bmWidth, bm.bmHeight); } - - ::DeleteObject(info.hbmMask); } - if ( info.hbmColor ) - ::DeleteObject(info.hbmColor); } } diff --git a/src/msw/graphics.cpp b/src/msw/graphics.cpp index 2527f3c124..a722619d0d 100644 --- a/src/msw/graphics.cpp +++ b/src/msw/graphics.cpp @@ -1736,9 +1736,8 @@ void wxGDIPlusContext::DrawIcon( const wxIcon &icon, wxDouble x, wxDouble y, wxD // the built-in conversion fails when there is alpha in the HICON (eg XP style icons), we can only // find out by looking at the bitmap data whether there really was alpha in it HICON hIcon = (HICON)icon.GetHICON(); - ICONINFO iconInfo ; - // IconInfo creates the bitmaps for color and mask, we must dispose of them after use - if (!GetIconInfo(hIcon,&iconInfo)) + AutoIconInfo iconInfo ; + if (!iconInfo.GetFrom(hIcon)) return; Bitmap interim(iconInfo.hbmColor,NULL); @@ -1788,8 +1787,6 @@ void wxGDIPlusContext::DrawIcon( const wxIcon &icon, wxDouble x, wxDouble y, wxD m_context->DrawImage(image,(REAL) x,(REAL) y,(REAL) w,(REAL) h) ; delete image ; - DeleteObject(iconInfo.hbmColor); - DeleteObject(iconInfo.hbmMask); } void wxGDIPlusContext::DoDrawText(const wxString& str,