kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

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.suminski@xxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxx
>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>>>>
>>>
>>> _______________________________________________
>>> 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
>>>
>>
>>
>> _______________________________________________
>> 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
>>
>>
>

Attachment: signature.asc
Description: OpenPGP digital signature