#10657: wxCursor regression

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

#10657: wxCursor regression

wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657>

#10657: wxCursor regression
----------------------+-----------------------------------------------------
 Reporter:  dghart    |       Owner:        
     Type:  defect    |      Status:  new    
 Priority:  normal    |   Milestone:  2.9.0  
Component:  wxGTK     |     Version:  2.9-svn
 Keywords:  wxCursor  |   Blockedby:        
    Patch:  1         |    Blocking:        
----------------------+-----------------------------------------------------
 In <2.9, the following code worked:
 wxCursor mycursor = wxCursor( wxT("/path/to/cursor.png") );
 In 2.9.0 it fails silently, resulting in a crash and X window system error
 when the cursor is used.

 The difference is that prior to 2.9 (perhaps because wxCursor was then
 derived from wxBitmap?) the filepath goes first to the wxImage ctor; after
 this the wxCursor(const wxImage&) ctor is used.
 In 2.9 this doesn't happen; instead the filepath goes straight to the
 wxCursor(const wxString& cursorName,  wxBitmapType,int,int) ctor, which
 fails for one or more reasons:
  1) This ctor uses gdk_pixmap_create_from_xpm, and states that only xpms
 will work. However it tests for this only by checking the value of
 wxBitmapType, which defaults to wxBITMAP_TYPE_XPM. So passing e.g. a png
 without specifying its type won't trigger the wxLogError message. On
 stepping through the ctor, nothing seems to go wrong; but the resulting
 cursor presumably contains garbage.
  2) More worryingly, testing with an .xpm file also failed silently,
 crashing with the same X message.

 A workaround is to change the user code to wxCursor mycursor = wxCursor(
 wxImage("/path/to/cursor.png") ); so you might consider this a
 documentation bug.

 I've attached a diff to the minimal sample that demonstrates problem 2).
 The inline xpm works; a filepath containing the same xpm crashes; but wrap
 the filepath with wxImage() and it works again. (You'll need to alter the
 diff so the filepath is correct for your installation, of course.)

 Debian lenny, gtk 2.12.11


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes (was: wxCursor regression)

wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:2>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:                    
      Type:  defect  |      Status:  confirmed          
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:          |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------
Changes (by vadz):

  * keywords:  wxCursor => wxCursor regression
  * priority:  normal => high
  * status:  new => confirmed


Comment:

 Apparently this code was never tested (there is a comment {{{ TODO: test
 this code! }}} there) and so it's not surprising that it doesn't work. I
 can reproduce the crash with just
 {{{
 --- samples/minimal/minimal.cpp (revision 60362)
 +++ samples/minimal/minimal.cpp (working copy)
 @@ -147,6 +147,7 @@
  {
      // set the frame icon
      SetIcon(wxICON(sample));
 +
 SetCursor(wxCursor("/usr/local/src/wx/svn/trunk/samples/sample.xpm"));

  #if wxUSE_MENUS
      // create a menu bar
 }}}
 (you'd need to adjust the path to point to the valid file name, of
 course...).

 I don't really see what exactly goes wrong here to be honest but before
 spending time on debugging/fixing this code I wonder why do we need it at
 all. I.e. why not just always pass by wxImage or at least wxBitmap?

 It would be great if someone familiar with this code (Robert? Francesco?)
 could have a look at it.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:2>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:3>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  accepted          
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:          |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------
Changes (by frm):

  * owner:  => frm
  * status:  confirmed => accepted



--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:3>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:4>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  accepted          
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:          |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------

Comment(by frm):

 Replying to [comment:2 vadz]:
 > I don't really see what exactly goes wrong here to be honest but before
 spending time on debugging/fixing this code I wonder why do we need it at
 all. I.e. why not just always pass by wxImage or at least wxBitmap?
 I couldn't find what is wrong with current implementation which causes an
 X error (I suspect some GDK bug passing the NULL mask to X server without
 checking it against NULL).

 Anyway since that ctor doesn't specify a mask and GTK+ needs it, I'm
 changing it to reuse wxImage ctor code.

 For those who wonder why I did add that (untested) ctor the reason is that
 it was a ctor documented to be present for all ports but it was missing on
 wxGTK...

 @dghart: if you could test your code against the latest SVN and eventually
 report any (other) problem with wxCursor, it would be nice. TIA!


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:4>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:5>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  closed            
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:  fixed   |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------
Changes (by frm):

  * status:  accepted => closed
  * resolution:  => fixed


Comment:

 Fixed in r60648


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:5>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:6>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  closed            
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:  fixed   |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------

Comment(by dghart):

 Replying to [comment:4 frm]:
 > > I don't really see what exactly goes wrong here to be honest but
 before spending time on debugging/fixing this code I wonder why do we need
 it at all. I.e. why not just always pass by wxImage or at least wxBitmap?
 > I couldn't find what is wrong with current implementation which causes
 an X error (I suspect some GDK bug passing the NULL mask to X server
 without checking it against NULL).
 >//
 > @dghart: if you could test your code against the latest SVN and
 eventually report any (other) problem with wxCursor, it would be nice.
 TIA!

 Hi Francesco,

 Many thanks for the fix, which is 99% successful:
   mycursor = wxCursor("/pathto/cursor.xpm"); now works
   mycursor = wxCursor("/pathto/cursor.png", wxBITMAP_TYPE_PNG); also works

 wxCursor("/pathto/cursor.png"), which used to work <2.9, still fails.
 However it no longer crashes; and there ''is'' now an "Image file is not
 of type 9" error dialog, which hints at the reason.

 I wonder if there should also be a wxASSERT after cursor.cpp:73 if
 (!img.LoadFile(cursor_file, type))


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:6>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:7>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  closed            
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:  fixed   |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------

Comment(by frm):

 Replying to [comment:6 dghart]:
 > Replying to [comment:4 frm]:
 > > > I don't really see what exactly goes wrong here to be honest but
 before spending time on debugging/fixing this code I wonder why do we need
 it at all. I.e. why not just always pass by wxImage or at least wxBitmap?
 > > I couldn't find what is wrong with current implementation which causes
 an X error (I suspect some GDK bug passing the NULL mask to X server
 without checking it against NULL).
 > >//
 > > @dghart: if you could test your code against the latest SVN and
 eventually report any (other) problem with wxCursor, it would be nice.
 TIA!
 >
 > Hi Francesco,
 >
 > Many thanks for the fix, which is 99% successful:
 >   mycursor = wxCursor("/pathto/cursor.xpm"); now works
 >   mycursor = wxCursor("/pathto/cursor.png", wxBITMAP_TYPE_PNG); also
 works
 >
 > wxCursor("/pathto/cursor.png"), which used to work <2.9, still fails.
 However it no longer crashes; and there ''is'' now an "Image file is not
 of type 9" error dialog, which hints at the reason.
 >
 > I wonder if there should also be a wxASSERT after cursor.cpp:73 if
 (!img.LoadFile(cursor_file, type))


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:7>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: #10657: wxGTK regression: loading wxCursor from XPM file crashes

wxTrac
In reply to this post by wxTrac
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:8>

#10657: wxGTK regression: loading wxCursor from XPM file crashes
---------------------+------------------------------------------------------
  Reporter:  dghart  |       Owner:  frm                
      Type:  defect  |      Status:  closed            
  Priority:  high    |   Milestone:  2.9.1              
 Component:  wxGTK   |     Version:  2.9-svn            
Resolution:  fixed   |    Keywords:  wxCursor regression
 Blockedby:          |       Patch:  1                  
  Blocking:          |  
---------------------+------------------------------------------------------

Comment(by frm):

 Replying to [comment:6 dghart]:
 > wxCursor("/pathto/cursor.png"), which used to work <2.9, still fails.
 However it no longer crashes; and there ''is'' now an "Image file is not
 of type 9" error dialog, which hints at the reason.
 the failure is ok I think.

 The problem is that before r55884, wxGTK was the only port missing the
 ctor taking const wxString&, wxBitmapType, etc parameters.
 Now doing wxCursor("/pathto/cursor.png") instead of
 wxCursor("/pathto/cursor.png", wxBITMAP_TYPE_PNG) fails on wxGTK exactly
 like it fails on wxMSW or on others platforms.
 Given that the missing ctor was documented, I think that was a needed
 change and that we can do nothing about backward compatibility in this
 case (in any case the logged error message seems quite explicit to me so
 it shouldn't be a big problem...).

 > I wonder if there should also be a wxASSERT after cursor.cpp:73 if
 (!img.LoadFile(cursor_file, type))
 Other ports (wxMSW in particular) do not assert nor log error messages for
 invalid cursor files; after all it's user's task to call IsOk() after
 using the ctor to check if everything went well...


--
Ticket URL: <http://trac.wxwidgets.org/ticket/10657#comment:8>
_______________________________________________
wx-dev mailing list
[hidden email]
http://lists.wxwidgets.org/mailman/listinfo/wx-dev
Loading...