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

[Maciej Sumiński]

I pushed both patches to the master branch. Jon, thank you for your
contribution to KiCad.

Cheers,
Orson

On 01/17/2018 05:54 PM, Maciej Sumiński wrote:
> I have just tested the patch on a Windows virtual machine where KiCad
> normally crashed and it seems that the fallback mechanism works as it
> should.
> 
> Chris, if you have time, please check it on you hardware too, if not - I
> will just push the patch to the master branch. I vaguely recall that
> Linux without OpenGL support might crash even before KiCad has a chance
> to throw an exception (X.Org error). Perhaps this case could be resolved
> with Jon's patch.
> 
> Cheers,
> Orson
> 
> On 01/17/2018 04:30 PM, Maciej Sumiński wrote:
>> 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 +, 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 +, 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  wrote:
>>
>>> Updated patch attached (missed initializers in the original)
>>>
>>> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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@lists.launchpad.net
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
> 
> 
> 
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 




signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Maciej Sumiński]

I have just tested the patch on a Windows virtual machine where KiCad
normally crashed and it seems that the fallback mechanism works as it
should.

Chris, if you have time, please check it on you hardware too, if not - I
will just push the patch to the master branch. I vaguely recall that
Linux without OpenGL support might crash even before KiCad has a chance
to throw an exception (X.Org error). Perhaps this case could be resolved
with Jon's patch.

Cheers,
Orson

On 01/17/2018 04:30 PM, Maciej Sumiński wrote:
> 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 +, 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 +, 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  wrote:
>
>> Updated patch attached (missed initializers in the original)
>>
>> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
> 
> 
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 




signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Maciej Sumiński]

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 +, 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 +, 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  wrote:

> Updated patch attached (missed initializers in the original)
>
> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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@lists.launchpad.net
> 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 
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( 

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

[Maciej Suminski]

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 +, 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 +, 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  wrote:
>>>
 Updated patch attached (missed initializers in the original)

 On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Chris Pavlina]

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 +, 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  wrote:
> 
> > Updated patch attached (missed initializers in the original)
> >
> > On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Maciej Sumiński]

I have already read the patch, and IMHO it has a chance to fix the
problem. Right now I am building KiCad on an ultraslow virtual machine
that does not support OpenGL 2.1, so stay tuned!

Cheers,
Orson

On 01/16/2018 05:26 PM, 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  wrote:
> 
>> Updated patch attached (missed initializers in the original)
>>
>> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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
>>>
>>
>>
> 




signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Jon Evans]

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  wrote:

> Updated patch attached (missed initializers in the original)
>
> On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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
>>
>
>


0001-Defer-canvas-type-setting-save-until-destruction-of.patch
Description: Binary data
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

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

[Jon Evans]

Updated patch attached (missed initializers in the original)

On Wed, Jan 10, 2018 at 11:40 PM, Jon Evans  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
>
From 1eb7a5ca257a9e8d0a5dfad08ed74f3a238df2b0 Mon Sep 17 00:00:00 2001
From: Jon Evans 
Date: Thu, 11 Jan 2018 23:02:18 -0500
Subject: [PATCH] Defer canvas type setting save until destruction of 
 EDA_DRAW_FRAME

Fixes: lp:1741787
* https://bugs.launchpad.net/kicad/+bug/1741787
---
 common/draw_frame.cpp| 5 +
 gerbview/events_called_functions.cpp | 4 +++-
 include/draw_frame.h | 6 ++
 pcbnew/basepcbframe.cpp  | 4 +++-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/common/draw_frame.cpp b/common/draw_frame.cpp
index bd9342155..49b2df5c6 100644
--- a/common/draw_frame.cpp
+++ b/common/draw_frame.cpp
@@ -142,6 +142,8 @@ EDA_DRAW_FRAME::EDA_DRAW_FRAME( KIWAY* aKiway, wxWindow* aParent,
 m_hotkeysDescrList= NULL;
 
 m_canvas  = NULL;
+m_canvasType  = EDA_DRAW_PANEL_GAL::GAL_TYPE_NONE;
+m_canvasTypeDirty = false;
 m_galCanvas   = NULL;
 m_galCanvasActive = false;
 m_actions = NULL;
@@ -216,6 +218,9 @@ EDA_DRAW_FRAME::EDA_DRAW_FRAME( KIWAY* aKiway, wxWindow* aParent,
 
 EDA_DRAW_FRAME::~EDA_DRAW_FRAME()
 {
+if( m_canvasTypeDirty )
+saveCanvasTypeSetting( m_canvasType );
+
 delete m_actions;
 delete m_toolManager;
 delete m_toolDispatcher;
diff --git a/gerbview/events_called_functions.cpp b/gerbview/events_called_functions.cpp
index 05d611aac..222ff9953 100644
--- a/gerbview/events_called_functions.cpp
+++ b/gerbview/events_called_functions.cpp
@@ -563,10 +563,12 @@ void GERBVIEW_FRAME::SwitchCanvas( wxCommandEvent& aEvent )
 break;
 }
 
-saveCanvasTypeSetting( canvasType );
 UseGalCanvas( use_gal );
 wxUpdateUIEvent e;
 OnUpdateSwitchCanvas( e );
+
+m_canvasType = canvasType;
+m_canvasTypeDirty = true;
 }
 
 
diff --git a/include/draw_frame.h b/include/draw_frame.h
index 899c9784b..6d925eb52 100644
--- a/include/draw_frame.h
+++ b/include/draw_frame.h
@@ -141,6 +141,12 @@ protected:
 /// One-shot to avoid a recursive mouse event during hotkey movement
 boolm_movingCursorWithKeyboard;
 
+/// Flag indicating that drawing canvas type needs to be saved to config
+boolm_canvasTypeDirty;
+
+/// The current canvas type
+EDA_DRAW_PANEL_GAL::GAL_TYPEm_canvasType;
+
 void SetScreen( BASE_SCREEN* aScreen )  { m_currentScreen = aScreen; }
 
 /**
diff --git a/pcbnew/basepcbframe.cpp b/pcbnew/basepcbframe.cpp
index 54ba21cb7..62245fc45 100644
--- a/pcbnew/basepcbframe.cpp
+++ b/pcbnew/basepcbframe.cpp
@@ -958,7 +958,9 @@ void PCB_BASE_FRAME::SwitchCanvas( wxCommandEvent& aEvent )
 }
 
 UseGalCanvas( use_gal );
-saveCanvasTypeSetting( canvasType );
+
+m_canvasType = canvasType;
+m_canvasTypeDirty = true;
 }
 
 
-- 
2.14.1

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp