Pcbnew crashes when changing value visibility

[Wim Lewis]

Pcbnew on OSX will reliably crash when changing the visibility of a module's value or reference via the popup menu.

To reproduce:
- Create a new board
- Place one module, e.g. SIL-10
- Switch back to the arrow tool
- Right-click on either the "SIL-10" text or the "Val**" text which is displayed next to the module
- Navigate to the submenu for the reference/value and select Edit
- Change the visibility to Invisible
- Click OK

Pcbnew will crash at this point.

It does not crash if you doubleclick on the module, then navigate through the dialog to change the visibility of the reference/value that way.

OSX 10.6, x86_64
Prebuilt pcbnew binary, (2013-mar-25)-stable, wxwidgets 2.9.5

I have attached the output of Apple's crash reporter which includes a stack trace. The proximate cause of the crash is an exception, "Unlocking Focus on wrong view (<wxNSView: 0x101652740>), expected <wxNSButton: 0x116748570>".

[Marco Serantoni (marco-serantoni)]

The issue is well know, and is due the interaction between wxwidgets and kicad's code.

http://trac.wxwidgets.org/ticket/14389

The wx-wigdget's view in the words of one of the main maintainer is:
"this is not really supported, actually if possible wxClientDCs should not be used anymore unless for measuring, everything else should be done in a EVT_PAINT handler, and if you really need wxClientDCs for tight updates in a loop, really keep them tight with no event handling interfering "

Hoping in a new GAL and/or an implementation of the drawing code with a wxGraphicsContext, that could help to a smoothest transition.

Anyway, I'll take a look soon in a deeper way.

[Andrew Plumb (andrew-plumb)]

Dunno if this will help, but if I disable the "m_currentText->Draw()" calls in pcbnew/dialogs/dialog_edit_module_text.cpp in the "void DialogEditModuleText::OnOkClick( wxCommandEvent& event )" procedure I don't get any crashes.

I have to manually refresh the screen to see the text edit take effect, but it's a small price to pay for stability.

I suspected the Draw() call because it could be generating some sort of wxPaintDC-type event outside what the initial wxClientDC object creation on pcbnew/onrightclick.cpp is able to manage. The longer-term fix is likely to pull the explicit Draw() refresh operation up to the end of PCB_EDIT_FRAME::OnRightClick() {}.

Here's my mini-patch to illustrate:

=== modified file 'pcbnew/dialogs/dialog_edit_module_text.cpp'
--- pcbnew/dialogs/dialog_edit_module_text.cpp 2013-05-05 07:17:48 +0000
+++ pcbnew/dialogs/dialog_edit_module_text.cpp 2013-10-11 18:01:41 +0000
@@ -183,7 +183,7 @@
     if ( m_module)
         m_parent->SaveCopyInUndoList( m_module, UR_CHANGED );

- if( m_dc ) //Erase old text on screen
+ if( 0 && m_dc ) //Erase old text on screen
     {
         m_currentText->Draw( m_parent->GetCanvas(), m_dc, GR_XOR,
                              (m_currentText->IsMoving()) ? MoveVector : wxPoint( 0, 0 ) );
@@ -238,7 +238,7 @@

     m_currentText->SetDrawCoord();

- if( m_dc ) // Display new text
+ if( 0 && m_dc ) // Display new text
     {
         m_currentText->Draw( m_parent->GetCanvas(), m_dc, GR_XOR,
                              (m_currentText->IsMoving()) ? MoveVector : wxPoint( 0, 0 ) );

[Marco Serantoni (marco-serantoni)]

Indeed is the correct diagnosys and cause, I wasn't expecting duplication of drawing code in Dialogs.
The correct way is to surround the two IFs with
#ifndef USE_WX_OVERLAY
if (..) {
 [..]
}
#endif

[Andrew Plumb (andrew-plumb)]

Confirmed via http://bazaar.launchpad.net/~andrew-plumb/kicad/kicad/revision/4381

The main gottcha now is the lack of an automated redraw. Perhaps some sort of "redraw when flag set" semaphore system could be implemented so that any dialog can set the flag and the top-level main window does a redraw+clear flag when necessary.

[Marco Serantoni (marco-serantoni)]

This is not a problem will be set soon.

[Marco Serantoni (marco-serantoni)]

Committed revision 4565 - Testing
Committed revision 4025 - Stable