← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Improve template selector for macOS

 

Thanks Seppe.

This bug has been irritating, thanks for fixing it!

Adam Wolf

On Mon, Sep 18, 2017 at 9:58 AM, jp charras <jp.charras@xxxxxxxxxx> wrote:
> Le 18/09/2017 à 16:11, Seppe Stas a écrit :
>> This fixes some issues with the template selector that cause it not to work properly on macOS.
>>
>> The main issue is a race condition where `OnPageChange` uses `m_notebook->GetSelection()` to get the
>> selected page number instead of `event.GetSelection()` as defined in the docs
>> <http://docs.wxwidgets.org/3.1/classwx_notebook.html#a24783577628b3da1537a9476629cc72b>.
>>
>> On macOS `m_notebook->GetSelection()` sometimes / typically returns the page number of the old page,
>> resulting in the template selector not working properly and issues as described in lp:1492577
>> <https://bugs.launchpad.net/kicad/+bug/1492577> and lp:1672116
>> <https://bugs.launchpad.net/kicad/+bug/1672116>.
>>
>> Another problem is the template pages not being rendered properly when initially added using the
>> `AddTemplatesPage` call. Only when the notebook page is active and shown it is possible to render
>> the template page, pointing in the direction of another race condition.
>>
>> I did not find a proper solution to this problem, but ignoring the template path not being updated
>> at least allows the "validate" and "browse" buttons to render the templates.
>>
>> On the long run, it's probably a good idea to completely redo the template system. It's riddled with
>> race conditions on macOS and render issues on Ubuntu, and it's one of the only parts of KiCad
>> heavily relying on wxFormBuilder projects.
>>
>> This is my first public contribution to the KiCad project and I plan to fix some more things. I'm
>> not sure if emails to this group or Launchpad PRs are prefered.
>
> Thanks Seppe.
>
> I committed your patch, because it looks good (and tested on W7).
>
> Sending patches to the dev. ML is the right way, especially for small patches, from my point of view.
> They are easy to review and to apply.
>
> --
> 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


References