Apparently clang analyzer has been correct this time. It turns out that PCB_PAINTER::Draw() and PCB_RENDER_SETTINGS::GetColor() assume that all objects are EDA_ITEMs, which is not true for VIEW_GROUP class. When a layer is changed, VIEW::UpdateAllLayersColor() iterates through all VIEW_ITEMs and eventually calls PCB_RENDER_SETTINGS::GetColor() which incorrectly casts VIEW_GROUP objects to EDA_ITEM.
The attached patch uses dynamic_cast to verify the downcast results. I measured performance drop at order of 1% in a debug build, so it does not seem like a huge loss. I think we have not seen any problems as RTree::Query() had never returned VIEW_GROUP objects as they had too large bounding box. I would rather keep Jon's patch, as it makes sense to simply iterate through a simple array of items during full world updates, rather than using RTree algorithms to extract the same list. Cheers, Orson On 02/26/2018 04:12 PM, Wayne Stambaugh wrote: > Orson, > > That's what I was doing so it sounds like it might be something with > your setup. > > Wayne > > On 2/26/2018 9:30 AM, Maciej Sumiński wrote: >> Wayne, >> >> I meant selecting any layer in the right-hand layer widget. Perhaps it >> is a false positive in clang analyzer, I will have a look at it. Pcbnew >> does not crash here when built with gcc (and no analyzer enabled). >> >> Cheers, >> Orson >> >> On 02/26/2018 03:02 PM, Wayne Stambaugh wrote: >>> Orson, >>> >>> What do you mean by "any layer switching"? I am not have any issues on >>> windows when selecting a different layer in the layer manager using >>> either canvas when launched from kicad or stand alone mode. >>> >>> Cheers, >>> >>> Wayne >>> >>> On 2/26/2018 5:01 AM, Maciej Sumiński wrote: >>>> Jon, >>>> >>>> With this patch applied, any layer switching in pcbnew leads to a crash, >>>> even with no layout loaded. Can anyone else confirm this? See the the >>>> attached address sanitizer report for details. >>>> >>>> Regards, >>>> Orson >>>> >>>> On 02/25/2018 10:01 PM, Jon Evans wrote: >>>>> This patch uses simple iteration instead of the RTREE search to update the >>>>> layer order and color in VIEW. On my Linux system, this results in a ~50% >>>>> speedup in release build (less in debug build) and is most noticeable when >>>>> switching layers in GerbView with large Gerber files loaded (i.e. high >>>>> number of items in the view). >>>>> >>>>> -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 >>> >> >> >> >> >> _______________________________________________ >> 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 >
From 5a970815c2463410ebcb1bd46b391252157c1e5f Mon Sep 17 00:00:00 2001 From: Maciej Suminski <maciej.sumin...@cern.ch> Date: Mon, 26 Feb 2018 16:51:18 +0100 Subject: [PATCH] PCB_PAINTER: use dynamic_cast to determine whether an object is of EDA_ITEM type --- pcbnew/pcb_painter.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp index 907ecbe24..52f9c5428 100644 --- a/pcbnew/pcb_painter.cpp +++ b/pcbnew/pcb_painter.cpp @@ -212,7 +212,7 @@ void PCB_RENDER_SETTINGS::LoadDisplayOptions( const PCB_DISPLAY_OPTIONS* aOption const COLOR4D& PCB_RENDER_SETTINGS::GetColor( const VIEW_ITEM* aItem, int aLayer ) const { int netCode = -1; - const EDA_ITEM* item = static_cast<const EDA_ITEM*>( aItem ); + const EDA_ITEM* item = dynamic_cast<const EDA_ITEM*>( aItem ); if( item ) { @@ -277,7 +277,10 @@ int PCB_PAINTER::getLineThickness( int aActualThickness ) const bool PCB_PAINTER::Draw( const VIEW_ITEM* aItem, int aLayer ) { - const EDA_ITEM* item = static_cast<const EDA_ITEM*>( aItem ); + const EDA_ITEM* item = dynamic_cast<const EDA_ITEM*>( aItem ); + + if( !item ) + return false; // the "cast" applied in here clarifies which overloaded draw() is called switch( item->Type() ) -- 2.13.3
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