1. fixed (to test) the bug with bitmaps without masks in wxImageList

2. reorganized wxImageList a bit, created a new wxInvertMask() function
3. an incredibly ugly fix (?) for "unsatisfied constraints" warnings
4. added wxIcon and wxBitmap ctors from XPM
5. XPM handler now creates bitmaps with mask
6. added wxPrinterDC::BitBlt() and DrawBitmap(), cleared the horrible mess in
   the wxDC methods with the same names


git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@5571 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/layout.h

@@ -177,8 +177,8 @@ public:
     bool SatisfyConstraints(wxWindowBase *win, int *noChanges);
     bool AreSatisfied() const
     {
-        return left.GetDone() && top.GetDone() && right.GetDone() &&
+        return left.GetDone() && top.GetDone() &&
-               bottom.GetDone() && centreX.GetDone() && centreY.GetDone();
+               width.GetDone() && height.GetDone();
     }
 };
 

include/wx/log.h

@@ -482,13 +482,13 @@ DECLARE_LOG_FUNCTION2(SysError, long lErrCode);
 #ifdef __VISUALC__
     #define wxLogApiError(api, rc)                                            \
         wxLogDebug(wxT("%s(%d): '%s' failed with error 0x%08lx (%s)."),       \
-                   __TFILE__, __LINE__, api,                                    \
+                   __TFILE__, __LINE__, _T(api),                              \
                    rc, wxSysErrorMsg(rc))
 #else // !VC++
     #define wxLogApiError(api, rc)                                            \
         wxLogDebug(wxT("In file %s at line %d: '%s' failed with "             \
                       "error 0x%08lx (%s)."),                                 \
-                   __TFILE__, __LINE__, api,                                    \
+                   __TFILE__, __LINE__, _T(api),                              \
                    rc, wxSysErrorMsg(rc))
 #endif // VC++/!VC++
 

include/wx/msw/bitmap.h

@@ -78,7 +78,8 @@ public:
     wxBitmap(const char bits[], int width, int height, int depth = 1);
 
     // Initialize with XPM data
-    wxBitmap(char **data, wxControl *anItem = NULL);
+    wxBitmap(const char **data) { CreateFromXpm(data); }
+    wxBitmap(char **data) { CreateFromXpm((const char **)data); }
 
     // Load a file or resource
     wxBitmap(const wxString& name, long type = wxBITMAP_TYPE_BMP_RESOURCE);
@@ -116,7 +117,7 @@ public:
 
     virtual ~wxBitmap();
 
-    // GRG, Dic/99
+    // get the given part of bitmap
     wxBitmap GetSubBitmap( const wxRect& rect ) const;
  
     // copies the contents and mask of the given (colour) icon to the bitmap
@@ -182,6 +183,9 @@ protected:
     virtual wxGDIImageRefData *CreateData() const
         { return new wxBitmapRefData; }
 
+    // creates the bitmap from XPM data, supposed to be called from ctor
+    bool CreateFromXpm(const char **bits);
+
 private:
 #ifdef __WIN32__
     // common part of CopyFromIcon/CopyFromCursor for Win32

include/wx/msw/dc.h

@@ -138,6 +138,9 @@ public:
         m_bOwnsDC = bOwnsDC;
     }
 
+    const wxBitmap& GetSelectedBitmap() const { return m_selectedBitmap; }
+    wxBitmap& GetSelectedBitmap() { return m_selectedBitmap; }
+
 protected:
     virtual void DoFloodFill(wxCoord x, wxCoord y, const wxColour& col,
                              int style = wxFLOOD_SURFACE);

include/wx/msw/dcprint.h

@@ -1,5 +1,5 @@
 /////////////////////////////////////////////////////////////////////////////
-// Name:        dcprint.h
+// Name:        wx/msw/dcprint.h
 // Purpose:     wxPrinterDC class
 // Author:      Julian Smart
 // Modified by:
@@ -13,7 +13,7 @@
 #define _WX_DCPRINT_H_
 
 #ifdef __GNUG__
-#pragma interface "dcprint.h"
+    #pragma interface "dcprint.h"
 #endif
 
 #if wxUSE_PRINTING_ARCHITECTURE
@@ -36,12 +36,20 @@ DECLARE_CLASS(wxPrinterDC)
 
     ~wxPrinterDC(void);
 
-    bool StartDoc(const wxString& message);
+    // override some base class virtuals
-    void EndDoc(void);
+    virtual bool StartDoc(const wxString& message);
-    void StartPage(void);
+    virtual void EndDoc();
-    void EndPage(void);
+    virtual void StartPage();
+    virtual void EndPage();
 
 protected:
+    virtual void DoDrawBitmap(const wxBitmap &bmp, wxCoord x, wxCoord y,
+                              bool useMask = FALSE);
+    virtual bool DoBlit(wxCoord xdest, wxCoord ydest,
+                        wxCoord width, wxCoord height,
+                        wxDC *source, wxCoord xsrc, wxCoord ysrc,
+                        int rop = wxCOPY, bool useMask = FALSE);
+
     wxPrintData m_printData;
 };
 

include/wx/msw/icon.h

@@ -60,15 +60,23 @@ public:
 class WXDLLEXPORT wxIcon : public wxIconBase
 {
 public:
-    wxIcon();
+    // ctors
+        // default
+    wxIcon() { }
 
-    // Copy constructors
+        // copy
     wxIcon(const wxIcon& icon) { Ref(icon); }
 
+        // from raw data
     wxIcon(const char bits[], int width, int height);
+        // from XPM data
+    wxIcon(const char **data) { CreateIconFromXpm(data); }
+    wxIcon(char **data) { CreateIconFromXpm((const char **)data); }
+        // from resource/file
     wxIcon(const wxString& name,
            long type = wxBITMAP_TYPE_ICO_RESOURCE,
            int desiredWidth = -1, int desiredHeight = -1);
+
     virtual ~wxIcon();
 
     virtual bool LoadFile(const wxString& name,
@@ -93,6 +101,12 @@ protected:
         return new wxIconRefData;
     }
 
+    // create from XPM data
+    void CreateIconFromXpm(const char **data);
+
+    // create from bitmap (which should have a mask unless it's monochrome)
+    void CopyFromBitmap(const wxBitmap& bmp);
+
 private:
     DECLARE_DYNAMIC_CLASS(wxIcon)
 };

include/wx/msw/private.h

@@ -231,6 +231,10 @@ inline void wxRGBToColour(wxColour& c, COLORREF rgb)
 extern void HIMETRICToPixel(LONG *x, LONG *y);
 extern void PixelToHIMETRIC(LONG *x, LONG *y);
 
+// Windows convention of the mask is opposed to the wxWindows one, so we need
+// to invert the mask each time we pass one/get one to/from Windows
+extern HBITMAP wxInvertMask(HBITMAP hbmpMask, int w = 0, int h = 0);
+
 // ---------------------------------------------------------------------------
 // small helper classes
 // ---------------------------------------------------------------------------

include/wx/window.h

@@ -617,7 +617,7 @@ public:
     virtual void SetConstraintSizes(bool recurse = TRUE);
     virtual bool LayoutPhase1(int *noChanges);
     virtual bool LayoutPhase2(int *noChanges);
-    virtual bool DoPhase(int);
+    virtual bool DoPhase(int phase);
 
         // these methods are virtual but normally won't be overridden
     virtual void SetSizeConstraint(int x, int y, int w, int h);

src/common/wincmn.cpp

@@ -932,10 +932,25 @@ bool wxWindowBase::Layout()
     }
     else
     {
-        // Evaluate child constraints
+        wxLayoutConstraints *constr = GetConstraints();
+        bool wasOk = constr && constr->AreSatisfied();
+
         ResetConstraints();   // Mark all constraints as unevaluated
-        DoPhase(1);           // Just one phase need if no sizers involved
+
-        DoPhase(2);
+        // if we're a top level panel (i.e. our parent is frame/dialog), our
+        // own constraints will never be satisfied any more unless we do it
+        // here
+        if ( wasOk )
+        {
+            int noChanges = 1;
+            while ( noChanges > 0 )
+            {
+                constr->SatisfyConstraints(this, &noChanges);
+            }
+        }
+
+        DoPhase(1);           // Layout children
+        DoPhase(2);           // Layout grand children
         SetConstraintSizes(); // Recursively set the real window sizes
     }
 
@@ -1039,8 +1054,7 @@ void wxWindowBase::ResetConstraints()
 void wxWindowBase::SetConstraintSizes(bool recurse)
 {
     wxLayoutConstraints *constr = GetConstraints();
-    if ( constr && constr->left.GetDone() && constr->right.GetDone( ) &&
+    if ( constr && constr->AreSatisfied() )
-            constr->width.GetDone() && constr->height.GetDone())
     {
         int x = constr->left.GetValue();
         int y = constr->top.GetValue();
@@ -1060,12 +1074,9 @@ void wxWindowBase::SetConstraintSizes(bool recurse)
     }
     else if ( constr )
     {
-        wxString winName = GetName();
+        wxLogDebug(wxT("Constraints not satisfied for %s named '%s'."),
-        if ( !winName )
-            winName = wxT("unnamed");
-        wxLogDebug(wxT("Constraint not satisfied for %s, name '%s'."),
                    GetClassInfo()->GetClassName(),
-                   winName.c_str());
+                   GetName().c_str());
     }
 
     if ( recurse )

src/msw/bitmap.cpp

@@ -50,10 +50,10 @@
 // macros
 // ----------------------------------------------------------------------------
 
-    IMPLEMENT_DYNAMIC_CLASS(wxBitmap, wxGDIObject)
+IMPLEMENT_DYNAMIC_CLASS(wxBitmap, wxGDIObject)
-    IMPLEMENT_DYNAMIC_CLASS(wxMask, wxObject)
+IMPLEMENT_DYNAMIC_CLASS(wxMask, wxObject)
 
-    IMPLEMENT_DYNAMIC_CLASS(wxBitmapHandler, wxObject)
+IMPLEMENT_DYNAMIC_CLASS(wxBitmapHandler, wxObject)
 
 // ============================================================================
 // implementation
@@ -131,26 +131,8 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon)
 
     // the mask returned by GetIconInfo() is inversed compared to the usual
     // wxWin convention
-    HBITMAP hbmpMask = ::CreateBitmap(w, h, 1, 1, 0);
+    refData->m_bitmapMask = new wxMask((WXHBITMAP)
-
+                                        wxInvertMask(iconInfo.hbmMask, w, h));
-    // the icons mask is opposite to the usual wxWin convention
-    HDC dcSrc = ::CreateCompatibleDC(NULL);
-    HDC dcDst = ::CreateCompatibleDC(NULL);
-    (void)SelectObject(dcSrc, iconInfo.hbmMask);
-    (void)SelectObject(dcDst, hbmpMask);
-
-    HBRUSH brush = ::CreateSolidBrush(RGB(255, 255, 255));
-    RECT rect = { 0, 0, w, h };
-    FillRect(dcDst, &rect, brush);
-
-    BitBlt(dcDst, 0, 0, w, h, dcSrc, 0, 0, SRCINVERT);
-
-    SelectObject(dcDst, NULL);
-    SelectObject(dcSrc, NULL);
-    DeleteDC(dcDst);
-    DeleteDC(dcSrc);
-
-    refData->m_bitmapMask = new wxMask((WXHBITMAP)hbmpMask);
 
 #if WXWIN_COMPATIBILITY_2
     refData->m_ok = TRUE;
@@ -293,52 +275,12 @@ wxBitmap::wxBitmap(const char bits[], int width, int height, int depth)
     SetHBITMAP((WXHBITMAP)hbmp);
 }
 
-// GRG, Dic/99
-wxBitmap wxBitmap::GetSubBitmap( const wxRect& rect) const
-{
-    wxCHECK_MSG( Ok() &&
-                 (rect.x >= 0) && (rect.y >= 0) &&
-                 (rect.x+rect.width <= GetWidth()) &&
-                 (rect.y+rect.height <= GetHeight()),
-                 wxNullBitmap, wxT("Invalid bitmap or bitmap region") );
-
-    wxBitmap ret( rect.width, rect.height, GetDepth() );
-    wxASSERT_MSG( ret.Ok(), wxT("GetSubBitmap error") );
-
-    // copy bitmap data
-    HDC dcSrc = ::CreateCompatibleDC(NULL);
-    HDC dcDst = ::CreateCompatibleDC(NULL);
-    SelectObject(dcSrc, (HBITMAP) GetHBITMAP());
-    SelectObject(dcDst, (HBITMAP) ret.GetHBITMAP());
-    BitBlt(dcDst, 0, 0, rect.width, rect.height, dcSrc, rect.x, rect.y, SRCCOPY);
-
-    // copy mask if there is one
-    if (GetMask())
-    {
-        HBITMAP hbmpMask = ::CreateBitmap(rect.width, rect.height, 1, 1, 0);
-
-        SelectObject(dcSrc, (HBITMAP) GetMask()->GetMaskBitmap());
-        SelectObject(dcDst, (HBITMAP) hbmpMask);
-        BitBlt(dcDst, 0, 0, rect.width, rect.height, dcSrc, rect.x, rect.y, SRCCOPY);
-
-        wxMask *mask = new wxMask((WXHBITMAP) hbmpMask);
-        ret.SetMask(mask);
-    }
-
-    SelectObject(dcDst, NULL);
-    SelectObject(dcSrc, NULL);
-    DeleteDC(dcDst);
-    DeleteDC(dcSrc);
-
-    return ret;
-}
-
 // Create from XPM data
-wxBitmap::wxBitmap(char **data, wxControl *WXUNUSED(anItem))
+bool wxBitmap::CreateFromXpm(const char **data)
 {
     Init();
 
-    (void)Create((void *)data, wxBITMAP_TYPE_XPM_DATA, 0, 0, 0);
+    return Create((void *)data, wxBITMAP_TYPE_XPM_DATA, 0, 0, 0);
 }
 
 wxBitmap::wxBitmap(int w, int h, int d)
@@ -465,6 +407,49 @@ bool wxBitmap::SaveFile(const wxString& filename, int type, const wxPalette *pal
     }
 }
 
+// ----------------------------------------------------------------------------
+// sub bitmap extraction
+// ----------------------------------------------------------------------------
+
+wxBitmap wxBitmap::GetSubBitmap( const wxRect& rect) const
+{
+    wxCHECK_MSG( Ok() &&
+                 (rect.x >= 0) && (rect.y >= 0) &&
+                 (rect.x+rect.width <= GetWidth()) &&
+                 (rect.y+rect.height <= GetHeight()),
+                 wxNullBitmap, wxT("Invalid bitmap or bitmap region") );
+
+    wxBitmap ret( rect.width, rect.height, GetDepth() );
+    wxASSERT_MSG( ret.Ok(), wxT("GetSubBitmap error") );
+
+    // copy bitmap data
+    HDC dcSrc = ::CreateCompatibleDC(NULL);
+    HDC dcDst = ::CreateCompatibleDC(NULL);
+    SelectObject(dcSrc, (HBITMAP) GetHBITMAP());
+    SelectObject(dcDst, (HBITMAP) ret.GetHBITMAP());
+    BitBlt(dcDst, 0, 0, rect.width, rect.height, dcSrc, rect.x, rect.y, SRCCOPY);
+
+    // copy mask if there is one
+    if (GetMask())
+    {
+        HBITMAP hbmpMask = ::CreateBitmap(rect.width, rect.height, 1, 1, 0);
+
+        SelectObject(dcSrc, (HBITMAP) GetMask()->GetMaskBitmap());
+        SelectObject(dcDst, (HBITMAP) hbmpMask);
+        BitBlt(dcDst, 0, 0, rect.width, rect.height, dcSrc, rect.x, rect.y, SRCCOPY);
+
+        wxMask *mask = new wxMask((WXHBITMAP) hbmpMask);
+        ret.SetMask(mask);
+    }
+
+    SelectObject(dcDst, NULL);
+    SelectObject(dcSrc, NULL);
+    DeleteDC(dcDst);
+    DeleteDC(dcSrc);
+
+    return ret;
+}
+
 // ----------------------------------------------------------------------------
 // wxBitmap accessors
 // ----------------------------------------------------------------------------
@@ -506,7 +491,7 @@ void wxBitmap::SetMask(wxMask *mask)
 wxBitmap wxBitmap::GetBitmapForDC(wxDC& dc) const
 {
     wxMemoryDC      memDC;
-    wxBitmap        tmpBitmap(this->GetWidth(), this->GetHeight(), dc.GetDepth());
+    wxBitmap        tmpBitmap(GetWidth(), GetHeight(), dc.GetDepth());
     HPALETTE        hPal = (HPALETTE) NULL;
     LPBITMAPINFO    lpDib;
     void            *lpBits = (void*) NULL;
@@ -643,38 +628,59 @@ bool wxMask::Create(const wxBitmap& bitmap, int paletteIndex)
 // the transparent area
 bool wxMask::Create(const wxBitmap& bitmap, const wxColour& colour)
 {
+    wxCHECK_MSG( bitmap.Ok(), FALSE, _T("invalid bitmap in wxMask::Create") );
+
     if ( m_maskBitmap )
     {
         ::DeleteObject((HBITMAP) m_maskBitmap);
         m_maskBitmap = 0;
     }
-    if (!bitmap.Ok())
+
+    int width = bitmap.GetWidth(),
+        height = bitmap.GetHeight();
+
+    // scan the bitmap for the transparent colour and set the corresponding
+    // pixels in the mask to BLACK and the rest to WHITE
+    COLORREF maskColour = wxColourToRGB(colour);
+    m_maskBitmap = (WXHBITMAP)::CreateBitmap(width, height, 1, 1, 0);
+
+    HDC srcDC = ::CreateCompatibleDC(NULL);
+    HDC destDC = ::CreateCompatibleDC(NULL);
+    if ( !srcDC || !destDC )
     {
+        wxLogLastError("CreateCompatibleDC");
+    }
+
+    if ( !::SelectObject(srcDC, GetHbitmapOf(bitmap)) )
+    {
+        wxLogLastError("SelectObject");
+    }
+    if ( !::SelectObject(destDC, (HBITMAP)m_maskBitmap) )
+    {
+        wxLogLastError("SelectObject");
+    }
+
+    // this is not very efficient, but I can't think of a better way of doing
+    // it
+    for ( int w = 0; w < width; w++ )
+    {
+        for ( int h = 0; h < height; h++ )
+        {
+            COLORREF col = GetPixel(srcDC, w, h);
+            if ( col == CLR_INVALID )
+            {
+                wxLogLastError("GetPixel");
+
+                // doesn't make sense to continue
+                ::SelectObject(srcDC, 0);
+                ::DeleteDC(srcDC);
+                ::SelectObject(destDC, 0);
+                ::DeleteDC(destDC);
+
                 return FALSE;
             }
 
-    // scan the bitmap for the transparent colour and set
+            if ( col == maskColour )
-    // the corresponding pixels in the mask to BLACK and
-    // the rest to WHITE
-    COLORREF maskColour = RGB(colour.Red(), colour.Green(), colour.Blue());
-    m_maskBitmap = (WXHBITMAP) ::CreateBitmap(
-            bitmap.GetWidth(),
-            bitmap.GetHeight(),
-            1, 1, 0
-                                             );
-    HDC srcDC = ::CreateCompatibleDC(0);
-    ::SelectObject(srcDC, (HBITMAP) bitmap.GetHBITMAP());
-    HDC destDC = ::CreateCompatibleDC(0);
-    ::SelectObject(destDC, (HBITMAP) m_maskBitmap);
-
-    // this is not very efficient, but I can't think
-    // of a better way of doing it
-    for (int w = 0; w < bitmap.GetWidth(); w++)
-    {
-        for (int h = 0; h < bitmap.GetHeight(); h++)
-        {
-            COLORREF col = GetPixel(srcDC, w, h);
-            if (col == maskColour)
             {
                 ::SetPixel(destDC, w, h, RGB(0, 0, 0));
             }
@@ -684,10 +690,12 @@ bool wxMask::Create(const wxBitmap& bitmap, const wxColour& colour)
             }
         }
     }
+
     ::SelectObject(srcDC, 0);
     ::DeleteDC(srcDC);
     ::SelectObject(destDC, 0);
     ::DeleteDC(destDC);
+
     return TRUE;
 }
 
@@ -803,4 +811,47 @@ void wxFreeDIB(LPBITMAPINFO lpDIBHeader)
     free(lpDIBHeader);
 }
 
+// ----------------------------------------------------------------------------
+// other helper functions
+// ----------------------------------------------------------------------------
 
+extern HBITMAP wxInvertMask(HBITMAP hbmpMask, int w, int h)
+{
+    wxCHECK_MSG( hbmpMask, 0, _T("invalid bitmap in wxInvertMask") );
+
+    // get width/height from the bitmap if not given
+    if ( !w || !h )
+    {
+        BITMAP bm;
+        ::GetObject(hbmpMask, sizeof(BITMAP), (LPVOID)&bm);
+        w = bm.bmWidth;
+        h = bm.bmHeight;
+    }
+
+    HDC hdcSrc = ::CreateCompatibleDC(NULL);
+    HDC hdcDst = ::CreateCompatibleDC(NULL);
+    if ( !hdcSrc || !hdcDst )
+    {
+        wxLogLastError("CreateCompatibleDC");
+    }
+
+    HBITMAP hbmpInvMask = ::CreateBitmap(w, h, 1, 1, 0);
+    if ( !hbmpInvMask )
+    {
+        wxLogLastError("CreateBitmap");
+    }
+
+    ::SelectObject(hdcSrc, hbmpMask);
+    ::SelectObject(hdcDst, hbmpInvMask);
+    if ( !::BitBlt(hdcDst, 0, 0, w, h,
+                   hdcSrc, 0, 0,
+                   NOTSRCCOPY) )
+    {
+        wxLogLastError("BitBlt");
+    }
+
+    ::DeleteDC(hdcSrc);
+    ::DeleteDC(hdcDst);
+
+    return hbmpInvMask;
+}

src/msw/dc.cpp

@@ -583,68 +583,22 @@ void wxDC::DoDrawEllipticArc(wxCoord x,wxCoord y,wxCoord w,wxCoord h,double sa,d
 
 void wxDC::DoDrawIcon(const wxIcon& icon, wxCoord x, wxCoord y)
 {
-#if defined(__WIN32__) && !defined(__SC__) && !defined(__TWIN32__)
+    wxCHECK_RET( icon.Ok(), wxT("invalid icon in DrawIcon") );
-    ::DrawIconEx(GetHdc(), XLOG2DEV(x), YLOG2DEV(y), (HICON) icon.GetHICON(),
+
-        icon.GetWidth(), icon.GetHeight(), 0, 0, DI_NORMAL);
+    ::DrawIcon(GetHdc(), XLOG2DEV(x), YLOG2DEV(y), GetHiconOf(icon));
-#else
-    ::DrawIcon(GetHdc(), XLOG2DEV(x), YLOG2DEV(y), (HICON) icon.GetHICON());
-#endif
 
     CalcBoundingBox(x, y);
-    CalcBoundingBox(x+icon.GetWidth(), y+icon.GetHeight());
+    CalcBoundingBox(x + icon.GetWidth(), y + icon.GetHeight());
 }
 
 void wxDC::DoDrawBitmap( const wxBitmap &bmp, wxCoord x, wxCoord y, bool useMask )
 {
-    if (!bmp.Ok())
+    wxCHECK_RET( bmp.Ok(), _T("invalid bitmap in wxDC::DrawBitmap") );
-        return;
 
-    bool needsPixelCopy = FALSE ;
+    int width = bmp.GetWidth(),
-    bool isPrinter = FALSE ;
+        height = bmp.GetHeight();
-    if (IsKindOf(CLASSINFO(wxPrinterDC)) )
-    {
-        isPrinter = TRUE ;
-        if ( ::GetDeviceCaps((HDC) m_hDC, RASTERCAPS) & RC_STRETCHDIB )
-       	{
-        }
-       	else
-       	{
-            needsPixelCopy = TRUE ;
-       	}
-    }
-    // If we're not drawing transparently, and not drawing to a printer,
-    // optimize this function to use Windows functions.
-    if (!useMask && !needsPixelCopy)
-    {
-        if ( isPrinter )
-        {
-            BITMAPINFO *info = (BITMAPINFO *) malloc( sizeof( BITMAPINFOHEADER ) + 256 * sizeof(RGBQUAD ) ) ;
-            int iBitsSize = ((bmp.GetWidth() + 3 ) & ~3 ) * bmp.GetHeight() ;
 
-            void* bits = malloc( iBitsSize ) ;
+    if ( !useMask )
-            
-            memset( info , 0 , sizeof( BITMAPINFOHEADER ) ) ;
-            
-            info->bmiHeader.biSize = sizeof( BITMAPINFOHEADER ) ;
-            info->bmiHeader.biWidth = bmp.GetWidth() ;
-            info->bmiHeader.biHeight = bmp.GetHeight() ;
-            info->bmiHeader.biPlanes = 1 ;
-            info->bmiHeader.biBitCount = 8 ;
-            info->bmiHeader.biCompression = BI_RGB ;
-            
-            HDC display = GetDC( NULL ) ;
-            if ( GetDIBits( display , (HBITMAP) bmp.GetHBITMAP( ) , 0 , bmp.GetHeight() , bits , info , DIB_RGB_COLORS ) )
-            {
-                StretchDIBits( (HDC) m_hDC, 
-                    x, y, bmp.GetWidth(), bmp.GetHeight() , 
-                    0 , 0 ,bmp.GetWidth(), bmp.GetHeight() , 
-                    bits , info , DIB_RGB_COLORS , SRCCOPY ) ;
-            }
-            ReleaseDC( NULL , display ) ;
-            free ( bits ) ;
-            free( info ) ;
-        }
-        else
     {
         HDC cdc = GetHdc();
         HDC memdc = ::CreateCompatibleDC( cdc );
@@ -664,30 +618,19 @@ void wxDC::DoDrawBitmap( const wxBitmap &bmp, wxCoord x, wxCoord y, bool useMask
         }
 
         ::SelectObject( memdc, hbitmap );
-            ::BitBlt( cdc, x, y, bmp.GetWidth(), bmp.GetHeight(), memdc, 0, 0, SRCCOPY);
+        ::BitBlt( cdc, x, y, width, height, memdc, 0, 0, SRCCOPY);
         ::DeleteDC( memdc );
 
         ::SetTextColor(GetHdc(), old_textground);
         ::SetBkColor(GetHdc(), old_background);
     }
-    }
     else
     {
         // Rather than reproduce wxDC::Blit, let's do it at the wxWin API level
         wxMemoryDC memDC;
         memDC.SelectObject(bmp);
 
-        /* Not sure if we need this. The mask should leave the
+        Blit(x, y, width, height, &memDC, 0, 0, wxCOPY, useMask);
-        * masked areas as per the original background of this DC.
-        */
-        /*
-        // There might be transparent areas, so make these
-        // the same colour as this DC
-        memDC.SetBackground(* GetBackground());
-        memDC.Clear();
-        */
-        
-        Blit(x, y, bmp.GetWidth(), bmp.GetHeight(), & memDC, 0, 0, wxCOPY, useMask);
 
         memDC.SelectObject(wxNullBitmap);
     }
@@ -1237,13 +1180,21 @@ wxCoord wxDCBase::LogicalToDeviceYRel(wxCoord y) const
 // ---------------------------------------------------------------------------
 // bit blit
 // ---------------------------------------------------------------------------
-bool wxDC::DoBlit(wxCoord xdest, wxCoord ydest, wxCoord width, wxCoord height,
+
-                  wxDC *source, wxCoord xsrc, wxCoord ysrc, int rop, bool useMask)
+bool wxDC::DoBlit(wxCoord xdest, wxCoord ydest,
+                  wxCoord width, wxCoord height,
+                  wxDC *source, wxCoord xsrc, wxCoord ysrc,
+                  int rop, bool useMask)
 {
-    wxCoord xdest1 = xdest;
+    wxMask *mask = NULL;
-    wxCoord ydest1 = ydest;
+    if ( useMask )
-    wxCoord xsrc1 = xsrc;
+    {
-    wxCoord ysrc1 = ysrc;
+        const wxBitmap& bmp = source->m_selectedBitmap;
+        mask = bmp.GetMask();
+
+        wxCHECK_MSG( bmp.Ok() && mask, FALSE,
+                     _T("can't blit with mask without mask") );
+    }
 
     COLORREF old_textground = ::GetTextColor(GetHdc());
     COLORREF old_background = ::GetBkColor(GetHdc());
@@ -1270,106 +1221,74 @@ bool wxDC::DoBlit(wxCoord xdest, wxCoord ydest, wxCoord width, wxCoord height,
                   rop == wxSRC_AND ? SRCAND :
                   SRCCOPY;
 
-    bool success = TRUE;
+    bool success;
-    bool needsPixelCopy = FALSE ;
-    bool isPrinter = FALSE ;
-
-    if (IsKindOf(CLASSINFO(wxPrinterDC)) )
-    {
-        isPrinter = TRUE ;
-        if ( ::GetDeviceCaps((HDC) m_hDC, RASTERCAPS) & RC_STRETCHDIB )
-       	{
-        }
-       	else
-       	{
-            needsPixelCopy = TRUE ;
-       	}
-    }
-    if (useMask && !source->m_selectedBitmap.Ok())
-        return FALSE;
-
-    if (useMask && !source->m_selectedBitmap.GetMask())
-        useMask = FALSE;
 
     if (useMask)
     {
-
+#ifdef __WIN32__
-#if 0 // __WIN32__
+        if ( ::MaskBlt(GetHdc(), xdest, ydest,
-        // Not implemented under Win95 (or maybe a specific device?)
+                       (int)width, (int)height,
-        if (MaskBlt(GetHdc(), xdest1, ydest1, (int)width, (int)height,
+                       GetHdcOf(*source), xsrc, ysrc,
-            (HDC) source->m_hDC, xsrc1, ysrc1, (HBITMAP) source->m_selectedBitmap.GetMask()->GetMaskBitmap(),
+                       (HBITMAP) mask->GetMaskBitmap(),
-            0, 0, 0xAACC0020))
+                       0, 0, MAKEROP4(SRCCOPY, PATCOPY)) != 0 )
         {
             // Success
+            success = TRUE;
         }
         else
-#endif
+#endif // Win32
         {
-            // New code from Chris Breeze, 15/7/98
             // Blit bitmap with mask
 
-            if (isPrinter)
-            {
-                // If we are printing source colours are screen colours
-                // not printer colours and so we need copy the bitmap
-                // pixel by pixel.
-                RECT rect;
-                HDC dc_mask = ::CreateCompatibleDC((HDC) source->m_hDC);
-                HDC dc_src = (HDC) source->m_hDC;
-
-                ::SelectObject(dc_mask, (HBITMAP) source->m_selectedBitmap.GetMask()->GetMaskBitmap());
-                for (int x = 0; x < width; x++)
-                {
-                    for (int y = 0; y < height; y++)
-                    {
-                        COLORREF cref = ::GetPixel(dc_mask, x, y);
-                        if (cref)
-                        {
-                            HBRUSH brush = ::CreateSolidBrush(::GetPixel(dc_src, x, y));
-                            rect.left = xdest1 + x; rect.right = rect.left + 1;
-                            rect.top = ydest1 + y;  rect.bottom = rect.top + 1;
-                            ::FillRect(GetHdc(), &rect, brush);
-                            ::DeleteObject(brush);
-                        }
-                    }
-                }
-                ::SelectObject(dc_mask, 0);
-                ::DeleteDC(dc_mask);
-            }
-            else
-            {
             // create a temp buffer bitmap and DCs to access it and the mask
-                HDC dc_mask = ::CreateCompatibleDC((HDC) source->m_hDC);
+            HDC dc_mask = ::CreateCompatibleDC(GetHdcOf(*source));
             HDC dc_buffer = ::CreateCompatibleDC(GetHdc());
             HBITMAP buffer_bmap = ::CreateCompatibleBitmap(GetHdc(), width, height);
-                ::SelectObject(dc_mask, (HBITMAP) source->m_selectedBitmap.GetMask()->GetMaskBitmap());
+            ::SelectObject(dc_mask, (HBITMAP) mask->GetMaskBitmap());
             ::SelectObject(dc_buffer, buffer_bmap);
 
             // copy dest to buffer
-                ::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
+            if ( !::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
-                    GetHdc(), xdest1, ydest1, SRCCOPY);
+                           GetHdc(), xdest, ydest, SRCCOPY) )
+            {
+                wxLogLastError("BitBlt");
+            }
 
             // copy src to buffer using selected raster op
-                ::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
+            if ( !::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
-                    (HDC) source->m_hDC, xsrc1, ysrc1, dwRop);
+                           GetHdcOf(*source), xsrc, ysrc, dwRop) )
+            {
+                wxLogLastError("BitBlt");
+            }
 
             // set masked area in buffer to BLACK (pixel value 0)
             COLORREF prevBkCol = ::SetBkColor(GetHdc(), RGB(255, 255, 255));
             COLORREF prevCol = ::SetTextColor(GetHdc(), RGB(0, 0, 0));
-                ::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
+            if ( !::BitBlt(dc_buffer, 0, 0, (int)width, (int)height,
-                    dc_mask, xsrc1, ysrc1, SRCAND);
+                           dc_mask, xsrc, ysrc, SRCAND) )
+            {
+                wxLogLastError("BitBlt");
+            }
 
             // set unmasked area in dest to BLACK
             ::SetBkColor(GetHdc(), RGB(0, 0, 0));
             ::SetTextColor(GetHdc(), RGB(255, 255, 255));
-                ::BitBlt(GetHdc(), xdest1, ydest1, (int)width, (int)height,
+            if ( !::BitBlt(GetHdc(), xdest, ydest, (int)width, (int)height,
-                    dc_mask, xsrc1, ysrc1, SRCAND);
+                           dc_mask, xsrc, ysrc, SRCAND) )
+            {
+                wxLogLastError("BitBlt");
+            }
             ::SetBkColor(GetHdc(), prevBkCol);   // restore colours to original values
             ::SetTextColor(GetHdc(), prevCol);
 
             // OR buffer to dest
-                success = (::BitBlt(GetHdc(), xdest1, ydest1, (int)width, (int)height,
+            success = ::BitBlt(GetHdc(), xdest, ydest,
-                    dc_buffer, 0, 0, SRCPAINT) != 0);
+                               (int)width, (int)height,
+                               dc_buffer, 0, 0, SRCPAINT) != 0;
+            if ( !success )
+            {
+                wxLogLastError("BitBlt");
+            }
 
             // tidy up temporary DCs and bitmap
             ::SelectObject(dc_mask, 0);
@@ -1379,79 +1298,17 @@ bool wxDC::DoBlit(wxCoord xdest, wxCoord ydest, wxCoord width, wxCoord height,
             ::DeleteObject(buffer_bmap);
         }
     }
+    else // no mask, just BitBlt() it
+    {
+        success = ::BitBlt(GetHdc(), xdest, ydest,
+                           (int)width, (int)height,
+                           GetHdcOf(*source), xsrc, ysrc, dwRop) != 0;
+        if ( !success )
+        {
+            wxLogLastError("BitBlt");
+        }
     }
-    else if (needsPixelCopy) // not masked, but we need pixel copy. Only true if it's a printer
-    {
-        // If we are printing, source colours are screen colours
-        // not printer colours and so we need copy the bitmap
-        // pixel by pixel.
-        if (isPrinter)
-        {
-            HDC dc_src = (HDC) source->m_hDC;
-	        RECT rect;
-	        for (int y = 0; y < height; y++)
-	        {
-                // This is Stefan Csomor's optimisation, where
-                // identical adjacent pixels are drawn together.
-	      	    for (int x = 0; x < width; x++)
-	            {
-	                COLORREF col = ::GetPixel(dc_src, x, y) ;
-	                HBRUSH brush = ::CreateSolidBrush( col );
 
-	                rect.left = xdest1 + x;
-	                rect.top = ydest1 + y;
-	                while( (x + 1 < width) && (::GetPixel(dc_src, x + 1, y) == col ) )
-	                {
-	          	        ++x ;
-	                }
-	                rect.right = xdest1 + x + 1;
-	                rect.bottom = rect.top + 1;
-	                ::FillRect((HDC) m_hDC, &rect, brush);
-	                ::DeleteObject(brush);
-                }
-	        }
-	    }
-        else
-        {
-            wxFAIL_MSG( "If needsPixelCopy is true, isPrinter should be true also." );
-        }
-    }
-    else if (isPrinter) // not masked, not pixel copy
-    {
-        wxBitmap& bmp = source->m_selectedBitmap ;
-        BITMAPINFO *info = (BITMAPINFO *) malloc( sizeof( BITMAPINFOHEADER ) + 256 * sizeof(RGBQUAD ) ) ;
-        int iBitsSize = ((bmp.GetWidth() + 3 ) & ~3 ) * bmp.GetHeight() ;
-            
-        void* bits = malloc( iBitsSize ) ;
-            
-        memset( info , 0 , sizeof( BITMAPINFOHEADER ) ) ;
-            
-        info->bmiHeader.biSize = sizeof( BITMAPINFOHEADER ) ;
-        info->bmiHeader.biWidth = bmp.GetWidth() ;
-        info->bmiHeader.biHeight = bmp.GetHeight() ;
-        info->bmiHeader.biPlanes = 1 ;
-        info->bmiHeader.biBitCount = 8 ;
-        info->bmiHeader.biCompression = BI_RGB ;
-            
-        HDC display = GetDC( NULL ) ;
-        if ( GetDIBits( display , (HBITMAP) bmp.GetHBITMAP( ) , 0 , bmp.GetHeight() , bits , info , DIB_RGB_COLORS ) )
-        {
-            success = (GDI_ERROR != StretchDIBits( (HDC) m_hDC,
-                    xdest1, ydest1, bmp.GetWidth(), bmp.GetHeight() ,
-                    xsrc1 , ysrc1 ,bmp.GetWidth(), bmp.GetHeight() ,
-                    bits , info , DIB_RGB_COLORS , SRCCOPY )) ;
-        }
-        else
-            success = FALSE;
-        ReleaseDC( NULL , display ) ;
-        free ( bits ) ;
-        free( info ) ;
-    }
-    else // Not masked, not printer, not pixel copy
-    {
-        success = (BitBlt(GetHdc(), xdest1, ydest1, (int)width, (int)height, (HDC) source->m_hDC,
-             xsrc1, ysrc1, dwRop) != 0);
-    }
     ::SetTextColor(GetHdc(), old_textground);
     ::SetBkColor(GetHdc(), old_background);
 

src/msw/dcprint.cpp

@@ -1,5 +1,5 @@
 /////////////////////////////////////////////////////////////////////////////
-// Name:        dcprint.cpp
+// Name:        src/msw/dcprint.cpp
 // Purpose:     wxPrinterDC class
 // Author:      Julian Smart
 // Modified by:
@@ -9,43 +9,62 @@
 // Licence:     wxWindows licence
 /////////////////////////////////////////////////////////////////////////////
 
+// ============================================================================
+// declarations
+// ============================================================================
+
+// ----------------------------------------------------------------------------
+// headers
+// ----------------------------------------------------------------------------
+
 #ifdef __GNUG__
-#pragma implementation "dcprint.h"
+    #pragma implementation "dcprint.h"
 #endif
 
 // For compilers that support precompilation, includes "wx.h".
 #include "wx/wxprec.h"
 
 #ifdef __BORLANDC__
-#pragma hdrstop
+    #pragma hdrstop
 #endif
 
 #ifndef WX_PRECOMP
+    #include "wx/string.h"
+    #include "wx/log.h"
+    #include "wx/window.h"
 #endif
 
-#include "wx/string.h"
-#include "wx/log.h"
-#include "wx/window.h"
 #include "wx/msw/private.h"
 #include "wx/dcprint.h"
 #include "math.h"
 
 #if wxUSE_COMMON_DIALOGS || defined(__WXWINE__)
-#include <commdlg.h>
+    #include <commdlg.h>
 #endif
 
 #ifndef __WIN32__
-#include <print.h>
+    #include <print.h>
 #endif
 
+// ----------------------------------------------------------------------------
+// wxWin macros
+// ----------------------------------------------------------------------------
 IMPLEMENT_CLASS(wxPrinterDC, wxDC)
 
+// ============================================================================
+// implementation
+// ============================================================================
+
+// ----------------------------------------------------------------------------
+// wxPrinterDC construction
+// ----------------------------------------------------------------------------
+
 // This form is deprecated
 wxPrinterDC::wxPrinterDC(const wxString& driver_name, const wxString& device_name, const wxString& file, bool interactive, int orientation)
 {
     m_isInteractive = interactive;
 
-    if (!file.IsNull() && file != wxT(""))
+    if ( !!file )
         m_printData.SetFilename(file);
 
 #if wxUSE_COMMON_DIALOGS
@@ -79,7 +98,7 @@ wxPrinterDC::wxPrinterDC(const wxString& driver_name, const wxString& device_nam
         //     m_dontDelete = TRUE;
     }
     else
-#endif
+#endif // wxUSE_COMMON_DIALOGS
         if ((!driver_name.IsNull() && driver_name != wxT("")) &&
             (!device_name.IsNull() && device_name != wxT("")) &&
             (!file.IsNull() && file != wxT("")))
@@ -138,10 +157,14 @@ wxPrinterDC::wxPrinterDC(WXHDC theDC)
     SetPen(*wxBLACK_PEN);
 }
 
-wxPrinterDC::~wxPrinterDC(void)
+wxPrinterDC::~wxPrinterDC()
 {
 }
 
+// ----------------------------------------------------------------------------
+// wxPrinterDC {Start/End}{Page/Doc} methods
+// ----------------------------------------------------------------------------
+
 bool wxPrinterDC::StartDoc(const wxString& message)
 {
     DOCINFO docinfo;
@@ -163,20 +186,7 @@ bool wxPrinterDC::StartDoc(const wxString& message)
     if (!m_hDC)
         return FALSE;
 
-    int ret =
+    int ret = ::StartDoc(GetHdc(), &docinfo);
-#ifndef __WIN32__
-        ::StartDoc((HDC) m_hDC, &docinfo);
-#else
-#ifdef UNICODE
-    ::StartDocW((HDC) m_hDC, &docinfo);
-#else
-#ifdef __TWIN32__
-    ::StartDoc((HDC) m_hDC, &docinfo);
-#else
-    ::StartDocA((HDC) m_hDC, &docinfo);
-#endif
-#endif
-#endif
 
 #ifndef __WIN16__
     if (ret <= 0)
@@ -189,18 +199,18 @@ bool wxPrinterDC::StartDoc(const wxString& message)
     return (ret > 0);
 }
 
-void wxPrinterDC::EndDoc(void)
+void wxPrinterDC::EndDoc()
 {
     if (m_hDC) ::EndDoc((HDC) m_hDC);
 }
 
-void wxPrinterDC::StartPage(void)
+void wxPrinterDC::StartPage()
 {
     if (m_hDC)
         ::StartPage((HDC) m_hDC);
 }
 
-void wxPrinterDC::EndPage(void)
+void wxPrinterDC::EndPage()
 {
     if (m_hDC)
         ::EndPage((HDC) m_hDC);
@@ -350,13 +360,13 @@ WXHDC WXDLLEXPORT wxGetPrinterDC(const wxPrintData& printDataConst)
     wxChar* driverName = (wxChar*) NULL;
 
     wxString devNameStr = printData.GetPrinterName();
-    wxChar* deviceName;
     wxChar* portName = (wxChar*) NULL; // Obsolete in WIN32
 
-    if (devNameStr == wxT(""))
+    const wxChar* deviceName;
+    if ( !devNameStr )
         deviceName = (wxChar*) NULL;
     else
-        deviceName = WXSTRINGCAST devNameStr;
+        deviceName = devNameStr.c_str();
 
     LPDEVMODE lpDevMode = (LPDEVMODE) NULL;
 
@@ -365,7 +375,7 @@ WXHDC WXDLLEXPORT wxGetPrinterDC(const wxPrintData& printDataConst)
     if ( hDevMode )
         lpDevMode = (DEVMODE*) GlobalLock(hDevMode);
 
-    if (devNameStr == wxT(""))
+    if ( !devNameStr )
     {
         // Retrieve the default device name
         wxString portName;
@@ -378,7 +388,7 @@ WXHDC WXDLLEXPORT wxGetPrinterDC(const wxPrintData& printDataConst)
 
         wxASSERT_MSG( ret, wxT("Could not get default device name.") );
 
-        deviceName = WXSTRINGCAST devNameStr;
+        deviceName = devNameStr.c_str();
     }
 
 #ifdef __WIN32__
@@ -393,3 +403,180 @@ WXHDC WXDLLEXPORT wxGetPrinterDC(const wxPrintData& printDataConst)
     return (WXHDC) hDC;
 }
 
+// ----------------------------------------------------------------------------
+// wxPrinterDC bit blitting/bitmap drawing
+// ----------------------------------------------------------------------------
+
+void wxPrinterDC::DoDrawBitmap(const wxBitmap &bmp,
+                               wxCoord x, wxCoord y,
+                               bool useMask)
+{
+    wxCHECK_RET( bmp.Ok(), _T("invalid bitmap in wxPrinterDC::DrawBitmap") );
+
+    int width = bmp.GetWidth(),
+        height = bmp.GetHeight();
+
+    if ( ::GetDeviceCaps(GetHdc(), RASTERCAPS) & RC_STRETCHDIB )
+    {
+        BITMAPINFO *info = (BITMAPINFO *) malloc( sizeof( BITMAPINFOHEADER ) + 256 * sizeof(RGBQUAD ) );
+        memset( info, 0, sizeof( BITMAPINFOHEADER ) );
+
+        int iBitsSize = ((width + 3 ) & ~3 ) * height;
+
+        void* bits = malloc( iBitsSize );
+
+        info->bmiHeader.biSize = sizeof( BITMAPINFOHEADER );
+        info->bmiHeader.biWidth = width;
+        info->bmiHeader.biHeight = height;
+        info->bmiHeader.biPlanes = 1;
+        info->bmiHeader.biBitCount = 8;
+        info->bmiHeader.biCompression = BI_RGB;
+
+        ScreenHDC display;
+        if ( GetDIBits(display, GetHbitmapOf(bmp), 0,
+                       bmp.GetHeight(), bits, info,
+                       DIB_RGB_COLORS) )
+        {
+            if ( ::StretchDIBits(GetHdc(), x, y,
+                                 width, height,
+                                 0 , 0, width, height,
+                                 bits, info,
+                                 DIB_RGB_COLORS, SRCCOPY) == GDI_ERROR )
+            {
+                wxLogLastError("StretchDIBits");
+            }
+        }
+
+        free(bits);
+        free(info);
+    }
+    else // no support for StretchDIBits()
+    {
+        wxMemoryDC memDC;
+        memDC.SelectObject(bmp);
+
+        Blit(x, y, width, height, &memDC, 0, 0, wxCOPY, useMask);
+
+        memDC.SelectObject(wxNullBitmap);
+    }
+}
+
+bool wxPrinterDC::DoBlit(wxCoord xdest, wxCoord ydest,
+                         wxCoord width, wxCoord height,
+                         wxDC *source,
+                         wxCoord xsrc, wxCoord ysrc,
+                         int rop, bool useMask)
+{
+    bool success = TRUE;
+
+    if ( useMask )
+    {
+        // If we are printing source colours are screen colours
+        // not printer colours and so we need copy the bitmap
+        // pixel by pixel.
+        RECT rect;
+        HDC dc_src = GetHdcOf(*source);
+        HDC dc_mask = ::CreateCompatibleDC(dc_src);
+
+        ::SelectObject(dc_mask, (HBITMAP) source->GetSelectedBitmap().GetMask()->GetMaskBitmap());
+        for (int x = 0; x < width; x++)
+        {
+            for (int y = 0; y < height; y++)
+            {
+                COLORREF cref = ::GetPixel(dc_mask, x, y);
+                if (cref)
+                {
+                    HBRUSH brush = ::CreateSolidBrush(::GetPixel(dc_src, x, y));
+                    rect.left = xdest + x;
+                    rect.right = rect.left + 1;
+                    rect.top = ydest + y; 
+                    rect.bottom = rect.top + 1;
+                    ::FillRect(GetHdc(), &rect, brush);
+                    ::DeleteObject(brush);
+                }
+            }
+        }
+        ::SelectObject(dc_mask, 0);
+        ::DeleteDC(dc_mask);
+    }
+    else // no mask
+    {
+        if ( ::GetDeviceCaps(GetHdc(), RASTERCAPS) & RC_STRETCHDIB )
+        {
+            wxBitmap& bmp = source->GetSelectedBitmap();
+            int width = bmp.GetWidth(),
+                height = bmp.GetHeight();
+
+            BITMAPINFO *info = (BITMAPINFO *) malloc( sizeof( BITMAPINFOHEADER ) + 256 * sizeof(RGBQUAD ) );
+            int iBitsSize = ((width + 3 ) & ~3 ) * height;
+
+            void* bits = malloc( iBitsSize );
+
+            memset( info , 0 , sizeof( BITMAPINFOHEADER ) );
+
+            info->bmiHeader.biSize = sizeof( BITMAPINFOHEADER );
+            info->bmiHeader.biWidth = width;
+            info->bmiHeader.biHeight = height;
+            info->bmiHeader.biPlanes = 1;
+            info->bmiHeader.biBitCount = 8;
+            info->bmiHeader.biCompression = BI_RGB;
+
+            ScreenHDC display;
+            if ( !::GetDIBits(display, GetHbitmapOf(bmp), 0,
+                              height, bits, info, DIB_RGB_COLORS) )
+            {
+                wxLogLastError("GetDIBits");
+
+                success = FALSE;
+            }
+
+            if ( success )
+            {
+                success = ::StretchDIBits(GetHdc(), xdest, ydest,
+                                          width, height,
+                                          xsrc, ysrc,
+                                          width, height,
+                                          bits, info ,
+                                          DIB_RGB_COLORS,
+                                          SRCCOPY) != GDI_ERROR;
+                if ( !success )
+                {
+                    wxLogLastError("StretchDIBits");
+                }
+            }
+
+            free(bits);
+            free(info);
+        }
+        else // no support for StretchDIBits
+        {
+            // as we are printing, source colours are screen colours not printer
+            // colours and so we need copy the bitmap pixel by pixel.
+            HDC dc_src = GetHdcOf(*source);
+            RECT rect;
+            for (int y = 0; y < height; y++)
+            {
+                // This is Stefan Csomor's optimisation, where identical adjacent
+                // pixels are drawn together.
+                for (int x = 0; x < width; x++)
+                {
+                    COLORREF col = ::GetPixel(dc_src, x, y);
+                    HBRUSH brush = ::CreateSolidBrush( col );
+
+                    rect.left = xdest + x;
+                    rect.top = ydest + y;
+                    while( (x + 1 < width) && (::GetPixel(dc_src, x + 1, y) == col ) )
+                    {
+                        ++x;
+                    }
+                    rect.right = xdest + x + 1;
+                    rect.bottom = rect.top + 1;
+                    ::FillRect((HDC) m_hDC, &rect, brush);
+                    ::DeleteObject(brush);
+                }
+            }
+        }
+    }
+
+    return success;
+}

src/msw/icon.cpp

@@ -47,7 +47,7 @@
 // wxWin macros
 // ----------------------------------------------------------------------------
 
-    IMPLEMENT_DYNAMIC_CLASS(wxIcon, wxIconBase)
+IMPLEMENT_DYNAMIC_CLASS(wxIcon, wxIconBase)
 
 // ============================================================================
 // implementation
@@ -71,14 +71,10 @@ void wxIconRefData::Free()
 // wxIcon
 // ----------------------------------------------------------------------------
 
-wxIcon::wxIcon()
+wxIcon::wxIcon(const char bits[], int width, int height)
-{
-}
-
-wxIcon::wxIcon(const char WXUNUSED(bits)[],
-               int WXUNUSED(width),
-               int WXUNUSED(height))
 {
+    wxBitmap bmp(bits, width, height);
+    CopyFromBitmap(bmp);
 }
 
 wxIcon::wxIcon(const wxString& iconfile,
@@ -94,6 +90,51 @@ wxIcon::~wxIcon()
 {
 }
 
+void wxIcon::CopyFromBitmap(const wxBitmap& bmp)
+{
+#ifdef __WIN32__
+    wxMask *mask = bmp.GetMask();
+    if ( !mask )
+    {
+        // we must have a mask for an icon, so even if it's probably incorrect,
+        // do create it (grey is the "standard" transparent colour)
+        mask = new wxMask(bmp, *wxLIGHT_GREY);
+    }
+
+    ICONINFO iconInfo;
+    iconInfo.fIcon = TRUE;  // we want an icon, not a cursor
+    iconInfo.hbmMask = wxInvertMask((HBITMAP)mask->GetMaskBitmap());
+    iconInfo.hbmColor = GetHbitmapOf(bmp);
+
+    HICON hicon = ::CreateIconIndirect(&iconInfo);
+    if ( !hicon )
+    {
+        wxLogLastError("CreateIconIndirect");
+    }
+    else
+    {
+        SetHICON((WXHICON)hicon);
+        SetSize(bmp.GetWidth(), bmp.GetHeight());
+    }
+
+    if ( !bmp.GetMask() )
+    {
+        // we created the mask, now delete it
+        delete mask;
+    }
+#else // Win16
+    // there are some functions in curico.cpp which probably could be used
+    // here...
+    wxFAIL_MSG("not implemented");
+#endif // Win32/16
+}
+
+void wxIcon::CreateIconFromXpm(const char **data)
+{
+    wxBitmap bmp(data);
+    CopyFromBitmap(bmp);
+}
+
 bool wxIcon::LoadFile(const wxString& filename,
                       long type,
                       int desiredWidth, int desiredHeight)

src/msw/imaglist.cpp

@@ -1,6 +1,6 @@
 /////////////////////////////////////////////////////////////////////////////
-// Name:        imaglist.cpp
+// Name:        src/msw/imaglist.cpp
-// Purpose:     wxImageList
+// Purpose:     wxImageList implementation for Win32
 // Author:      Julian Smart
 // Modified by:
 // Created:     04/01/98
@@ -9,26 +9,34 @@
 // Licence:     wxWindows license
 /////////////////////////////////////////////////////////////////////////////
 
+// ============================================================================
+// declarations
+// ============================================================================
+
+// ----------------------------------------------------------------------------
+// headers
+// ----------------------------------------------------------------------------
+
 #ifdef __GNUG__
-#pragma implementation "imaglist.h"
+    #pragma implementation "imaglist.h"
 #endif
 
 // For compilers that support precompilation, includes "wx.h".
 #include "wx/wxprec.h"
 
 #ifdef __BORLANDC__
-#pragma hdrstop
+    #pragma hdrstop
 #endif
 
 #if defined(__WIN95__)
 
 #ifndef WX_PRECOMP
-#include <stdio.h>
+    #include "wx/window.h"
-#include "wx/setup.h"
+    #include "wx/icon.h"
-#include "wx/window.h"
+    #include "wx/dc.h"
-#include "wx/icon.h"
+    #include "wx/string.h"
-#include "wx/dc.h"
+
-#include "wx/string.h"
+    #include <stdio.h>
 #endif
 
 #include "wx/log.h"
@@ -37,87 +45,97 @@
 #include "wx/msw/imaglist.h"
 #include "wx/msw/private.h"
 
-#if (defined(__WIN95__) && !defined(__GNUWIN32__)) || defined(__TWIN32__) || defined(wxUSE_NORLANDER_HEADERS)
+#if !defined(__GNUWIN32_OLD__) && !defined(__TWIN32__)
-#include <commctrl.h>
+    #include <commctrl.h>
 #endif
 
+// ----------------------------------------------------------------------------
+// wxWin macros
+// ----------------------------------------------------------------------------
+
 IMPLEMENT_DYNAMIC_CLASS(wxImageList, wxObject)
 
-wxImageList::wxImageList(void)
+#define GetHImageList()     ((HIMAGELIST)m_hImageList)
+
+// ----------------------------------------------------------------------------
+// private functions
+// ----------------------------------------------------------------------------
+
+// returns the mask if it's valid, otherwise the bitmap mask and, if it's not
+// valid neither, a "solid" mask (no transparent zones at all)
+static wxBitmap GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask);
+
+// ============================================================================
+// implementation
+// ============================================================================
+
+// ----------------------------------------------------------------------------
+// wxImageList creation/destruction
+// ----------------------------------------------------------------------------
+
+wxImageList::wxImageList()
 {
     m_hImageList = 0;
 }
 
-wxImageList::~wxImageList(void)
-{
-	if ( m_hImageList )
-		ImageList_Destroy((HIMAGELIST) m_hImageList);
-	m_hImageList = 0;
-}
-
-
-// Attributes
-////////////////////////////////////////////////////////////////////////////
-
-// Returns the number of images in the image list.
-int wxImageList::GetImageCount(void) const
-{
-	return ImageList_GetImageCount((HIMAGELIST) m_hImageList);
-}
-
-// Operations
-////////////////////////////////////////////////////////////////////////////
-
 // Creates an image list
 bool wxImageList::Create(int width, int height, bool mask, int initial)
 {
-	UINT flags = 0;
+    UINT flags = 0; // TODO shouldallow to specify ILC_COLORxxx here
     if ( mask )
         flags |= ILC_MASK;
 
     // Grow by 1, I guess this is reasonable behaviour most of the time
-	m_hImageList = (WXHIMAGELIST) ImageList_Create(width, height, flags, initial, 1);
+    m_hImageList = (WXHIMAGELIST) ImageList_Create(width, height, flags,
-	return (m_hImageList != 0);
+                                                   initial, 1);
+    if ( !m_hImageList )
+    {
+        wxLogLastError("ImageList_Create()");
+    }
+
+    return m_hImageList != 0;
 }
 
+wxImageList::~wxImageList()
+{
+    if ( m_hImageList )
+    {
+        ImageList_Destroy(GetHImageList());
+        m_hImageList = 0;
+    }
+}
+
+// ----------------------------------------------------------------------------
+// wxImageList attributes
+// ----------------------------------------------------------------------------
+
+// Returns the number of images in the image list.
+int wxImageList::GetImageCount() const
+{
+    wxASSERT_MSG( m_hImageList, _T("invalid image list") );
+
+    return ImageList_GetImageCount(GetHImageList());
+}
+
+// ----------------------------------------------------------------------------
+// wxImageList operations
+// ----------------------------------------------------------------------------
+
 // Adds a bitmap, and optionally a mask bitmap.
 // Note that wxImageList creates new bitmaps, so you may delete
 // 'bitmap' and 'mask'.
 int wxImageList::Add(const wxBitmap& bitmap, const wxBitmap& mask)
 {
-	HBITMAP hBitmap1 = (HBITMAP) bitmap.GetHBITMAP();
+    wxBitmap bmpMask = GetMaskForImage(bitmap, mask);
-	HBITMAP hBitmap2 = 0;
+    HBITMAP hbmpMask = wxInvertMask(GetHbitmapOf(bmpMask));
-	if ( mask.Ok() )
-	    hBitmap2 = (HBITMAP) mask.GetHBITMAP();
-    else if (bitmap.GetMask())
-        hBitmap2 = (HBITMAP) bitmap.GetMask()->GetMaskBitmap();
 
-    HBITMAP hBitmapI=0;
+    int index = ImageList_Add(GetHImageList(), GetHbitmapOf(bitmap), hbmpMask);
-    if(hBitmap2!=0) {
-        // Microsoft imagelist masks are inverted from wxWindows mask standard (white is mask color)
-        BITMAP bm;
-        ::GetObject(hBitmap2,sizeof(BITMAP),(LPVOID)&bm);
-        int w=bm.bmWidth;
-        int h=bm.bmHeight;
-        HDC hdc = ::CreateCompatibleDC(NULL);   
-        HDC hdci = ::CreateCompatibleDC(NULL);  
-        hBitmapI = ::CreateCompatibleBitmap(hdci, w, h);
-        ::SelectObject(hdc, hBitmap2);
-        ::SelectObject(hdci, hBitmapI);
-        ::BitBlt(hdci, 0, 0, w, h, hdc, 0, 0, NOTSRCCOPY);
-        ::DeleteDC(hdc);
-        ::DeleteDC(hdci);
-    }
-
-    int index = ImageList_Add((HIMAGELIST) GetHIMAGELIST(), hBitmap1, hBitmapI);
     if ( index == -1 )
     {
         wxLogError(_("Couldn't add an image to the image list."));
     }
 
-    // Clean up inverted mask
+    ::DeleteObject(hbmpMask);
-    if(hBitmapI!=0)
-        ::DeleteObject(hBitmapI);
 
     return index;
 }
@@ -128,111 +146,128 @@ int wxImageList::Add(const wxBitmap& bitmap, const wxBitmap& mask)
 int wxImageList::Add(const wxBitmap& bitmap, const wxColour& maskColour)
 {
 #ifdef __TWIN32__
-        wxFAIL_MSG("ImageList_AddMasked not implemented in TWIN32");
+    wxFAIL_MSG(_T("ImageList_AddMasked not implemented in TWIN32"));
     return -1;
 #else
-	HBITMAP hBitmap1 = (HBITMAP) bitmap.GetHBITMAP();
+    int index = ImageList_AddMasked(GetHImageList(),
-	COLORREF colorRef = PALETTERGB(maskColour.Red(), maskColour.Green(), maskColour.Blue());
+                                    GetHbitmapOf(bitmap),
-	return ImageList_AddMasked((HIMAGELIST) GetHIMAGELIST(), hBitmap1, colorRef);
+                                    wxColourToRGB(maskColour));
+    if ( index == -1 )
+    {
+        wxLogError(_("Couldn't add an image to the image list."));
+    }
+
+    return index;
 #endif
 }
 
 // Adds a bitmap and mask from an icon.
 int wxImageList::Add(const wxIcon& icon)
 {
-	HICON hIcon = (HICON) icon.GetHICON();
+    int index = ImageList_AddIcon(GetHImageList(), GetHiconOf(icon));
-	return ImageList_AddIcon((HIMAGELIST) GetHIMAGELIST(), hIcon);
+    if ( index == -1 )
+    {
+        wxLogError(_("Couldn't add an image to the image list."));
+    }
+
+    return index;
 }
 
 // Replaces a bitmap, optionally passing a mask bitmap.
 // Note that wxImageList creates new bitmaps, so you may delete
 // 'bitmap' and 'mask'.
-bool wxImageList::Replace(int index, const wxBitmap& bitmap, const wxBitmap& mask)
+bool wxImageList::Replace(int index,
+                          const wxBitmap& bitmap, const wxBitmap& mask)
 {
 #ifdef __TWIN32__
-        wxFAIL_MSG("ImageList_Replace not implemented in TWIN32");
+    wxFAIL_MSG(_T("ImageList_Replace not implemented in TWIN32"));
     return FALSE;
 #else
-	HBITMAP hBitmap1 = (HBITMAP) bitmap.GetHBITMAP();
+    wxBitmap bmpMask = GetMaskForImage(bitmap, mask);
-	HBITMAP hBitmap2 = 0;
+    HBITMAP hbmpMask = wxInvertMask(GetHbitmapOf(bmpMask));
-	if ( mask.Ok() )
+
-	    hBitmap2 = (HBITMAP) mask.GetHBITMAP();
+    bool ok = ImageList_Replace(GetHImageList(), index,
-	return (ImageList_Replace((HIMAGELIST) GetHIMAGELIST(), index, hBitmap1, hBitmap2) != 0);
+                                GetHbitmapOf(bitmap), hbmpMask) != 0;
+    if ( !ok )
+    {
+        wxLogLastError("ImageList_Add()");
+    }
+
+    ::DeleteObject(hbmpMask);
+
+    return ok;
 #endif
 }
 
-/* Not supported by Win95
-// Replacing a bitmap, using the specified colour to create the mask bitmap
-// Note that wxImageList creates new bitmaps, so you may delete
-// 'bitmap'.
-bool wxImageList::Replace(int index, const wxBitmap& bitmap, const wxColour& maskColour)
-{
-	HBITMAP hBitmap1 = (HBITMAP) bitmap.GetHBITMAP();
-	COLORREF colorRef = PALETTERGB(maskColour.Red(), maskColour.Green(), maskColour.Blue());
-	return (bool) ImageList_ReplaceMasked((HIMAGELIST) GetHIMAGELIST(), index, hBitmap1, colorRef);
-}
-*/
-
 // Replaces a bitmap and mask from an icon.
-bool wxImageList::Replace(int index, const wxIcon& icon)
+bool wxImageList::Replace(int i, const wxIcon& icon)
 {
-	HICON hIcon = (HICON) icon.GetHICON();
+    bool ok = ImageList_ReplaceIcon(GetHImageList(), i, GetHiconOf(icon)) != 0;
-	return (ImageList_ReplaceIcon((HIMAGELIST) GetHIMAGELIST(), index, hIcon) != 0);
+    if ( !ok )
+    {
+        wxLogLastError("ImageList_ReplaceIcon()");
+    }
+
+    return ok;
 }
 
 // Removes the image at the given index.
 bool wxImageList::Remove(int index)
 {
 #ifdef __TWIN32__
-        wxFAIL_MSG("ImageList_Replace not implemented in TWIN32");
+    wxFAIL_MSG(_T("ImageList_Replace not implemented in TWIN32"));
     return FALSE;
 #else
-	return (ImageList_Remove((HIMAGELIST) GetHIMAGELIST(), index) != 0);
+    bool ok = ImageList_Remove(GetHImageList(), index) != 0;
+    if ( !ok )
+    {
+        wxLogLastError("ImageList_Remove()");
+    }
+
+    return ok;
 #endif
 }
 
 // Remove all images
-bool wxImageList::RemoveAll(void)
+bool wxImageList::RemoveAll()
 {
-	// TODO: Is this correct?
+    bool ok = ImageList_RemoveAll(GetHImageList()) != 0;
-	while ( GetImageCount() > 0 )
+    if ( !ok )
     {
-		Remove(0);
+        wxLogLastError("ImageList_RemoveAll()");
     }
-	return TRUE;
+
+    return ok;
 }
 
 // Draws the given image on a dc at the specified position.
 // If 'solidBackground' is TRUE, Draw sets the image list background
 // colour to the background colour of the wxDC, to speed up
 // drawing by eliminating masked drawing where possible.
-bool wxImageList::Draw(int index, wxDC& dc, int x, int y,
+bool wxImageList::Draw(int index,
-    int flags, bool solidBackground)
+                       wxDC& dc,
+                       int x, int y,
+                       int flags,
+                       bool solidBackground)
 {
 #ifdef __TWIN32__
-        wxFAIL_MSG("ImageList_Replace not implemented in TWIN32");
+    wxFAIL_MSG(_T("ImageList_Replace not implemented in TWIN32"));
     return FALSE;
 #else
-	HDC hDC = (HDC) dc.GetHDC();
+    HDC hDC = GetHdcOf(dc);
-	if ( !hDC )
+    wxCHECK_MSG( hDC, FALSE, _T("invalid wxDC in wxImageList::Draw") );
-		return FALSE;
 
+    COLORREF clr = CLR_NONE;    // transparent by default
     if ( solidBackground )
     {
         wxBrush *brush = & dc.GetBackground();
-		if ( brush && brush->Ok())
+        if ( brush && brush->Ok() )
         {
-			wxColour col(brush->GetColour());
+            clr = wxColourToRGB(brush->GetColour());
-			ImageList_SetBkColor((HIMAGELIST) GetHIMAGELIST(),
-					PALETTERGB(col.Red(), col.Green(), col.Blue()));
         }
-		else
-			ImageList_SetBkColor((HIMAGELIST) GetHIMAGELIST(),
-				CLR_NONE);
     }
-	else
+
-		ImageList_SetBkColor((HIMAGELIST) GetHIMAGELIST(),
+    ImageList_SetBkColor(GetHImageList(), clr);
-				CLR_NONE);
 
     UINT style = 0;
     if ( flags & wxIMAGELIST_DRAW_NORMAL )
@@ -244,10 +279,51 @@ bool wxImageList::Draw(int index, wxDC& dc, int x, int y,
     if ( flags & wxIMAGELIST_DRAW_FOCUSED )
         style |= ILD_FOCUS;
 
-	return (ImageList_Draw((HIMAGELIST) GetHIMAGELIST(), index, hDC,
+    bool ok = ImageList_Draw(GetHImageList(), index, hDC, x, y, style) != 0;
-		x, y, style) != 0);
+    if ( !ok )
+    {
+        wxLogLastError("ImageList_Draw()");
+    }
+
+    return ok;
 #endif
 }
 
-#endif
+// ----------------------------------------------------------------------------
+// helpers
+// ----------------------------------------------------------------------------
+
+static wxBitmap GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask)
+{
+    wxBitmap bmpMask;
+
+    if ( mask.Ok() )
+    {
+        bmpMask = mask;
+    }
+    else
+    {
+        wxMask *pMask = bitmap.GetMask();
+        if ( pMask )
+        {
+            bmpMask.SetHBITMAP(pMask->GetMaskBitmap());
+        }
+    }
+
+    if ( !bmpMask.Ok() )
+    {
+        // create a non transparent mask - apparently, this is needed under
+        // Win9x (it doesn't behave correctly if it's passed 0 mask)
+        bmpMask.Create(bitmap.GetWidth(), bitmap.GetHeight(), 1);
+
+        wxMemoryDC dcMem;
+        dcMem.SelectObject(bmpMask);
+        dcMem.Clear();
+        dcMem.SelectObject(wxNullBitmap);
+    }
+
+    return bmpMask;
+}
+
+#endif // Win95
 

src/msw/notebook.cpp

@@ -144,7 +144,7 @@ bool wxNotebook::Create(wxWindow *parent,
 
   // colors and font
   m_backgroundColour = wxColour(GetSysColor(COLOR_BTNFACE));
-  m_foregroundColour = *wxBLACK ;
+  m_foregroundColour = *wxBLACK;
 
   // style
   m_windowStyle = style | wxTAB_TRAVERSAL;
@@ -450,10 +450,13 @@ void wxNotebook::OnSize(wxSizeEvent& event)
   GetSize((int *)&rc.right, (int *)&rc.bottom);
 
   TabCtrl_AdjustRect(m_hwnd, FALSE, &rc);
+
+  int width = rc.right - rc.left,
+      height = rc.bottom - rc.top;
   size_t nCount = m_aPages.Count();
   for ( size_t nPage = 0; nPage < nCount; nPage++ ) {
     wxNotebookPage *pPage = m_aPages[nPage];
-    pPage->SetSize(rc.left, rc.top, rc.right - rc.left, rc.bottom - rc.top);
+    pPage->SetSize(rc.left, rc.top, width, height);
   }
 
   event.Skip();
@@ -513,13 +516,13 @@ void wxNotebook::OnNavigationKey(wxNavigationKeyEvent& event)
 
 // override these 2 functions to do nothing: everything is done in OnSize
 
-void wxNotebook::SetConstraintSizes(bool /* recurse */)
+void wxNotebook::SetConstraintSizes(bool WXUNUSED(recurse))
 {
   // don't set the sizes of the pages - their correct size is not yet known
   wxControl::SetConstraintSizes(FALSE);
 }
 
-bool wxNotebook::DoPhase(int /* nPhase */)
+bool wxNotebook::DoPhase(int WXUNUSED(nPhase))
 {
   return TRUE;
 }

src/msw/xpmhand.cpp

@@ -50,6 +50,7 @@ static void XpmToBitmap(wxBitmap *bitmap,
     wxBitmapRefData *refData = bitmap->GetBitmapData();
     refData->m_hBitmap = (WXHBITMAP)ximage->bitmap;
 
+    // first set the bitmap width, height, depth...
     BITMAP bm;
     if ( !::GetObject(GetHbitmapOf(*bitmap), sizeof(bm), (LPSTR) & bm) )
     {
@@ -60,6 +61,26 @@ static void XpmToBitmap(wxBitmap *bitmap,
     refData->m_height = bm.bmHeight;
     refData->m_depth = bm.bmPlanes * bm.bmBitsPixel;
     refData->m_numColors = xpmAttr.npixels;
+
+    // next get the mask, if any
+    if ( xpmAttr.mask_pixel != XpmUndefPixel )
+    {
+        int red, green, blue;
+        const char *clrString = xpmAttr.colorTable[xpmAttr.mask_pixel].c_color;
+        if ( strcmp(clrString, "None") == 0 )
+        {
+            // TODO what to do here??
+            red = green = 0;
+            blue = 1;
+        }
+        else
+        {
+            sscanf(clrString, "#%02x%02x%02x", &red, &green, &blue);
+        }
+
+        wxMask *mask = new wxMask(*bitmap, wxColour(red, green, blue));
+        bitmap->SetMask(mask);
+    }
 }
 
 #endif // wxUSE_XPM_IN_MSW
@@ -79,13 +100,18 @@ bool wxXPMFileHandler::LoadFile(wxBitmap *bitmap,
     dc = CreateCompatibleDC(NULL);
     if (dc)
     {
-      xpmAttr.valuemask = XpmReturnPixels;
+        xpmAttr.valuemask = XpmReturnPixels | XpmColorTable;
-      int errorStatus = XpmReadFileToImage(&dc, wxMBSTRINGCAST name.fn_str(), &ximage, (XImage **) NULL, &xpmAttr);
+        int errorStatus = XpmReadFileToImage(&dc,
+                                             wxMBSTRINGCAST name.fn_str(),
+                                             &ximage,
+                                             (XImage **)NULL,
+                                             &xpmAttr);
         DeleteDC(dc);
         if (errorStatus == XpmSuccess)
         {
             XpmToBitmap(bitmap, ximage, xpmAttr);
 
+            XpmFree(xpmAttr.pixels);
             XpmFreeAttributes(&xpmAttr);
             XImageFree(ximage);
         }
@@ -96,7 +122,7 @@ bool wxXPMFileHandler::LoadFile(wxBitmap *bitmap,
 
         return bitmap->Ok();
     }
-#endif
+#endif // wxUSE_XPM_IN_MSW
 
     return FALSE;
 }
@@ -107,11 +133,9 @@ bool wxXPMFileHandler::SaveFile(wxBitmap *bitmap,
                                 const wxPalette *palette)
 {
 #if wxUSE_XPM_IN_MSW
-    HDC     dc = NULL;
-
     XImage ximage;
 
-    dc = CreateCompatibleDC(NULL);
+    HDC dc = CreateCompatibleDC(NULL);
     if (dc)
     {
         if ( SelectObject(dc, GetHbitmapOf(*bitmap)) )
@@ -132,13 +156,11 @@ bool wxXPMFileHandler::SaveFile(wxBitmap *bitmap,
 
             if (errorStatus == XpmSuccess)
                 return TRUE;    /* no error */
-            else
+        }
+    }
+#endif // !wxUSE_XPM_IN_MSW
+
     return FALSE;
-        } else return FALSE;
-    } else return FALSE;
-#else
-    return FALSE;
-#endif
 }
 
 IMPLEMENT_DYNAMIC_CLASS(wxXPMDataHandler, wxBitmapHandler)
@@ -152,33 +174,27 @@ bool wxXPMDataHandler::Create(wxBitmap *bitmap,
 {
 #if wxUSE_XPM_IN_MSW
   XImage *ximage;
-  int     ErrorStatus;
   XpmAttributes xpmAttr;
-  HDC     dc;
 
-  dc = CreateCompatibleDC(NULL);  /* memory DC */
+  HDC dc = CreateCompatibleDC(NULL);  /* memory DC */
 
   if (dc)
   {
-    xpmAttr.valuemask = XpmReturnInfos; /* get infos back */
+      xpmAttr.valuemask = XpmReturnInfos | XpmColorTable;
-    ErrorStatus = XpmCreateImageFromData(&dc, (char **)data,
+      int errorStatus = XpmCreateImageFromData(&dc, (char **)data,
-                                         &ximage, (XImage **) NULL, &xpmAttr);
+                                               &ximage,
+                                               (XImage **) NULL,
+                                               &xpmAttr);
+      DeleteDC(dc);
 
-    if (ErrorStatus == XpmSuccess)
+      if ( errorStatus == XpmSuccess )
       {
           XpmToBitmap(bitmap, ximage, xpmAttr);
 
+          XpmFree(xpmAttr.pixels);
           XpmFreeAttributes(&xpmAttr);
-
+          XImageFree(ximage); // releases the malloc, but does not destroy bitmap
-      XImageFree(ximage); // releases the malloc, but does not detroy
-                          // the bitmap
       }
-    else
-    {
-      //    XpmDebugError(ErrorStatus, NULL);
-    }
-
-    DeleteDC(dc);
 
 #if WXWIN_COMPATIBILITY_2
       bitmap->SetOk(errorStatus == XpmSuccess);
@@ -186,7 +202,7 @@ bool wxXPMDataHandler::Create(wxBitmap *bitmap,
 
       return bitmap->Ok();
   }
-#endif
+#endif // wxUSE_XPM_IN_MSW
 
   return FALSE;
 }