kicad-developers team mailing list archive

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

Hi Jon,

I am not entirely sure about the problem root cause, but there are some
hints:
- VIEW_GROUP uses a list of items obtained via its virtual
updateDrawList() method
- SELECTION class, which represents the selected items (visually as
well) is a VIEW_GROUP, but its overridden updateDrawList() is never called
- most of items in VIEW::updateLayers() method are anyway reinserted,
because viewData->getLayers() returns a sorted vector of layer numbers,
whereas aItem->ViewGetLayers() return them in any order
- SELECTION is a VIEW_GROUP, so it is placed only on the OVERLAY layer,
so there is no chance the order of layers return by
VIEW_ITEM::ViewGetLayers() and VIEW_ITEM_DATA::getLayers() could be
different

More hints to come later, I need to switch my context now.

Regards,
Orson

On 09/22/2017 04:04 AM, Jon Evans wrote:
> Hi Orson,
> 
> I have dug into Seth's report of slowness on this large Gerber file, and
> reproduced it.
> It looks like VIEW is struggling under the load of all the items, so this
> is a generic GAL problem, but Gerber files in general have many more draw
> items than PCB files, so we only notice it now.
> 
> I have played around a bit with some profiling and optimizations, but have
> gotten stuck on one.
> It seems like RTREE::Remove is very costly, and so one optimization I tried
> is to skip the remove/add step in VIEW::updateLayers() if the layers are
> the same.
> This has some subtle problems that I don't understand, so maybe you can
> advise?
> 
> 1) If I compile the code with the attached patch, GerbView is much faster
> (1 minute load time -> 5 seconds on Seth's file), but the message box info
> doesn't show up for selected items.  In PcbNew, selected items are
> invisible.
> 2) If I comment out line 1262 ("if( !same )") to unconditionally run the
> insert, the message box and invisible items problems are solved, but the
> programs segfault after selecting a few items.
> 
> I don't have a good understanding of the inner workings of the R-Tree or
> whether Remove could be optimized so that we wouldn't have to skip it, but
> I also don't understand why if updateLayers() is called and the layers of
> an item haven't changed, it seems like we still have to do the remove and
> insert loops to get proper behavior.
> 
> 
> Thanks,
> -Jon
> 
> On Thu, Sep 21, 2017 at 3:13 PM, Seth Hillbrand <seth.hillbrand@xxxxxxxxx>
> wrote:
> 
>> Hi Jon-
>>
>> Sorry, my wording was bad.  This is not a merging show-stopper.  Legacy
>> use is unaffected.
>>
>> Best-
>> Seth
>>
>> On Thu, Sep 21, 2017 at 11:55 AM, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>>
>>> Hi Seth,
>>>
>>> Thanks for reporting and providing your example file.  I will investigate
>>> this issue, but is this actually a show stopper for merging?  Can't you
>>> just keep using legacy canvas?
>>>
>>> Thanks,
>>> Jon
>>>
>>> On Thu, Sep 21, 2017 at 2:47 PM, Seth Hillbrand <seth.hillbrand@xxxxxxxxx
>>>> wrote:
>>>
>>>> -- Apologies if this message is a repeat.  Looks like first attempt
>>>> didn't go through --
>>>>
>>>> Hi Jon-
>>>>
>>>> This is really neat work!  I especially like the selection highlighting.
>>>>
>>>> However, I do have a show-stopping (for me) issue.  Loading a few of my
>>>> projects takes a really long time.  Using the legacy canvas, I can load the
>>>> attached layer in about 2 seconds on my relatively modest machine.  When I
>>>> use the OpenGL canvas, it takes almost a full minute.  Same again when I
>>>> zoom and scroll.  There is no lag on the legacy canvas, but at least 2-3
>>>> seconds to zoom or scroll under OpenGL.
>>>>
>>>> I am running Linux, latest NVidia drivers on a GeForce 750 GTX.
>>>>
>>>> Best-
>>>> Seth
>>>>
>>>> On Thu, Sep 21, 2017 at 3:05 AM, Maciej Sumiński  wrote:
>>>>
>>>>> Hi Jon,
>>>>>
>>>>> Thanks you, this is really cool! Now it is even more tempting to merge
>>>>> the gerbview_gal branch. I am going to wait one more day for vetos and
>>>>> tomorrow I will push it to the master branch.
>>>>>
>>>>> Regards,
>>>>> Orson
>>>>>
>>>>> On 09/20/2017 09:57 PM, Jon Evans wrote:
>>>>>> Hi Orson,
>>>>>>
>>>>>> Give this a shot in your branch.  It should work in pcbnew also now.
>>>>>>
>>>>>> -Jon
>>>>>>
>>>>>> On Wed, Sep 20, 2017 at 9:28 AM, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>>>>>>
>>>>>>> Hi Orson,
>>>>>>>
>>>>>>> Thank you for staging this for merge on your branch.  I checked and
>>>>> you do
>>>>>>> have all the patches.
>>>>>>>
>>>>>>> 1) Yes I planned on refactoring the selection tool once things
>>>>> stabilized
>>>>>>> with the highlighting etc.
>>>>>>> 2) Do you mean when you are highlighting Gerber X2 attributes, or
>>>>> when you
>>>>>>> are deselecting things, or something else?
>>>>>>> 3) That's a good idea on VIEW_GROUP, I will give it a try and send a
>>>>> patch.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jon
>>>>>>>
>>>>>>> On Wed, Sep 20, 2017 at 5:46 AM, Maciej Sumiński <
>>>>> maciej.suminski@xxxxxxx>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Jon,
>>>>>>>>
>>>>>>>> GALifying GerbView is a huge task, so thank you very much for your
>>>>> work!
>>>>>>>> I have just tested your changes and in my opinion it is in a state
>>>>> that
>>>>>>>> deserves merging and further tests. The new way of item
>>>>> highlighting is
>>>>>>>> awesome, we need to port it to pcbnew as well.
>>>>>>>>
>>>>>>>> For now I keep your patches in a separate branch, with some minor
>>>>>>>> modifications on top of it [1]. Please verify it contains all the
>>>>> needed
>>>>>>>> patches. If nobody objects, I would like to merge it this week.
>>>>>>>>
>>>>>>>> Just a few minor remarks:
>>>>>>>> - It seems there is some code that could be refactored to share it
>>>>> with
>>>>>>>> pcbnew (e.g. selection tool).
>>>>>>>> - 'Clear highlight' operation takes long time to finish (seems more
>>>>> than
>>>>>>>> with the legacy canvas), but I cannot really see what is happening
>>>>>>>> there. If it cannot be easily fixed, perhaps it could set the mouse
>>>>>>>> cursor to busy.
>>>>>>>> - For the new highlighting method: perhaps a more universal way is
>>>>> to
>>>>>>>> create a temporary VIEW_GROUP object containing the selection
>>>>> candidate.
>>>>>>>> This way it can be temporarily displayed on the overlay layer,
>>>>> without
>>>>>>>> modifying the original ViewGetLayer() methods.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Orson
>>>>>>>>
>>>>>>>> 1. https://code.launchpad.net/~orsonmmz/kicad/+git/kicad/+ref/
>>>>>>>> gerbview_gal
>>>>>>>>
>>>>>>>> On 09/18/2017 12:47 AM, Jon Evans wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> The day has finally come!  I have distilled my GerbView GAL branch
>>>>> into
>>>>>>>> a
>>>>>>>>> patchset attached to this email.  Hopefully with this merged into
>>>>>>>> master we
>>>>>>>>> can identify any remaining bugs and clean it up for 5.0.
>>>>>>>>>
>>>>>>>>> Note that this set is split into 5 patches to make review easier,
>>>>> but
>>>>>>>> they
>>>>>>>>> are not intended to compile and work independently.
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> 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
>>>>
>>>>
>>>
>>
>

Attachment: signature.asc
Description: OpenPGP digital signature