wxToolBar & wxNullBitmap

[Carsten Grohmann]

Hi,

I'm developing wxGlade and I've some trouble with a unittest for wxPython3. The unittest runs fine with wx2.8.

The root cause is probably in logical changes in wxToolBarTool::SetImage. The code used in 2.8 accepted also wxNullBitmaps but it didn't do anything with wxNullBitmaps. The new code accepts valid bitmaps (IsOK() == True).

From my expectations, wxNullBitmap is also an valid bitmap and should be accepted in SetImage too.

It it possible to add a more relaxed bitmap check like "IsOK() or IsNull()"?

My current configuration:
- Python 2.7.9
- wxPython 3.0.2 + wxgtk 3.0.2
- wxPython 2.8.12.1 + wxgtk 2.8.12.1
- ArchLinux

The Python error message
========================
# python2 AllWidgets_28.py
AllWidgets_28.py:331: wxPyDeprecationWarning: Using deprecated class PySimpleApp.
AllWidgets28App = wx.PySimpleApp(0)
Traceback (most recent call last):
File "AllWidgets_28.py", line 332, in <module>
All_Widgets = All_Widgets_Frame(None, wx.ID_ANY, "")
File "AllWidgets_28.py", line 37, in __init__
self.All_Widgets_toolbar.AddLabelTool(wx.ID_OPEN, _("Open"), wx.NullBitmap, wx.NullBitmap, wx.ITEM_NORMAL, _("Open a new file"), _("Open a new file"))
File "/usr/lib/python2.7/site-packages/wx-3.0-gtk2/wx/_controls.py", line 3678, in AddLabelTool
shortHelp, longHelp, clientData)
File "/usr/lib/python2.7/site-packages/wx-3.0-gtk2/wx/_controls.py", line 3600, in DoAddTool
return _controls_.ToolBarBase_DoAddTool(*args, **kwargs)
wx._core.PyAssertionError: C++ assertion "bitmap.IsOk()" failed at ./src/gtk/toolbar.cpp(262) in SetImage(): invalid bitmap for wxToolBar icon

The related Python code
=======================
self.All_Widgets_toolbar = wx.ToolBar(self, -1)
self.SetToolBar(self.All_Widgets_toolbar)
self.All_Widgets_toolbar.AddLabelTool(wx.ID_OPEN, _("Open"), wx.NullBitmap, wx.NullBitmap, wx.ITEM_NORMAL, _("Open a new file"), _("Open a new file"))
self.All_Widgets_toolbar.Realize()


New code of SetImage()
======================
void wxToolBarTool::SetImage()
{
const wxBitmap& bitmap = GetNormalBitmap();
wxCHECK_RET(bitmap.IsOk(), "invalid bitmap for wxToolBar icon");

GtkWidget* image = gtk_tool_button_get_icon_widget(GTK_TOOL_BUTTON(m_item));
// always use pixbuf, because pixmap mask does not
// work with disabled images in some themes
gtk_image_set_from_pixbuf(GTK_IMAGE(image), bitmap.GetPixbuf());
}

Old code of SetImage()
======================
void SetImage(const wxBitmap& bitmap)
{
if (bitmap.Ok())
{
// setting from pixmap doesn't seem to work right, but pixbuf works well
gtk_image_set_from_pixbuf((GtkImage*)m_image, bitmap.GetPixbuf());
}
}

Regards,
Carsten

[Vadim Zeitlin]

On Fri, 3 Apr 2015 19:03:43 +0200 Carsten Grohmann wrote:

CG> The root cause is probably in logical changes in
CG> wxToolBarTool::SetImage. The code used in 2.8 accepted also
CG> wxNullBitmaps but it didn't do anything with wxNullBitmaps. The new
CG> code accepts valid bitmaps (IsOK() == True).

Hello,

I don't know why was this change done (commit a1cb0b11 which did this
doesn't explain the reason for it, unfortunately), but it seems reasonable
to me. I'd only consider reverting it if other ports, notably MSW, accept
invalid bitmaps here. Do they?

CG> From my expectations, wxNullBitmap is also an valid bitmap and should
CG> be accepted in SetImage too.

Sorry but it seems pretty clear that wxNullBitmap is an invalid bitmap,
it's almost a definition of being invalid -- if this one were valid, there
would be no invalid bitmaps at all!

CG> It it possible to add a more relaxed bitmap check like "IsOK() or IsNull()"?

I'm against it because it doesn't make sense and I don't even think there
are any reasonable backwards compatibility considerations in this case: in
the best case, the toolbar tool wouldn't look right, which hardly seems
like something anybody would rely on.

What's wrong with just changing the unit test?
VZ

[Carsten Grohmann]

Hi Vadim,

Am 03.04.2015 20:00 schrieb Vadim Zeitlin:
> I don't know why was this change done (commit a1cb0b11 which did this
> doesn't explain the reason for it, unfortunately), but it seems
> reasonable to me. I'd only consider reverting it if other ports,
> notably MSW, accept invalid bitmaps here. Do they?

It's not exactly the same error on MSW, but it's very similar:

# python AllWidgets_28.py
AllWidgets_28.py:327: wxPyDeprecationWarning: Using deprecated class PySimpleApp

.
AllWidgets28App = wx.PySimpleApp(0)
Traceback (most recent call last):

File "AllWidgets_28.py", line 328, in <module>

All_Widgets = All_Widgets_Frame(None, wx.ID_ANY, "")

File "AllWidgets_28.py", line 124, in __init__
self.__set_properties()
File "AllWidgets_28.py", line 140, in __set_properties
self.All_Widgets_toolbar.Realize()
File "C:\Python27\lib\site-packages\wx-3.0-msw\wx\_controls.py", line 3797, in
Realize
return _controls_.ToolBarBase_Realize(*args, **kwargs)
wx._core.PyAssertionError: C++ assertion "Assert failure" failed at ..\..\src\ms
w\toolbar.cpp(801) in wxToolBar::Realize(): invalid tool button bitmap

Looks like using wxNullBitmaps is a bad idea, even if it was possible on 2.8. I'll stop using NullBitmaps in my unittests now.

Thank you for your support,
Carsten