← Back to team overview

kicad-developers team mailing list archive

Re: PATCH: Component chooser now with thumbnail image of the component. And Unit select.

 

On 18 February 2014 12:10, jp charras <jp.charras@xxxxxxxxxx> wrote:
> Le 18/02/2014 20:28, Wayne Stambaugh a écrit :
>> On 2/18/2014 12:22 PM, Henner Zeller wrote:
>>> On 18 February 2014 08:31, Wayne Stambaugh <stambaughw@xxxxxxxxxxx> wrote:
>>>> On 2/18/2014 2:40 AM, Henner Zeller wrote:
>>>>> Hi,
>>>>>
>>>>> This is an update patch to the new component chooser containing the
>>>>> planned followup-change to display a mini-preview of the component.
>>>>> Clicking on that box opens the 'big' component browser (Also fixes
>>>>> [Bug 1280567]). (Since we already can preselect a lot, it would be
>>>>> good if that browser would take these defaults; planned for another
>>>>> patch).
>>>>>
>>>>> Also, the component browser now allows to choose not only the
>>>>> component itself, but the units (or 'Gates' or 'Parts' however you'd
>>>>> call them). The four NANDs in a 7400 for instance, are provided in a
>>>>> sub-tree to select (the preview also shows the right unit).
>>>>>
>>>>> The best part of that: the chosen unit is also kept in the history. So
>>>>> if you previously selected 'Unit B' of the 7400, you now can press 'a'
>>>>> (add component - opens dialog), cursor-down (go from 'Unit B' to 'Unit
>>>>> C') and ENTER: now you can place 'Unit C'. I find that this very much
>>>>> speeds up the way I choose components.
>>>>>
>>>>> Also now: internally it works with the Aliases instead of the
>>>>> Components - the previous implementation just gave out component
>>>>> names, but it should've given out Alias names (so even if you chose
>>>>> 74HC00, you got 74LS00. This is now fixed).
>>>>>
>>>>> The preview is based on the suggestion by jp; and like he predicted
>>>>> there were several places that I needed to change to properly deal
>>>>> with NULL-aPanels. Included in this patch..
>>>>>
>>>>> Commit message:
>>>>>  * Allow to select units in components that have more than one right
>>>>> in the component chooser dialog.
>>>>>  * Keep chosen unit in history.
>>>>>  * Show preview of current component-unit as thumbnail image next to
>>>>> the description box.
>>>>>  * Fixes 1280567
>>>>>
>>>>> View here:
>>>>> https://github.com/hzeller/kicad/compare/master...component-selection#files_bucket
>>>>>
>>>>> Download here:
>>>>> https://github.com/hzeller/kicad/compare/master...component-selection.diff
>>>>>
>>>>> -h
>>>>>
>>>>
>>>> Henner,
>>>>
>>>> Nice work.  This is infinitely better than the old component selection
>>>> dialog.  I will commit this patch as is if no one has any objections but
>>>> I have a few comments and one minor bug.  On windows builds with
>>>> wxWidget 3.0.0, the component view panel does not update when a new
>>>> component is selected.  When the dialog is closed and reopen it, then
>>>> the selected component is shown correctly.
>>>
>>> This might be due to the way I am force-updating the panel. I'll have
>>> to fully grok how wxWidgets does redraws I suppose :)
>>> It was according to jc's suggestion, but I suppose since it draws into
>>> the panel without explicit redraw command, it
>>> might be lost depending on how the underlying windowing system is
>>> dealing with it.
>
> You should not have any issue with a wxPanel.

You mean keep the wxPanel as now, but do the painting like now
manually in the dialog, but
with better hooking up events and stuff as you describe below.

>
> Just:
> do not call renderPreview from updateSelection().
> Just call Refresh() (you are using a wxPaintDC, which is usually called
> only inside a Paint event).

ok. Thanks, this is exactly the kind of wxWidgets domain knowledge
that I still have to get accustomed to.
So 'Refresh()' will issue a re-paint event internally ?

>
> Also:
> - for the wxPanel Style: use wxFULL_REPAINT_ON_RESIZE (to force a call
> to the Paint event when resizing the panel)
> - in wxFormBuilder, activate OnPaint and OnLeftUp for m_componentView.
> No need to activate them in your code.

Yeah, I could do that in the form-builder. I thought it was more
explicit this way, and it was easier to deal with while developing
rather than

> -I am thinking using LIB_COMPONENT::Draw and
> LIB_COMPONENT::GetBoundingBox could avoid duplicate code.

Oh, I might have overlooked that there is already something that does that :)

Thanks, I'll see if I can fix this up.

Wayne, let me know if I should keep working on the current patch for
that, or better start a new patch based on this one.

-h

>
> Thanks.
> Your component selection Dialog is very good.
>
>>>
>>> In general it would be better if this panel was its own separate
>>> component (derived from wxPanel) that does the
>>> component draw. If I would do that, do I just override a Render(wxDC *) method ?
>>> I might not get around working on it the next couple of days (busy
>>> short week), is it ok if we submit this now and I'll iterate from here
>>> ?
>>
>> I agree with your assessment about EDA_DRAW_PANEL being decoupled from
>> the frame window.  My guess is that would be quite a significant
>> undertaking given the current design.  If this is something you want to
>> tackle, I would do it one function at a time until all occurrences of
>> GetParent() and GetScreen() are gone and any parent code other than
>> default or overloaded wxWindows member functions are pushed up the stack
>> into EDA_DRAW_FRAME or converted to a wxMessage and sent to the parent
>> window.  Otherwise, you could break things is significant ways if you
>> are not careful.  I'm not sure what your best course of action is in
>> this case.  I never tried to use EDA_DRAW_PANEL in any window other than
>> an EDA_DRAW_FRAME so I don't know how much help I can give you.
>>
>> I will try to get this patch committed ASAP.
>>
>
> --
> Jean-Pierre CHARRAS
>
> _______________________________________________
> 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