← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Add footprint select dropdown to component chooser, serious refactoring

 

On Fri, Mar 24, 2017 at 08:43:00AM -0400, Wayne Stambaugh wrote:
> Chris,
> 
> I did some testing on this and it looks good.  I have a few comments and
> questions.
> 
> There is a noticeable response time lag when the footprints are being
> loaded on windows.  This may be specific to windows.  I will test it on
> linux when I get a chance.

It should only happen on first load. There's a part of the loading
process that I cannot make properly asynchronous. I have some ideas for
fixing that, which I may look into implementing, but until then the
first-load lag is unavoidable. The locale switching required by the
parser is the issue here. It's much shorter than the one when cvpcb
opens, though ;)

> 
> Are you planning on filtering the footprint list?  Right now, the
> footprint drop down control only shows all of the footprints from the
> first footprint library in the table.

That's weird, it doesn't happen for me. Could you please give me more
specifics to reproduce it? I'd prefer if we could merge first - this
patch being so large, it's become quite a hassle to keep my branches in
sync while this one is up in the air. Should be a quick fix.

> 
> When cancelling the footprint viewer after double clicking on the symbol
> panel, the component select dialog is also dismissed.  I was expecting
> to fall back to the component select dialog.  I'm not sure if this is
> what you intended but I may not be the only person surprised by this.

Canceling how? If you're pressing Esc with the dropdown expanded, this
is one of the known UI quirks that are going to be fixed in the
following UI adjustments once the main patch is merged.

> 
> It would be nice if one of our osx devs could test this before we push
> this to the master branch.

FWIW I have tested on all three platforms.

> 
> Cheers,
> 
> Wayne
> 
> On 3/23/2017 3:22 PM, Chris Pavlina wrote:
> > Hi again,
> > 
> > Updated patch unless this doesn't apply cleanly anymore after b47a6e4
> > 
> > 
> > On Wed, Mar 22, 2017 at 09:09:01PM -0400, Chris Pavlina wrote:
> >> Hi,
> >>
> >> Footprint selection in the component chooser is now working - here is a
> >> patch. I'd like to merge this, but it required serious refactoring to
> >> make everything work cleanly, so I'm posting to the list. Wayne, please
> >> have a look when you get a chance.
> >>
> >> There are a couple known issues, but IMO they aren't merge-stoppers. The
> >> sooner I get this merged the sooner I can get actual feedback, and the
> >> smoother integration of any changes that are made can be.
> >>
> >> Here's a summary (yes, the *summary* is big):
> >>
> >> - DIALOG_CHOOSE_COMPONENT changes:
> >>    + Add FOOTPRINT_SELECT_WIDGET.
> >>    + Add support for DIALOG_CHOOSE_COMPONENT to pass arbitrary field value
> >>      overrides to the caller. This is of course to allow setting the footprint;
> >>      in the future it could be used to allow more field edits.
> >>    + Add an option to hide everything that can edit fields, for use when
> >>      that doesn't make sense (libedit, etc).
> >>
> >> - Add FOOTPRINT_SELECT_WIDGET
> >>   This is an adapter widget that combines into one FOOTPRINT_CHOICE view:
> >>     + Footprint listings from FOOTPRINT_LIST
> >>     + Filtering from FOOTPRINT_FILTER
> >>     + Loading progress display from FOOTPRINT_ASYNC_LOADER and wxGauge
> >>
> >>   It presents as a status progress bar that transforms in-place to a selection
> >>   dropdown when the footprints finish loading. The GUI remains fully interactive
> >>   as the footprints load; the user can even exit and reopen the dialog and it
> >>   will continue to load in the background.
> >>
> >> - Add FOOTPRINT_CHOICE widget
> >>   This is a customized wxComboCtrl with some extra features:
> >>     + Greying out of library name for readability
> >>     + List separators
> >>
> >> - Add FOOTPRINT_FILTER class
> >>   Provides a reusable filtered view of a FOOTPRINT_LIST, fully iterable
> >>
> >>    + Make cvpcb use FOOTPRINT_FILTER instead of providing its own filter
> >>      code.
> >>
> >> - Seriously rework FOOTPRINT_INFO
> >>    + Add partially asynchronous loading via the new FP_LIB_TABLE::PrefetchLib.
> >>      A FOOTPRINT_ASYNC_LOADER class is added that can spawn loader threads and
> >>      provide progress updates to the GUI while they work.
> >>    + Completely rewrite footprint loader worker threads. They are now a
> >>      queue-driven pool of workers rather than each loading a fixed number
> >>      of libs (more efficient, a bit faster) and the main thread does no
> >>      work, so it can return.
> >>    + Make FOOTPRINT_INFO available to the world, by making them virtual base
> >>      classes, putting the real implementation in
> >>      pcbnew/footprint_info_impl.h/cpp, and adding a factory function to
> >>      create an instance from anywhere via Kiface.
> >>
> >> - Add FP_LIB_TABLE::PrefetchLib
> >>   This pulls everything that is async-safe (download from github, but not
> >>   parsing due to a threadsafety issue) into a separate loader so the user
> >>   can continue interacting as footprints download.
> >>
> >>   Parsing itself remains synchronous, but the time it takes is tiny
> >>   compared to downloading.
> >>
> >> - Allow access to the global fp_lib_table from anywhere via kiface
> >>   (IfaceOrAddress()). Most methods to manipulate the table are still not
> >>   compiled in everywhere (they have seriously large dependencies), but the
> >>   table can be fetched as an opaque object.
> >>
> >> - Add a SYNC_QUEUE template class providing a std::queue wrapper with locking,
> >>   for ease of passing things to and from worker threads.
> >>
> >> - Minor changes:
> >>    + Add EDA_PATTERN_MATCH::GetPattern()
> >>    + Create a type to represent component history items instead of just storing
> >>      a list of strings and a unit. We'll need to track footprints too.
> >>    + Make DIALOG_CHOOSE_COMPONENT quasimodal so it can summon the footprint
> >>      picker.
> >>    + Add kiface_ids.h for storing arbitrary IDs used in kiface. This is used
> >>      for KifaceOrAddress(), as I'm the first person to actually use that method
> >>    + Remove KICAD_FOOTPRINT_SELECTOR build option, no reason for it to be
> >>      optional now.
> >>
> >> ===========
> >> Known issues, planned improvements:
> >>
> >> - History items don't remember their selected footprints.
> >>   There are some implementation issues and UI/UX issues I still have to work
> >>   out.
> >>
> >> - eeschema and cvpcb/pcbnew have separate footprint caches, so the first time
> >>   you open one of the latter, it'll fetch the footprints even if eeschema
> >>   already did.
> >>
> >> - FOOTPRINT_ASYNC_LOADER can also be used in cvpcb so cvpcb doesn't freeze
> >>   as the footprints load! :)
> >>
> >> - Footprint picker isn't hidden in standalone mode, but it's useless.
> >>
> >> -- 
> >> Chris
> > 
> > 
> > 
> > _______________________________________________
> > 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


Follow ups

References