From 675728e1c085bd109abce67a99d0f8a1ab11bba0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 20 Jun 2023 20:42:26 +0100 Subject: [PATCH 1/4] Fix crash when using wxDataObjectComposite with bitmap in wxMSW This reverts the changes of b1fad4da44 (Convert 0RGB wxBitmaps to RGB when copying them to clipboard, 2017-04-30) that added a cast from wxDataObject to wxBitmapDataObject for any object supporting bitmaps, which resulted in a crash when this happened to be wxDataObjectComposite and not wxBitmapDataObject, and does the same thing in wxBitmapDataObject itself which now converts 0RGB bitmaps to RGB ones when asked for the bitmap data. The main change is that the code using wxDataObjectComposite doesn't crash any longer, but another nice side effect is that this is possibly more efficient as well because the conversion could be skipped entirely if the bitmap is never pasted. See #17640. Closes #23564. --- src/msw/clipbrd.cpp | 21 --------------------- src/msw/ole/dataobj.cpp | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/msw/clipbrd.cpp b/src/msw/clipbrd.cpp index 8268221d59..885facb370 100644 --- a/src/msw/clipbrd.cpp +++ b/src/msw/clipbrd.cpp @@ -34,7 +34,6 @@ #include "wx/intl.h" #include "wx/log.h" #include "wx/dataobj.h" - #include "wx/dcmemory.h" #endif #if wxUSE_METAFILE @@ -585,26 +584,6 @@ bool wxClipboard::AddData( wxDataObject *data ) wxCHECK_MSG( data, false, wxT("data is invalid") ); - const wxDataFormat format = data->GetPreferredFormat(); - if ( format == wxDF_BITMAP || format == wxDF_DIB ) - { - wxBitmapDataObject* bmpData = (wxBitmapDataObject*)data; - wxBitmap bmp = bmpData->GetBitmap(); - wxASSERT_MSG( bmp.IsOk(), wxS("Invalid bitmap") ); - // Replace 0RGB bitmap with its RGB copy - // to ensure compatibility with applications - // not recognizing bitmaps in 0RGB format. - if ( bmp.GetDepth() == 32 && !bmp.HasAlpha() ) - { - wxBitmap bmpRGB(bmp.GetSize(), 24); - wxMemoryDC dc(bmpRGB); - dc.DrawBitmap(bmp, 0, 0); - dc.SelectObject(wxNullBitmap); - - bmpData->SetBitmap(bmpRGB); - } - } - #if wxUSE_OLE_CLIPBOARD HRESULT hr = OleSetClipboard(data->GetInterface()); if ( FAILED(hr) ) diff --git a/src/msw/ole/dataobj.cpp b/src/msw/ole/dataobj.cpp index 7740c15f6c..1b69a8439f 100644 --- a/src/msw/ole/dataobj.cpp +++ b/src/msw/ole/dataobj.cpp @@ -23,6 +23,7 @@ #if wxUSE_DATAOBJ #ifndef WX_PRECOMP + #include "wx/dcmemory.h" #include "wx/intl.h" #include "wx/log.h" #include "wx/utils.h" @@ -1042,9 +1043,41 @@ const wxChar *wxDataObject::GetFormatName(wxDataFormat format) // wxBitmapDataObject supports CF_DIB format // ---------------------------------------------------------------------------- +namespace +{ + +// Modify bitmap if necessary, i.e. if it uses 0RGB format in which alpha +// channel is present but is entirely 0, to make it just plain RGB, i.e. +// without alpha channel at all, to ensure compatibility with the applications +// not recognizing the special case of 0RGB and handling such bitmaps as +// completely transparent, see #17640. +void RemoveAlphaIfNecessary(wxBitmap& bmp) +{ + // Replace 0RGB bitmap with its RGB copy to ensure compatibility with + // applications not recognizing bitmaps in 0RGB format, see #17640. + if ( bmp.GetDepth() == 32 && !bmp.HasAlpha() ) + { + wxBitmap bmpRGB(bmp.GetSize(), 24); + { + wxMemoryDC dc(bmpRGB); + dc.DrawBitmap(bmp, 0, 0); + } + + bmp = bmpRGB; + } +} + +} // anonymous namespace + size_t wxBitmapDataObject::GetDataSize() const { #if wxUSE_WXDIB + wxBitmap& bmp = const_cast(this)->m_bitmap; + + // Note that we need to do this here too and not just in GetDataHere() + // because the size of the bitmap without the alpha channel is different. + RemoveAlphaIfNecessary(bmp); + return wxDIB::ConvertFromBitmap(nullptr, GetHbitmapOf(GetBitmap())); #else return 0; @@ -1054,9 +1087,13 @@ size_t wxBitmapDataObject::GetDataSize() const bool wxBitmapDataObject::GetDataHere(void *buf) const { #if wxUSE_WXDIB + wxBitmap& bmp = const_cast(this)->m_bitmap; + + RemoveAlphaIfNecessary(bmp); + BITMAPINFO * const pbi = (BITMAPINFO *)buf; - return wxDIB::ConvertFromBitmap(pbi, GetHbitmapOf(GetBitmap())) != 0; + return wxDIB::ConvertFromBitmap(pbi, GetHbitmapOf(bmp)) != 0; #else wxUnusedVar(buf); return false; From 8a95a93a1d788cb3546d89978270e4a41aaa6443 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 20 Jun 2023 20:52:08 +0100 Subject: [PATCH 2/4] Remove wxUSE_OLE_CLIPBOARD and always use OLE for wxClipboard It doesn't make sense to keep the old and almost never used alternative wxClipboard implementation which has known fatal bugs (such as casts of wxDataObject that could be wxDataObjectComposite to wxBitmapDataObject, see #23564) when wxUSE_OLE is never going to be disabled anyhow. Note that this means wxSetClipboardData() function doesn't exist at all any longer when previously it was available in the wxUSE_OLE==0 builds. As the official builds always used wxUSE_OLE==1, this doesn't look like a huge loss, but if needed, we could add it back later and reimplement it in terms of wxClipboard::SetData(). --- docs/doxygen/mainpages/const_wxusedef.h | 1 - include/wx/msw/chkconf.h | 9 + include/wx/msw/clipbrd.h | 5 - src/msw/clipbrd.cpp | 477 +----------------------- 4 files changed, 11 insertions(+), 481 deletions(-) diff --git a/docs/doxygen/mainpages/const_wxusedef.h b/docs/doxygen/mainpages/const_wxusedef.h index c34151c662..7749423c4b 100644 --- a/docs/doxygen/mainpages/const_wxusedef.h +++ b/docs/doxygen/mainpages/const_wxusedef.h @@ -323,7 +323,6 @@ library: manifest from wxWidgets RC file. See also wxUSE_RC_MANIFEST.} @itemdef{wxUSE_OLE, Enables OLE helper routines.} @itemdef{wxUSE_OLE_AUTOMATION, Enable OLE automation utilities.} -@itemdef{wxUSE_OLE_CLIPBOARD, Use OLE clipboard.} @itemdef{wxUSE_POSTSCRIPT_ARCHITECTURE_IN_MSW, Use PS printing in wxMSW.} @itemdef{wxUSE_PS_PRINTING, See src/msw/dcprint.cpp file.} @itemdef{wxUSE_RC_MANIFEST, Include manifest for common controls library v6 diff --git a/include/wx/msw/chkconf.h b/include/wx/msw/chkconf.h index d130da9993..8110a2e10d 100644 --- a/include/wx/msw/chkconf.h +++ b/include/wx/msw/chkconf.h @@ -302,6 +302,15 @@ # endif # endif +# if wxUSE_CLIPBOARD +# ifdef wxABORT_ON_CONFIG_ERROR +# error "wxUSE_CLIPBOARD requires wxUSE_OLE" +# else +# undef wxUSE_CLIPBOARD +# define wxUSE_CLIPBOARD 0 +# endif +# endif + # if wxUSE_DRAG_AND_DROP # ifdef wxABORT_ON_CONFIG_ERROR # error "wxUSE_DRAG_AND_DROP requires wxUSE_OLE" diff --git a/include/wx/msw/clipbrd.h b/include/wx/msw/clipbrd.h index de57970b76..5f74817af2 100644 --- a/include/wx/msw/clipbrd.h +++ b/include/wx/msw/clipbrd.h @@ -24,11 +24,6 @@ WXDLLIMPEXP_CORE bool wxCloseClipboard(); // get/set data WXDLLIMPEXP_CORE bool wxEmptyClipboard(); -#if !wxUSE_OLE -WXDLLIMPEXP_CORE bool wxSetClipboardData(wxDataFormat dataFormat, - const void *data, - int width = 0, int height = 0); -#endif // !wxUSE_OLE // clipboard formats WXDLLIMPEXP_CORE bool wxIsClipboardFormatAvailable(wxDataFormat dataFormat); diff --git a/src/msw/clipbrd.cpp b/src/msw/clipbrd.cpp index 885facb370..89a3c5151d 100644 --- a/src/msw/clipbrd.cpp +++ b/src/msw/clipbrd.cpp @@ -50,17 +50,7 @@ #include "wx/msw/dib.h" #endif -#if wxUSE_OLE - // use OLE clipboard - #define wxUSE_OLE_CLIPBOARD 1 -#else // !wxUSE_DATAOBJ - // use Win clipboard API - #define wxUSE_OLE_CLIPBOARD 0 -#endif - -#if wxUSE_OLE_CLIPBOARD - #include -#endif // wxUSE_OLE_CLIPBOARD +#include // =========================================================================== // implementation @@ -162,294 +152,6 @@ bool wxIsClipboardFormatAvailable(wxDataFormat dataFormat) } } - -#if !wxUSE_OLE_CLIPBOARD -namespace -{ -struct wxRawImageData -{ - size_t m_size; - void* m_data; -}; -} - -bool wxSetClipboardData(wxDataFormat dataFormat, - const void *data, - int width, int height) -{ - HANDLE handle = 0; // return value of SetClipboardData - - switch (dataFormat) - { - case wxDF_BITMAP: - { - wxBitmap *bitmap = (wxBitmap *)data; - - HDC hdcMem = CreateCompatibleDC(nullptr); - HDC hdcSrc = CreateCompatibleDC(nullptr); - HBITMAP old = (HBITMAP) - ::SelectObject(hdcSrc, (HBITMAP)bitmap->GetHBITMAP()); - HBITMAP hBitmap = CreateCompatibleBitmap(hdcSrc, - bitmap->GetWidth(), - bitmap->GetHeight()); - if (!hBitmap) - { - SelectObject(hdcSrc, old); - DeleteDC(hdcMem); - DeleteDC(hdcSrc); - return false; - } - - HBITMAP old1 = (HBITMAP) SelectObject(hdcMem, hBitmap); - BitBlt(hdcMem, 0, 0, bitmap->GetWidth(), bitmap->GetHeight(), - hdcSrc, 0, 0, SRCCOPY); - - // Select new bitmap out of memory DC - SelectObject(hdcMem, old1); - - // Set the data - handle = ::SetClipboardData(CF_BITMAP, hBitmap); - - // Clean up - SelectObject(hdcSrc, old); - DeleteDC(hdcSrc); - DeleteDC(hdcMem); - break; - } - -#if wxUSE_WXDIB - case wxDF_DIB: - { - wxBitmap *bitmap = (wxBitmap *)data; - - if ( bitmap && bitmap->IsOk() ) - { - wxDIB dib(*bitmap); - - if ( dib.IsOk() ) - { - DIBSECTION ds; - int n = ::GetObject(dib.GetHandle(), sizeof(DIBSECTION), &ds); - wxASSERT( n == sizeof(DIBSECTION) && ds.dsBm.bmBits ); - // Number of colours in the palette. - int numColors; - switch ( ds.dsBmih.biCompression ) - { - case BI_BITFIELDS: - numColors = 3; - break; - case BI_RGB: - case BI_RLE8: - case BI_RLE4: - numColors = ds.dsBmih.biClrUsed; - if ( !numColors ) - { - numColors = ds.dsBmih.biBitCount <= 8 ? 1 << ds.dsBmih.biBitCount : 0; - } - break; - default: - numColors = 0; - } - - unsigned long bmpSize = wxDIB::GetLineSize(ds.dsBmih.biWidth, ds.dsBmih.biBitCount) * - abs(ds.dsBmih.biHeight); - HANDLE hMem; - hMem = ::GlobalAlloc(GHND, ds.dsBmih.biSize + numColors*sizeof(RGBQUAD) + bmpSize); - if ( !hMem ) - break; - - { - GlobalPtrLock ptr(hMem); - char* pDst = (char*)ptr.Get(); - memcpy(pDst, &ds.dsBmih, ds.dsBmih.biSize); - pDst += ds.dsBmih.biSize; - if ( numColors > 0 ) - { - // Get colour table. - MemoryHDC hDC; - SelectInHDC sDC(hDC, dib.GetHandle()); - ::GetDIBColorTable(hDC, 0, numColors, (RGBQUAD*)pDst); - pDst += numColors*sizeof(RGBQUAD); - } - memcpy(pDst, dib.GetData(), bmpSize); - } // unlock hMem - - handle = ::SetClipboardData(CF_DIB, hMem); - } - } - break; - } -#endif - - // VZ: I'm told that this code works, but it doesn't seem to work for me - // and, anyhow, I'd be highly surprised if it did. So I leave it here - // but IMNSHO it is completely broken. -#if wxUSE_METAFILE && !defined(wxMETAFILE_IS_ENH) - case wxDF_METAFILE: - { - wxMetafile *wxMF = (wxMetafile *)data; - HANDLE data = GlobalAlloc(GHND, sizeof(METAFILEPICT) + 1); - { - GlobalPtrLock ptr(data); - METAFILEPICT *mf = (METAFILEPICT *)data.Get(); - - mf->mm = wxMF->GetWindowsMappingMode(); - mf->xExt = width; - mf->yExt = height; - mf->hMF = (HMETAFILE) wxMF->GetHMETAFILE(); - wxMF->SetHMETAFILE(nullptr); - } // unlock data - - handle = SetClipboardData(CF_METAFILEPICT, data); - break; - } -#endif // wxUSE_METAFILE - -#if wxUSE_ENH_METAFILE - case wxDF_ENHMETAFILE: - { - wxEnhMetaFile *emf = (wxEnhMetaFile *)data; - wxEnhMetaFile emfCopy = *emf; - - handle = SetClipboardData(CF_ENHMETAFILE, - (void *)emfCopy.GetHENHMETAFILE()); - } - break; -#endif // wxUSE_ENH_METAFILE - - case CF_SYLK: - case CF_DIF: - case CF_TIFF: - case CF_PALETTE: - default: - { - wxLogError(_("Unsupported clipboard format.")); - return false; - } - - case wxDF_OEMTEXT: - dataFormat = wxDF_TEXT; - // fall through - - case wxDF_TEXT: - { - char *s = (char *)data; - - width = strlen(s) + 1; - height = 1; - DWORD l = (width * height); - HANDLE hGlobalMemory = GlobalAlloc(GHND, l); - if ( hGlobalMemory ) - { - memcpy(GlobalPtrLock(hGlobalMemory), s, l); - } - - handle = SetClipboardData(dataFormat, hGlobalMemory); - break; - } - - case wxDF_UNICODETEXT: - { - LPWSTR s = (LPWSTR)data; - DWORD size = sizeof(WCHAR) * (lstrlenW(s) + 1); - HANDLE hGlobalMemory = ::GlobalAlloc(GHND, size); - if ( hGlobalMemory ) - { - memcpy(GlobalPtrLock(hGlobalMemory), s, size); - } - - handle = ::SetClipboardData(CF_UNICODETEXT, hGlobalMemory); - } - break; - - case wxDF_HTML: - { - char* html = (char *)data; - - // Create temporary buffer for HTML header... - char *buf = new char [400 + strlen(html)]; - if(!buf) return false; - - // Create a template string for the HTML header... - strcpy(buf, - "Version:0.9\r\n" - "StartHTML:00000000\r\n" - "EndHTML:00000000\r\n" - "StartFragment:00000000\r\n" - "EndFragment:00000000\r\n" - "\r\n" - "\r\n"); - - // Append the HTML... - strcat(buf, html); - strcat(buf, "\r\n"); - // Finish up the HTML format... - strcat(buf, - "\r\n" - "\r\n" - ""); - - // Now go back, calculate all the lengths, and write out the - // necessary header information. Note, wsprintf() truncates the - // string when you overwrite it so you follow up with code to replace - // the 0 appended at the end with a '\r'... - char *ptr = strstr(buf, "StartHTML"); - sprintf(ptr+10, "%08u", (unsigned)(strstr(buf, "") - buf)); - *(ptr+10+8) = '\r'; - - ptr = strstr(buf, "EndHTML"); - sprintf(ptr+8, "%08u", (unsigned)strlen(buf)); - *(ptr+8+8) = '\r'; - - ptr = strstr(buf, "StartFragment"); - sprintf(ptr+14, "%08u", (unsigned)(strstr(buf, "