← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Defer canvas type setting save until destruction of EDA_DRAW_FRAME

 

An alternative approach attached. Perhaps the two patches could be
combined for extra safety.

Cheers,
Orson

On 01/16/2018 08:15 PM, Maciej Suminski wrote:
> For me - not entirely, but definitely it is better than the current
> implementation. If you run KiCad for the first time and select 'Enable
> hardware acceleration' on a machine that does not support OpenGL, it
> switches to Cairo (correctly). Then, when you turn off KiCad, it will
> save OpenGL as the default canvas and you will not be able to run pcbnew
> again until you manually modify the configuration files (or remove them).
> 
> I will have another trial at fixing the problem, finally I have an
> environment where I can test different solutions.
> 
> Cheers,
> Orson
> 
> On 01/16/2018 07:04 PM, Chris Pavlina wrote:
>> Yes! You got it, works for me :)
>>
>> On Tue, Jan 16, 2018 at 05:16:57PM +0000, Chris Pavlina wrote:
>>> Sorry I disappeared for a while. I'm building on the machine where I
>>> experienced the bug right now.
>>>
>>> On Tue, Jan 16, 2018 at 04:26:16PM +0000, Jon Evans wrote:
>>>> Wayne/Orson, I'm not sure Chris will have time to review this, could you
>>>> please take a look?
>>>>
>>>> Thanks,
>>>> Jon
>>>>
>>>> On Thu, Jan 11, 2018 at 11:02 PM, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>>>>
>>>>> Updated patch attached (missed initializers in the original)
>>>>>
>>>>> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>>>>>
>>>>>> Hopefully this fixes:
>>>>>> https://bugs.launchpad.net/kicad/+bug/1741787
>>>>>> although I don't currently have a good way of reproducing the crash.
>>>>>>
>>>>>> Chris, if you can give it a try on your crashing machine that would be
>>>>>> greatly appreciated.
>>>>>>
>>>>>> Thanks
>>>>>> -Jon
>>>>>>
>>>>>
>>>>>
>>>
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

From 124678294d15286714f885c74eca7cfd51c0a4b4 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Wed, 17 Jan 2018 08:58:44 +0100
Subject: [PATCH] Fallback to Cairo when OpenGL is not supported

Fixes: lp:1741787
* https://bugs.launchpad.net/kicad/+bug/1741787
---
 common/draw_panel_gal.cpp     | 21 +++++++++------
 include/draw_frame.h          |  6 +++++
 include/wxBasePcbFrame.h      |  5 ++--
 pcbnew/basepcbframe.cpp       | 60 +++++++++++++++++++++----------------------
 pcbnew/moduleframe.cpp        |  6 ++---
 pcbnew/pcb_draw_panel_gal.cpp | 14 ++++++++++
 pcbnew/pcbframe.cpp           | 10 ++++----
 pcbnew/wxPcbStruct.h          |  2 +-
 8 files changed, 75 insertions(+), 49 deletions(-)

diff --git a/common/draw_panel_gal.cpp b/common/draw_panel_gal.cpp
index c98cc6412..4037ba1f7 100644
--- a/common/draw_panel_gal.cpp
+++ b/common/draw_panel_gal.cpp
@@ -121,7 +121,7 @@ EDA_DRAW_PANEL_GAL::~EDA_DRAW_PANEL_GAL()
 {
     StopDrawing();
 
-    assert( !m_drawing );
+    wxASSERT( !m_drawing );
 
     delete m_viewControls;
     delete m_view;
@@ -163,10 +163,10 @@ void EDA_DRAW_PANEL_GAL::onPaint( wxPaintEvent& WXUNUSED( aEvent ) )
 
     m_viewControls->UpdateScrollbars();
 
-    m_view->UpdateItems();
-
     try
     {
+        m_view->UpdateItems();
+
         m_gal->BeginDrawing();
         m_gal->SetClearColor( settings->GetBackgroundColor() );
         m_gal->SetCursorColor( settings->GetLayerColor( LAYER_CURSOR ) );
@@ -188,13 +188,17 @@ void EDA_DRAW_PANEL_GAL::onPaint( wxPaintEvent& WXUNUSED( aEvent ) )
     }
     catch( std::runtime_error& err )
     {
-        assert( GetBackend() != GAL_TYPE_CAIRO );
-
-        // Cairo is supposed to be the safe backend, there is not a single "throw" in its code
-        SwitchBackend( GAL_TYPE_CAIRO );
+        constexpr auto GAL_FALLBACK = GAL_TYPE_CAIRO;
 
         if( m_edaFrame )
-            m_edaFrame->UseGalCanvas( true );
+        {
+            bool use_gal = m_edaFrame->SwitchCanvas( GAL_FALLBACK );
+            m_edaFrame->UseGalCanvas( use_gal );
+        }
+        else
+        {
+            SwitchBackend( GAL_TYPE_CAIRO );
+        }
 
         DisplayError( m_parent, wxString( err.what() ) );
     }
@@ -363,6 +367,7 @@ bool EDA_DRAW_PANEL_GAL::SwitchBackend( GAL_TYPE aGalType )
     }
     catch( std::runtime_error& err )
     {
+        // Create a dummy GAL
         new_gal = new KIGFX::GAL( m_options );
         aGalType = GAL_TYPE_NONE;
         DisplayError( m_parent, wxString( err.what() ) );
diff --git a/include/draw_frame.h b/include/draw_frame.h
index 899c9784b..7b89637de 100644
--- a/include/draw_frame.h
+++ b/include/draw_frame.h
@@ -331,6 +331,12 @@ public:
     bool ShowPageLimits() const { return m_showPageLimits; }
     void SetShowPageLimits( bool aShow ) { m_showPageLimits = aShow; }
 
+    virtual bool SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE aCanvasType )
+    {
+        wxFAIL_MSG( "SwitchCanvas not implemented" );
+        return false;
+    }
+
     EDA_DRAW_PANEL* GetCanvas() { return m_canvas; }
 
     virtual wxString GetScreenDesc() const;
diff --git a/include/wxBasePcbFrame.h b/include/wxBasePcbFrame.h
index 6b0f23ee2..37a40b84a 100644
--- a/include/wxBasePcbFrame.h
+++ b/include/wxBasePcbFrame.h
@@ -40,7 +40,6 @@
 #include <eda_text.h>                // EDA_DRAW_MODE_T
 #include <richio.h>
 #include <class_pcb_screen.h>
-
 #include <pcb_display_options.h>
 #include <pcb_general_settings.h>
 
@@ -646,6 +645,8 @@ public:
     void OnTogglePolarCoords( wxCommandEvent& aEvent );
     void OnTogglePadDrawMode( wxCommandEvent& aEvent );
 
+    virtual void OnSwitchCanvas( wxCommandEvent& aEvent );
+
     // User interface update event handlers.
     void OnUpdateCoordType( wxUpdateUIEvent& aEvent );
     void OnUpdatePadDrawMode( wxUpdateUIEvent& aEvent );
@@ -686,7 +687,7 @@ public:
     /**
      * switches currently used canvas (default / Cairo / OpenGL).
      */
-    virtual void SwitchCanvas( wxCommandEvent& aEvent );
+    bool SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE aCanvasType ) override;

     /**
      * Update UI called when switches currently used canvas (default / Cairo / OpenGL).
diff --git a/pcbnew/basepcbframe.cpp b/pcbnew/basepcbframe.cpp
index 54ba21cb7..5745377a4 100644
--- a/pcbnew/basepcbframe.cpp
+++ b/pcbnew/basepcbframe.cpp
@@ -466,6 +466,25 @@ void PCB_BASE_FRAME::OnTogglePadDrawMode( wxCommandEvent& aEvent )
 }
 
 
+void PCB_BASE_FRAME::OnSwitchCanvas( wxCommandEvent& aEvent )
+{
+    switch( aEvent.GetId() )
+    {
+    case ID_MENU_CANVAS_LEGACY:
+        SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE );
+        break;
+
+    case ID_MENU_CANVAS_CAIRO:
+        SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE_CAIRO );
+        break;
+
+    case ID_MENU_CANVAS_OPENGL:
+        SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE_OPENGL );
+        break;
+    }
+}
+
+
 void PCB_BASE_FRAME::OnUpdateCoordType( wxUpdateUIEvent& aEvent )
 {
     auto displ_opts = (PCB_DISPLAY_OPTIONS*)GetDisplayOptions();
@@ -932,36 +951,6 @@ void PCB_BASE_FRAME::SetPrevGrid()
 }
 
 
-void PCB_BASE_FRAME::SwitchCanvas( wxCommandEvent& aEvent )
-{
-    bool use_gal = false;
-    EDA_DRAW_PANEL_GAL::GAL_TYPE canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE;
-
-    switch( aEvent.GetId() )
-    {
-    case ID_MENU_CANVAS_LEGACY:
-        break;
-
-    case ID_MENU_CANVAS_CAIRO:
-        use_gal = GetGalCanvas()->SwitchBackend( EDA_DRAW_PANEL_GAL::GAL_TYPE_CAIRO );
-
-        if( use_gal )
-            canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_CAIRO;
-        break;
-
-    case ID_MENU_CANVAS_OPENGL:
-        use_gal = GetGalCanvas()->SwitchBackend( EDA_DRAW_PANEL_GAL::GAL_TYPE_OPENGL );
-
-        if( use_gal )
-            canvasType = EDA_DRAW_PANEL_GAL::GAL_TYPE_OPENGL;
-        break;
-    }
-
-    UseGalCanvas( use_gal );
-    saveCanvasTypeSetting( canvasType );
-}
-
-
 void PCB_BASE_FRAME::UseGalCanvas( bool aEnable )
 {
     EDA_DRAW_FRAME::UseGalCanvas( aEnable );
@@ -1000,6 +989,17 @@ void PCB_BASE_FRAME::UseGalCanvas( bool aEnable )
 }
 
 
+bool PCB_BASE_FRAME::SwitchCanvas( EDA_DRAW_PANEL_GAL::GAL_TYPE aCanvasType )
+{
+    bool use_gal = GetGalCanvas()->SwitchBackend( aCanvasType );
+    use_gal &= aCanvasType != EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE;
+    UseGalCanvas( use_gal );
+    saveCanvasTypeSetting( use_gal ? aCanvasType : EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE );
+
+    return use_gal;
+}
+
+
 void PCB_BASE_FRAME::OnUpdateSwitchCanvas( wxUpdateUIEvent& aEvent )
 {
     wxMenuBar* menuBar = GetMenuBar();
diff --git a/pcbnew/moduleframe.cpp b/pcbnew/moduleframe.cpp
index 8e3886112..0ed3507a2 100644
--- a/pcbnew/moduleframe.cpp
+++ b/pcbnew/moduleframe.cpp
@@ -181,9 +181,9 @@ BEGIN_EVENT_TABLE( FOOTPRINT_EDIT_FRAME, PCB_BASE_FRAME )
     EVT_MENU( ID_MENU_PCB_SHOW_3D_FRAME, FOOTPRINT_EDIT_FRAME::Show3D_Frame )
 
     // Switching canvases
-    EVT_MENU( ID_MENU_CANVAS_LEGACY, PCB_BASE_FRAME::SwitchCanvas )
-    EVT_MENU( ID_MENU_CANVAS_CAIRO, PCB_BASE_FRAME::SwitchCanvas )
-    EVT_MENU( ID_MENU_CANVAS_OPENGL, PCB_BASE_FRAME::SwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_LEGACY, PCB_BASE_FRAME::OnSwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_CAIRO, PCB_BASE_FRAME::OnSwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_OPENGL, PCB_BASE_FRAME::OnSwitchCanvas )
 
     // UI update events.
     EVT_UPDATE_UI( ID_MODEDIT_DELETE_PART, FOOTPRINT_EDIT_FRAME::OnUpdateLibSelected )
diff --git a/pcbnew/pcb_draw_panel_gal.cpp b/pcbnew/pcb_draw_panel_gal.cpp
index 61abe3274..5a2dd4fed 100644
--- a/pcbnew/pcb_draw_panel_gal.cpp
+++ b/pcbnew/pcb_draw_panel_gal.cpp
@@ -37,6 +37,7 @@
 #include <class_track.h>
 #include <class_marker_pcb.h>
 #include <wxBasePcbFrame.h>
+#include <confirm.h>
 
 #include <gal/graphics_abstraction_layer.h>
 
@@ -356,6 +357,19 @@ void PCB_DRAW_PANEL_GAL::OnShow()
 {
     PCB_BASE_FRAME* frame = dynamic_cast<PCB_BASE_FRAME*>( GetParent() );
 
+    try
+    {
+        // Check if the current rendering backend can be properly initialized
+        m_view->UpdateItems();
+    }
+    catch( const std::runtime_error& e )
+    {
+        // Fallback to software renderer
+        DisplayError( frame, e.what() );
+        bool use_gal = SwitchBackend( GAL_TYPE_CAIRO );
+        frame->UseGalCanvas( use_gal );
+    }
+
     if( frame )
     {
         SetTopLayer( frame->GetActiveLayer() );
diff --git a/pcbnew/pcbframe.cpp b/pcbnew/pcbframe.cpp
index 6c7b7e022..2675ff9f2 100644
--- a/pcbnew/pcbframe.cpp
+++ b/pcbnew/pcbframe.cpp
@@ -187,9 +187,9 @@ BEGIN_EVENT_TABLE( PCB_EDIT_FRAME, PCB_BASE_FRAME )
     EVT_MENU( ID_MENU_PCB_SHOW_3D_FRAME, PCB_EDIT_FRAME::Show3D_Frame )
 
     // Switching canvases
-    EVT_MENU( ID_MENU_CANVAS_LEGACY, PCB_EDIT_FRAME::SwitchCanvas )
-    EVT_MENU( ID_MENU_CANVAS_CAIRO, PCB_EDIT_FRAME::SwitchCanvas )
-    EVT_MENU( ID_MENU_CANVAS_OPENGL, PCB_EDIT_FRAME::SwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_LEGACY, PCB_EDIT_FRAME::OnSwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_CAIRO, PCB_EDIT_FRAME::OnSwitchCanvas )
+    EVT_MENU( ID_MENU_CANVAS_OPENGL, PCB_EDIT_FRAME::OnSwitchCanvas )
 
     // Menu Get Design Rules Editor
     EVT_MENU( ID_MENU_PCB_SHOW_DESIGN_RULES_DIALOG, PCB_EDIT_FRAME::ShowDesignRulesEditor )
@@ -1176,10 +1176,10 @@ void PCB_EDIT_FRAME::OnLayerColorChange( wxCommandEvent& aEvent )
 }
 
 
-void PCB_EDIT_FRAME::SwitchCanvas( wxCommandEvent& aEvent )
+void PCB_EDIT_FRAME::OnSwitchCanvas( wxCommandEvent& aEvent )
 {
     // switches currently used canvas (default / Cairo / OpenGL).
-    PCB_BASE_FRAME::SwitchCanvas( aEvent );
+    PCB_BASE_FRAME::OnSwitchCanvas( aEvent );
 
     // The base class method reinit the layers manager.
     // We must upate the layer widget to match board visibility states,
diff --git a/pcbnew/wxPcbStruct.h b/pcbnew/wxPcbStruct.h
index b0b36bb74..41a2ee023 100644
--- a/pcbnew/wxPcbStruct.h
+++ b/pcbnew/wxPcbStruct.h
@@ -116,7 +116,7 @@ protected:
     /**
      * switches currently used canvas (default / Cairo / OpenGL).
      */
-    virtual void SwitchCanvas( wxCommandEvent& aEvent ) override;
+    virtual void OnSwitchCanvas( wxCommandEvent& aEvent ) override;
 
 #if defined(KICAD_SCRIPTING) && defined(KICAD_SCRIPTING_ACTION_MENU)
     /**
-- 
2.13.3

Attachment: signature.asc
Description: OpenPGP digital signature


Follow ups

References