← Back to team overview

kicad-developers team mailing list archive

Re: Partial fix for Bug #1737132

 

Hi Jean-Pierre,

On 12/12/2017 07:00 PM, jp charras wrote:
> Hi Orson,
> 
> Could you have a look into this patch.
> 
> It fixes the Bug #1737132 (component tree widget cleared in lib editor after any change in symbol),
> but there is still a minor issue.
> 
> The reason the component tree widget was cleared on Windows is the fact
> m_adapter->UpdateSearchString( m_query_ctrl->GetLineText( 0 ) )
> is called on any change of the symbol.
> But if m_query_ctrl is empty, m_query_ctrl->GetLineText( 0 ) returns the "hint" (currently the word
> "Search") not the actual value, so there is no "candidate" symbol in list.
> Using m_query_ctrl->GetValue() returns the actual value and fixes this issue.

Thank you very much, I could not even reproduce the problem and I did
not find a mistake just by looking at the code. The strange thing is
this line of code was in COMPONENT_TREE class since its origin and it
used to work. Anyway, I am grateful for fixing the problem.
I tested the change with Linux and it seems to work fine, so I pushed
the patch.

> However I still have an other (minor but annoying) issue:
> When the current symbol is edited, the selection in the tree changes after each edition:
> sometimes the previous tree item is selected, sometimes the next item, and sometimes it does not happen.
> It does not happen for each symbol, so perhaps this issue depends on the phase of the moon.

Very likely it is the phase of the moon playing a role here. I think is
the time to tell the wxDataViewCtrl story. It is a great widget, given
you do not do massive model updates, which really hits it performance.

The initial implementation did partial model updates using notifications
(wxDataViewModel::Item{Added,Deleted,Changed}), but it suffered from
terrible performance. It means it is fine for quick updates, but it is
unusable for component filtering. The advantage is you keep the state of
the widget, so any node that was expanded stays expanded, it preserves
selections, etc.

Then I understood why Chris went for clear-rebuild approach - it is
significantly faster. Due to performance complaints, I changed the code
to use this method as well. The problems start when you want to keep the
state of the tree, so before the widget is cleared, one has to store its
state (which nodes are expanded or selected) and restore the state
afterwards. Ridiculous, is not it? And such rebuild has to occur on
every modification to update the nodes attributes (color selected, bold
for modified). Obviously, you do not get the same behavior on all
platforms, so e.g. on one system it scrolls to the selected node on
another not - this is why you may get strange scrolling jumps on refresh.

This is how you waste a lot of time on simple things. I will have
another trial on fixing this a bit later. One thing to add is you always
have the main component selected in the tree and not its aliases.

Regards,
Orson

> Thanks
> 
> 
> 
> _______________________________________________
> 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


References