diff --git a/docs/changes.txt b/docs/changes.txt index c6bba1ebd9..7653c3a2f5 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -129,6 +129,10 @@ Changes in behaviour not resulting in compilation errors - wxSpinCtrl::SetValue(wxString) overload doesn't generate any events with wxMSW, which was already the documented behaviour. +- wxButton::GetBitmap{Current,Disabled,Focus,Pressed}() only return valid + bitmaps in wxMSW if the corresponding Set had been called before, as in the + other ports, instead of returning the normal bitmap as fallback in this case. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/include/wx/osx/bitmap.h b/include/wx/osx/bitmap.h index d1388a85fb..bae0e68537 100644 --- a/include/wx/osx/bitmap.h +++ b/include/wx/osx/bitmap.h @@ -204,16 +204,17 @@ public: // returns a CGImageRef which must released after usage with CGImageRelease CGImageRef CreateCGImage() const ; - WXImage GetImage() const; + // returns nil for invalid bitmap + WXImage OSXGetImage() const; #if wxOSX_USE_COCOA // returns an autoreleased version of the image WX_NSImage GetNSImage() const - { return GetImage(); } + { return OSXGetImage(); } #endif #if wxOSX_USE_IPHONE // returns an autoreleased version of the image WX_UIImage GetUIImage() const - { return GetImage(); } + { return OSXGetImage(); } #endif #if WXWIN_COMPATIBILITY_3_0 diff --git a/interface/wx/anybutton.h b/interface/wx/anybutton.h index c993b062e2..29c0eb12ce 100644 --- a/interface/wx/anybutton.h +++ b/interface/wx/anybutton.h @@ -41,10 +41,10 @@ public: wxBitmap GetBitmap() const; /** - Returns the bitmap used when the mouse is over the button, which may be - invalid. + Returns the bitmap used when the mouse is over the button. - @see SetBitmapCurrent() + The returned bitmap is only valid if SetBitmapCurrent() had been + previously called. @since 2.9.1 (available as wxBitmapButton::GetBitmapHover() in previous versions) @@ -52,18 +52,20 @@ public: wxBitmap GetBitmapCurrent() const; /** - Returns the bitmap for the disabled state, which may be invalid. + Returns the bitmap used for the disabled state. - @see SetBitmapDisabled() + The returned bitmap is only valid if SetBitmapDisabled() had been + previously called. @since 2.9.1 (available in wxBitmapButton only in previous versions) */ wxBitmap GetBitmapDisabled() const; /** - Returns the bitmap for the focused state, which may be invalid. + Returns the bitmap used for the focused state. - @see SetBitmapFocus() + The returned bitmap is only valid if SetBitmapFocus() had been + previously called. @since 2.9.1 (available in wxBitmapButton only in previous versions) */ @@ -82,9 +84,10 @@ public: wxBitmap GetBitmapLabel() const; /** - Returns the bitmap for the pressed state, which may be invalid. + Returns the bitmap used when the button is pressed. - @see SetBitmapPressed() + The returned bitmap is only valid if SetBitmapPressed() had been + previously called. @since 2.9.1 (available as wxBitmapButton::GetBitmapSelected() in previous versions) diff --git a/src/msw/anybutton.cpp b/src/msw/anybutton.cpp index 67c7640b7b..21c9bd0823 100644 --- a/src/msw/anybutton.cpp +++ b/src/msw/anybutton.cpp @@ -101,11 +101,45 @@ extern wxWindowMSW *wxWindowBeingErased; // From src/msw/window.cpp class wxButtonImageData: public wxObject { public: - wxButtonImageData() { } + wxButtonImageData() + { + for ( int n = 0; n < wxAnyButton::State_Max; ++n ) + { + // The normal bitmap is always set explicitly when the image data + // is created, but the others ones are not (yet). + m_bitmapSetExplicitly[n] = n == wxAnyButton::State_Normal; + } + } + virtual ~wxButtonImageData() { } - virtual wxBitmap GetBitmap(wxAnyButton::State which) const = 0; - virtual void SetBitmap(const wxBitmap& bitmap, wxAnyButton::State which) = 0; + // Bitmap can be set either explicitly, when the bitmap for the given state + // is specified by the application, or implicitly, when the bitmap for some + // state is set as a side effect of setting another bitmap. + // + // These functions check the flags stored in the base class remembering + // whether each bitmap is implicit or explicit and behave accordingly. + wxBitmap GetExplicitBitmap(wxAnyButton::State which) const + { + return m_bitmapSetExplicitly[which] ? GetBitmap(which) : wxBitmap(); + } + + void SetExplicitBitmap(const wxBitmap& bitmap, wxAnyButton::State which) + { + SetBitmap(bitmap, which); + m_bitmapSetExplicitly[which] = true; + } + + wxBitmap GetImplicitBitmap(wxAnyButton::State which) const + { + return GetBitmap(which); + } + + void SetImplicitBitmap(const wxBitmap& bitmap, wxAnyButton::State which) + { + SetBitmap(bitmap, which); + m_bitmapSetExplicitly[which] = false; + } virtual wxSize GetBitmapMargins() const = 0; virtual void SetBitmapMargins(wxCoord x, wxCoord y) = 0; @@ -114,6 +148,13 @@ public: virtual void SetBitmapPosition(wxDirection dir) = 0; private: + // These functions are private to force using explicit/implicit versions of + // them in the code to make it clear which bitmap is needed. + virtual wxBitmap GetBitmap(wxAnyButton::State which) const = 0; + virtual void SetBitmap(const wxBitmap& bitmap, wxAnyButton::State which) = 0; + + bool m_bitmapSetExplicitly[wxAnyButton::State_Max]; + wxDECLARE_NO_COPY_CLASS(wxButtonImageData); }; @@ -129,7 +170,7 @@ class wxODButtonImageData : public wxButtonImageData public: wxODButtonImageData(wxAnyButton *btn, const wxBitmap& bitmap) { - SetBitmap(bitmap, wxAnyButton::State_Normal); + SetExplicitBitmap(bitmap, wxAnyButton::State_Normal); #if wxUSE_IMAGE SetBitmap(bitmap.ConvertToDisabled(), wxAnyButton::State_Disabled); #endif @@ -505,7 +546,7 @@ void wxAnyButton::AdjustForBitmapSize(wxSize &size) const wxCHECK_RET( m_imageData, wxT("shouldn't be called if no image") ); // account for the bitmap size, including the user-specified margins - const wxSize sizeBmp = m_imageData->GetBitmap(State_Normal).GetSize() + const wxSize sizeBmp = m_imageData->GetImplicitBitmap(State_Normal).GetSize() + 2*m_imageData->GetBitmapMargins(); const wxDirection dirBmp = m_imageData->GetBitmapPosition(); if ( dirBmp == wxLEFT || dirBmp == wxRIGHT ) @@ -660,7 +701,7 @@ WXLRESULT wxAnyButton::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPar if ( IsEnabled() && ( wxUxThemeIsActive() || - (m_imageData && m_imageData->GetBitmap(State_Current).IsOk()) + (m_imageData && m_imageData->GetImplicitBitmap(State_Current).IsOk()) ) ) { @@ -678,7 +719,7 @@ WXLRESULT wxAnyButton::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPar wxBitmap wxAnyButton::DoGetBitmap(State which) const { - return m_imageData ? m_imageData->GetBitmap(which) : wxBitmap(); + return m_imageData ? m_imageData->GetExplicitBitmap(which) : wxBitmap(); } void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) @@ -697,8 +738,8 @@ void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) else { // Replace the removed bitmap with the normal one. - wxBitmap bmpNormal = m_imageData->GetBitmap(State_Normal); - m_imageData->SetBitmap(which == State_Disabled + wxBitmap bmpNormal = m_imageData->GetImplicitBitmap(State_Normal); + m_imageData->SetImplicitBitmap(which == State_Disabled ? bmpNormal.ConvertToDisabled() : bmpNormal, which); @@ -714,9 +755,9 @@ void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) // Check if we already had bitmaps of different size. if ( m_imageData && - bitmap.GetSize() != m_imageData->GetBitmap(State_Normal).GetSize() ) + bitmap.GetSize() != m_imageData->GetImplicitBitmap(State_Normal).GetSize() ) { - wxASSERT_MSG( (which == State_Normal) || bitmap.IsNull(), + wxASSERT_MSG( which == State_Normal, "Must set normal bitmap with the new size first" ); #if wxUSE_UXTHEME @@ -767,7 +808,7 @@ void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) } else { - m_imageData->SetBitmap(bitmap, which); + m_imageData->SetExplicitBitmap(bitmap, which); // if the focus bitmap is specified but current one isn't, use // the focus bitmap for hovering as well if this is consistent @@ -777,9 +818,9 @@ void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) // and also makes it much easier to do "the right thing" for // all platforms (some of them, such as Windows, have "hot" // buttons while others don't) - if ( which == State_Focused && !m_imageData->GetBitmap(State_Current).IsOk() ) + if ( which == State_Focused && !m_imageData->GetExplicitBitmap(State_Current).IsOk() ) { - m_imageData->SetBitmap(bitmap, State_Current); + m_imageData->SetImplicitBitmap(bitmap, State_Current); } } @@ -1226,11 +1267,16 @@ void wxAnyButton::MakeOwnerDrawn() // if necessary. if ( m_imageData && wxDynamicCast(m_imageData, wxODButtonImageData) == NULL ) { - wxODButtonImageData* newData = new wxODButtonImageData(this, m_imageData->GetBitmap(State_Normal)); + wxODButtonImageData* newData = new wxODButtonImageData(this, m_imageData->GetImplicitBitmap(State_Normal)); for ( int n = 0; n < State_Max; n++ ) { State st = static_cast(n); - newData->SetBitmap(m_imageData->GetBitmap(st), st); + + wxBitmap bmp = m_imageData->GetExplicitBitmap(st); + if ( bmp.IsOk() ) + newData->SetExplicitBitmap(bmp, st); + else + newData->SetImplicitBitmap(m_imageData->GetImplicitBitmap(st), st); } newData->SetBitmapPosition(m_imageData->GetBitmapPosition()); wxSize margs = m_imageData->GetBitmapMargins(); @@ -1390,9 +1436,9 @@ bool wxAnyButton::MSWOnDraw(WXDRAWITEMSTRUCT *wxdis) // draw the image, if any if ( m_imageData ) { - wxBitmap bmp = m_imageData->GetBitmap(GetButtonState(this, state)); + wxBitmap bmp = m_imageData->GetImplicitBitmap(GetButtonState(this, state)); if ( !bmp.IsOk() ) - bmp = m_imageData->GetBitmap(State_Normal); + bmp = m_imageData->GetImplicitBitmap(State_Normal); const wxSize sizeBmp = bmp.GetSize(); const wxSize margin = m_imageData->GetBitmapMargins(); diff --git a/src/osx/carbon/graphics.cpp b/src/osx/carbon/graphics.cpp index 8dfcacacef..77585ff9d0 100644 --- a/src/osx/carbon/graphics.cpp +++ b/src/osx/carbon/graphics.cpp @@ -2419,7 +2419,7 @@ void wxMacCoreGraphicsContext::DrawBitmap( const wxBitmap &bmp, wxDouble x, wxDo if (EnsureIsValid()) { CGRect r = CGRectMake( (CGFloat) x , (CGFloat) y , (CGFloat) w , (CGFloat) h ); - wxOSXDrawNSImage( m_cgContext, &r, bmp.GetImage()); + wxOSXDrawNSImage( m_cgContext, &r, bmp.GetNSImage()); } #else wxGraphicsBitmap bitmap = GetRenderer()->CreateBitmap(bmp); @@ -2482,7 +2482,7 @@ void wxMacCoreGraphicsContext::DrawIcon( const wxIcon &icon, wxDouble x, wxDoubl #if wxOSX_USE_COCOA { CGRect r = CGRectMake( (CGFloat) x , (CGFloat) y , (CGFloat) w , (CGFloat) h ); - wxOSXDrawNSImage( m_cgContext, &r, icon.GetImage()); + wxOSXDrawNSImage( m_cgContext, &r, icon.GetNSImage()); } #endif diff --git a/src/osx/cocoa/dataview.mm b/src/osx/cocoa/dataview.mm index 3b603a36e9..ac596c5114 100644 --- a/src/osx/cocoa/dataview.mm +++ b/src/osx/cocoa/dataview.mm @@ -3438,10 +3438,7 @@ bool wxDataViewIconTextRenderer::MacRender() cell = (wxImageTextCell*) GetNativeData()->GetItemCell(); iconText << GetValue(); - if (iconText.GetIcon().IsOk()) - [cell setImage:wxBitmap(iconText.GetIcon()).GetNSImage()]; - else - [cell setImage:nil]; + [cell setImage:iconText.GetIcon().GetNSImage()]; [cell setStringValue:wxCFStringRef(iconText.GetText()).AsNSString()]; return true; } diff --git a/src/osx/cocoa/window.mm b/src/osx/cocoa/window.mm index de39ab8afe..8858d3aaee 100644 --- a/src/osx/cocoa/window.mm +++ b/src/osx/cocoa/window.mm @@ -3444,11 +3444,7 @@ void wxWidgetCocoaImpl::SetBitmap( const wxBitmap& bitmap ) { if ( [m_osxView respondsToSelector:@selector(setImage:)] ) { - if (bitmap.IsOk()) - [m_osxView setImage:bitmap.GetNSImage()]; - else - [m_osxView setImage:nil]; - + [m_osxView setImage:bitmap.GetNSImage()]; [m_osxView setNeedsDisplay:YES]; } } diff --git a/src/osx/core/bitmap.cpp b/src/osx/core/bitmap.cpp index cb9f2163e3..d7211a50e4 100644 --- a/src/osx/core/bitmap.cpp +++ b/src/osx/core/bitmap.cpp @@ -782,7 +782,7 @@ wxBitmapRefData::~wxBitmapRefData() bool wxBitmap::CopyFromIcon(const wxIcon& icon) { - return Create( icon.GetImage() ); + return Create( icon.OSXGetImage() ); } wxBitmap::wxBitmap(const char bits[], int the_width, int the_height, int no_bits) @@ -949,9 +949,9 @@ bool wxBitmap::Create(CGContextRef bitmapcontext) return GetBitmapData()->IsOk() ; } -WXImage wxBitmap::GetImage() const +WXImage wxBitmap::OSXGetImage() const { - return GetBitmapData()->GetImage(); + return IsOk() ? GetBitmapData()->GetImage() : NULL; } wxBitmap wxBitmap::GetSubBitmap(const wxRect &rect) const diff --git a/src/osx/iphone/button.mm b/src/osx/iphone/button.mm index 6db36c9852..4fe7f56de3 100644 --- a/src/osx/iphone/button.mm +++ b/src/osx/iphone/button.mm @@ -111,7 +111,7 @@ wxWidgetImplType* wxWidgetImpl::CreateBitmapButton( wxWindowMac* wxpeer, v.frame = r; if (bitmap.IsOk()) - [v setImage:bitmap.GetImage() forState:UIControlStateNormal]; + [v setImage:bitmap.GetUIImage() forState:UIControlStateNormal]; wxWidgetIPhoneImpl* c = new wxWidgetIPhoneImpl( wxpeer, v ); return c; diff --git a/tests/controls/buttontest.cpp b/tests/controls/buttontest.cpp index c6a5725646..ce958bc5e9 100644 --- a/tests/controls/buttontest.cpp +++ b/tests/controls/buttontest.cpp @@ -156,10 +156,26 @@ TEST_CASE_METHOD(ButtonTestCase, "Button::Bitmap", "[button]") CHECK(m_button->GetBitmap().IsOk()); + // The call above shouldn't affect any other bitmaps as returned by the API + // even though the same (normal) bitmap does appear for all the states. + CHECK( !m_button->GetBitmapCurrent().IsOk() ); + CHECK( !m_button->GetBitmapDisabled().IsOk() ); + CHECK( !m_button->GetBitmapFocus().IsOk() ); + CHECK( !m_button->GetBitmapPressed().IsOk() ); + + // Do set one of the bitmaps now. + m_button->SetBitmapPressed(wxArtProvider::GetBitmap(wxART_ERROR)); + CHECK( m_button->GetBitmapPressed().IsOk() ); + // Check that resetting the button label doesn't result in problems when // updating the bitmap later, as it used to be the case in wxGTK (#18898). m_button->SetLabel(wxString()); CHECK_NOTHROW( m_button->Disable() ); + + // Also check that setting an invalid bitmap doesn't do anything untoward, + // such as crashing, as it used to do in wxOSX (#19257). + CHECK_NOTHROW( m_button->SetBitmapPressed(wxNullBitmap) ); + CHECK( !m_button->GetBitmapPressed().IsOk() ); } #endif //wxUSE_BUTTON