Alright, thank you for quick verification. I have just committed the fix to the master branch.
Cheers, Orson On 02/26/2018 05:12 PM, Jon Evans wrote: > Sounds good to me. 1% hit should not cancel out the gains from not using > the RTree in large worlds (I saw layer-switch time drop from ~80ms to ~30ms > on my machine) > We can consider revisiting this later to eliminate the need for > dynamic_cast by ensuring that anything that VIEW calls on an item it owns > is a member of that object. > > -Jon > > On Mon, Feb 26, 2018 at 11:03 AM, Maciej Sumiński <maciej.sumin...@cern.ch> > wrote: > >> 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 >>> >> >> >> _______________________________________________ >> 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
