kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 2/7/2017 11:02 AM, jp charras wrote:
> Le 07/02/2017 à 14:29, Chris Pavlina a écrit :
>> Commit 	43cb4560bfcf0624e9707c4c512cc3ccce385ce9
>>
>> Rewrite code for PANEL_PREV_3D because the way events were previously managed are not compatible with a good mouse event management.
>> To avoid a lot of tedious code, wxFormbuilder is used to create the PANEL_PREV_3D_BASE class.
>>
>> ...didn't we agree we should reduce the use of wxformbuilder in new
>> code? There are plenty of ways the tedious code could be avoided without
>> using it - for instance, that panel is very repetitive, a function could
>> be created to populate a section...
>>
> 
> AFAIK, there were some time ago concerns about wxFormbuilder because there was very few activity on
> wxFormbuilder site, especially concerns about it looked like it was no more maintained (no or very
> few commits during many months).
> 
> Since a few months, wxFormbuilder code has moved on Github
> ( https://github.com/wxFormBuilder/wxFormBuilder )
> 
> and since this move many commits were made on Github.
> 
> So these concerns about wxFormbuilder are no more valid, from my point of view.

Yes, but how long will this last?  If memory serves, wxFormbuilder has
gone through several of these up and down cycles since we started using it.

> 
> wxFormbuilder is not just a GUI builder.
> It avoids mistakes by misusing some wxWidgets features, especially events, not always easy to use in
> complicated cases (like in the PANEL_PREV_3D).
> Misusing wxWidgets features can create issues very hard to fix, because there is no bug in the code,
> just improper use of some feature.
> The perfect case was the PANEL_PREV_3D. Remember patches to add increment/decrement by mouse wheel
> did not work.
> 
> The reason was not a bug in code, but improper use of events, which was *immediately* found by
> wxFormbuilder.

There was a bug in the code.  The person who wrote it did not understand
how the events worked which is not unusual.  Event driven systems
(almost all GUIs) are difficult because you have to understand the
interaction and timing of events to get it correct.  Using wxFB does not
guarantee that your dialogs will be free of event handler issues.  You
could just as easily break thinks in the derived class.  In fact, almost
every dialog in kicad is broken by directly calling either EndModal() or
EndQuasiModal() in the OK button event handler.  If you don't believe
me, open a dialog that calls EndQuasiModal() in the ok button event
handler by opening it with the ShowModal() call and see what happens
when you click the ok button.  You should never directly call EndModal()
or EndQuasiModal() from inside the dialog code unless you can guarantee
it will be open in the correct mode.  The only reason we don't have
issues with this is that the person who wrote the dialog typically
writes the code that calls the dialog so the know which show function to
use.

> 
> 
> Tedious code is not always a repetitive code.
> 
> It can be due to other reasons, for instance when using a lot of sizers,
> not always easy to manage by hand.
> For instance it avoid this repetitive cycle:
> "set sizers, compile and see the result on screen" made many times.

That's why I suggested using wxFB as a prototyping tool rather than a
code generating tool.

> 
> So, currently, I do not see a good reason to avoid using wxFormbuilder, because it is now maintained.

There is one big reason to avoid it: inconsistency.  The sizer padding
in our dialogs is all over the map.  This is a direct result of using
wxFB.  It encourages every designer to do their own thing when laying
out dialogs and discourages reuse.   The wx folks even have an object to
solve this problem: wxSizerFlags()[1].  It's trivial to create a base
wxSizerFlag() object (and derivatives) and use it in every dialog and
the padding would always be the same.  If at some time in the future you
decide increase or decrease the padding, a single line of code would
update every dialog.  You would have to manually edit every single
dialog with wxFB to achieve this.

I understand the appeal of wxFB especially when programming in C++
because of the change layout/compile iterations.  But it would be
trivial to use wxFB to see how layout elements effect the dialog layout
without dragging all of the wxFB stuff into kicad along with it's
inherent drawbacks.

[1]: http://docs.wxwidgets.org/3.0/classwx_sizer_flags.html