Re: [Kicad-developers] Add a cmake option for legacy canvas

[John Beard]

Here is an example of the kind of commit that would remove a feature
from the legacy canvas: Duplicate Item.

This tool doesn't have a lot of code behind it because it mostly
piggy-backs on other block operations, so this patch is a fairly good
example of the overhead and wiring that can be removed. The rest of
the block code is also very complex but most of it is also used for
the copy/paste operation too, so can't be removed at this time.

Nevertheless, this removes over 200 lines of code and touches 18 files
including very pervasive headers, which illustrates the complexity of
legacy tools. The GAL equivalent is around 70 lines of code, contained
in a single file and a few lines for declaring the tool actions,
without touching the "core" headers.

I haven't removed any icons, as that would bloat the diff and make it
less clear for illustrative purposes.

Basic aspects:

* Removal of virtual and non-virtual functions from PCB_BASE_EDIT_FRAME
* Removal of overrides from each of FOOTPRINT_EDIT_FRAME and PCB_EDIT_FRAME
* Removal of legacy hotkey and handlers
* Removal of tool from legacy menus
* Removal of duplicate-specific code in the block operation

The primary "win" here is the removal of interfaces from the *FRAME
classes. The secondary win is the (very minor in this case)
simplification of backend code (block.cpp) which would help future
removal efforts by decreasing complexity. Tertiary win is saving on
code size, compile/link times and bug surface, but because this tool
doesn't have a lot of backend of it's own, it's not a significant
factor.

Note: I am not submitting this patch in the expectation that it be
merged, it's an example (I wouldn't stop anyone though!).

Cheers,

John

On Sun, Feb 5, 2017 at 1:25 PM, John Beard  wrote:
> Hi,
>
> I support the general idea of being able to turn off bits of the
> legacy canvas at will, in principle. However, I'm not sure how many
> people will actually bother to check if stuff behind an #ifdef is
> getting broken, and legacy stuff is quite vulnerable because of tight
> binding to large classes like PCB_EDIT_FRAME, etc. I think if you had
> this option on, you'd most get one of these options: 1) no-one uses it
> 2) everyone who does use it has to compile and test everything twice
> (leading to case 1) or 3) lots of people use it and don't fully check
> legacy works for every patch, and effort has to be expended
> maintaining and revising patches and requesting re-tests (this is
> already bad enough with platform-specific problems, we don't want to
> segment testing any further, I think).
>
> I would suggest that the legacy canvas remains as it is, but
> individual non-essential features of it that are available and solid
> in GAL get removed piece by piece. This way you get everyone and
> compiling and testing and _using_ the same code, and no-one loses a
> feature for want of a re-compile (they might need to switch canvases
> though). Additionally, legacy code complexity is drained down slowly
> and deliberately, a logically separate piece at a time, which helps
> locate refactoring opportunities and indentify legacy switch-off
> problem points sooner rather than later. Waiting for GAL-Day and
> nuking legacy fast would be more likely (IMO) to result in breakages
> and less careful consideration of how each separate piece can be
> tidied away into a GAL-only framework.
>
> Examples of "non-essential" features might be things like net
> highlighting, arrays tools and so on, and could even extend to things
> like the drawing tool and copy/paste over time, leaving only a hard
> core of very basic moving/view change functions and any remaining
> non-GAL features, which get drawn off over time. Any major breakage
> during this time (e.g. if the legacy track tool is removed and PNS
> breaks) is breakage that probably would still have happened during a
> firesale removal of legacy, but it will happen under controlled and
> more easily-revertable circumstances. The remaining "core" of legacy
> functionality, by the end of the process, should be dramatically
> smaller and more easily conceptualised, making it easier to finally
> remove it entirely.
>
> I'm not proposing a timeline, but if legacy is to be part of a stable
> release again, I would suggest that fewer features in it would drive
> use and testing of GAL tools, reduce user (especially newly-arrived
> Eagle users!) frustration with using legacy tools, and reduce bug
> surface area though reduction of legacy code volume, which is
> substantial, complex, and ultimately doomed anyway. It would also free
> code that is currently used by both GAL and legacy (e.g. many dialogs)
> to be able to evolve more freely when only the GAL calling code needs
> to be updated to support it, enabling faster response to bugs and
> feature requests on both stable and devel branches. It would also
> promote a lot of careful checking of legacy/GAL crossover code and
> refactorings, so it would 

Re: [Kicad-developers] Add a cmake option for legacy canvas

[José Ignacio]

I really like this idea for after 5 is out

On Sat, Feb 4, 2017 at 11:25 PM, John Beard  wrote:

> Hi,
>
> I support the general idea of being able to turn off bits of the
> legacy canvas at will, in principle. However, I'm not sure how many
> people will actually bother to check if stuff behind an #ifdef is
> getting broken, and legacy stuff is quite vulnerable because of tight
> binding to large classes like PCB_EDIT_FRAME, etc. I think if you had
> this option on, you'd most get one of these options: 1) no-one uses it
> 2) everyone who does use it has to compile and test everything twice
> (leading to case 1) or 3) lots of people use it and don't fully check
> legacy works for every patch, and effort has to be expended
> maintaining and revising patches and requesting re-tests (this is
> already bad enough with platform-specific problems, we don't want to
> segment testing any further, I think).
>
> I would suggest that the legacy canvas remains as it is, but
> individual non-essential features of it that are available and solid
> in GAL get removed piece by piece. This way you get everyone and
> compiling and testing and _using_ the same code, and no-one loses a
> feature for want of a re-compile (they might need to switch canvases
> though). Additionally, legacy code complexity is drained down slowly
> and deliberately, a logically separate piece at a time, which helps
> locate refactoring opportunities and indentify legacy switch-off
> problem points sooner rather than later. Waiting for GAL-Day and
> nuking legacy fast would be more likely (IMO) to result in breakages
> and less careful consideration of how each separate piece can be
> tidied away into a GAL-only framework.
>
> Examples of "non-essential" features might be things like net
> highlighting, arrays tools and so on, and could even extend to things
> like the drawing tool and copy/paste over time, leaving only a hard
> core of very basic moving/view change functions and any remaining
> non-GAL features, which get drawn off over time. Any major breakage
> during this time (e.g. if the legacy track tool is removed and PNS
> breaks) is breakage that probably would still have happened during a
> firesale removal of legacy, but it will happen under controlled and
> more easily-revertable circumstances. The remaining "core" of legacy
> functionality, by the end of the process, should be dramatically
> smaller and more easily conceptualised, making it easier to finally
> remove it entirely.
>
> I'm not proposing a timeline, but if legacy is to be part of a stable
> release again, I would suggest that fewer features in it would drive
> use and testing of GAL tools, reduce user (especially newly-arrived
> Eagle users!) frustration with using legacy tools, and reduce bug
> surface area though reduction of legacy code volume, which is
> substantial, complex, and ultimately doomed anyway. It would also free
> code that is currently used by both GAL and legacy (e.g. many dialogs)
> to be able to evolve more freely when only the GAL calling code needs
> to be updated to support it, enabling faster response to bugs and
> feature requests on both stable and devel branches. It would also
> promote a lot of careful checking of legacy/GAL crossover code and
> refactorings, so it would hopefully be a good vehicle for code quality
> discussion too.
>
> I hope that makes as much sense as it did in my head!
>
> Cheers,
>
> John
>
> On Tue, Jan 31, 2017 at 11:22 PM, Chris Pavlina 
> wrote:
> > I think it's worth revisiting this. I know we're not ready to remove
> > legacy yet, but we're getting close. Starting to factor it out into a
> > switchable build option is a good way to make sure the transition is
> > smooth and help find anything that is still missing. Obviously the
> > default build option should be to keep legacy in, so users won't notice
> > anything.
> >
> > On Sat, Sep 17, 2016 at 10:59:45PM +1200, Simon Wells wrote:
> >> As legacy canvas in pcbnew is legacy is it worth conditional compiling
> >> all the code related and only used by legacy canvas based on a cmake
> >> option aka something like
> >>
> >> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
> >> who have no use for the legacy canvas as they have a truly functional
> >> opengl canvas to see in their usual workflow if anything is missing.
> >>
> >> I realise that wayne is waiting on a replacement non-gl dependent GAL
> >> canvas before removing legacy, but in the interim is it worth making
> >> it an option making less work later on when its time to remove it
> >>
> >> Simon
> >>
> >> ___
> >> Mailing list: https://launchpad.net/~kicad-developers
> >> Post to : kicad-developers@lists.launchpad.net
> >> Unsubscribe : https://launchpad.net/~kicad-developers
> >> More help   : https://help.launchpad.net/ListHelp
> >
> > ___
> > 

Re: [Kicad-developers] Add a cmake option for legacy canvas

[John Beard]

Hi,

I support the general idea of being able to turn off bits of the
legacy canvas at will, in principle. However, I'm not sure how many
people will actually bother to check if stuff behind an #ifdef is
getting broken, and legacy stuff is quite vulnerable because of tight
binding to large classes like PCB_EDIT_FRAME, etc. I think if you had
this option on, you'd most get one of these options: 1) no-one uses it
2) everyone who does use it has to compile and test everything twice
(leading to case 1) or 3) lots of people use it and don't fully check
legacy works for every patch, and effort has to be expended
maintaining and revising patches and requesting re-tests (this is
already bad enough with platform-specific problems, we don't want to
segment testing any further, I think).

I would suggest that the legacy canvas remains as it is, but
individual non-essential features of it that are available and solid
in GAL get removed piece by piece. This way you get everyone and
compiling and testing and _using_ the same code, and no-one loses a
feature for want of a re-compile (they might need to switch canvases
though). Additionally, legacy code complexity is drained down slowly
and deliberately, a logically separate piece at a time, which helps
locate refactoring opportunities and indentify legacy switch-off
problem points sooner rather than later. Waiting for GAL-Day and
nuking legacy fast would be more likely (IMO) to result in breakages
and less careful consideration of how each separate piece can be
tidied away into a GAL-only framework.

Examples of "non-essential" features might be things like net
highlighting, arrays tools and so on, and could even extend to things
like the drawing tool and copy/paste over time, leaving only a hard
core of very basic moving/view change functions and any remaining
non-GAL features, which get drawn off over time. Any major breakage
during this time (e.g. if the legacy track tool is removed and PNS
breaks) is breakage that probably would still have happened during a
firesale removal of legacy, but it will happen under controlled and
more easily-revertable circumstances. The remaining "core" of legacy
functionality, by the end of the process, should be dramatically
smaller and more easily conceptualised, making it easier to finally
remove it entirely.

I'm not proposing a timeline, but if legacy is to be part of a stable
release again, I would suggest that fewer features in it would drive
use and testing of GAL tools, reduce user (especially newly-arrived
Eagle users!) frustration with using legacy tools, and reduce bug
surface area though reduction of legacy code volume, which is
substantial, complex, and ultimately doomed anyway. It would also free
code that is currently used by both GAL and legacy (e.g. many dialogs)
to be able to evolve more freely when only the GAL calling code needs
to be updated to support it, enabling faster response to bugs and
feature requests on both stable and devel branches. It would also
promote a lot of careful checking of legacy/GAL crossover code and
refactorings, so it would hopefully be a good vehicle for code quality
discussion too.

I hope that makes as much sense as it did in my head!

Cheers,

John

On Tue, Jan 31, 2017 at 11:22 PM, Chris Pavlina  wrote:
> I think it's worth revisiting this. I know we're not ready to remove
> legacy yet, but we're getting close. Starting to factor it out into a
> switchable build option is a good way to make sure the transition is
> smooth and help find anything that is still missing. Obviously the
> default build option should be to keep legacy in, so users won't notice
> anything.
>
> On Sat, Sep 17, 2016 at 10:59:45PM +1200, Simon Wells wrote:
>> As legacy canvas in pcbnew is legacy is it worth conditional compiling
>> all the code related and only used by legacy canvas based on a cmake
>> option aka something like
>>
>> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
>> who have no use for the legacy canvas as they have a truly functional
>> opengl canvas to see in their usual workflow if anything is missing.
>>
>> I realise that wayne is waiting on a replacement non-gl dependent GAL
>> canvas before removing legacy, but in the interim is it worth making
>> it an option making less work later on when its time to remove it
>>
>> Simon
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp

___
Mailing list: 

Re: [Kicad-developers] Add a cmake option for legacy canvas

[Chris Pavlina]

I think it's worth revisiting this. I know we're not ready to remove
legacy yet, but we're getting close. Starting to factor it out into a
switchable build option is a good way to make sure the transition is
smooth and help find anything that is still missing. Obviously the
default build option should be to keep legacy in, so users won't notice
anything.

On Sat, Sep 17, 2016 at 10:59:45PM +1200, Simon Wells wrote:
> As legacy canvas in pcbnew is legacy is it worth conditional compiling
> all the code related and only used by legacy canvas based on a cmake
> option aka something like
> 
> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
> who have no use for the legacy canvas as they have a truly functional
> opengl canvas to see in their usual workflow if anything is missing.
> 
> I realise that wayne is waiting on a replacement non-gl dependent GAL
> canvas before removing legacy, but in the interim is it worth making
> it an option making less work later on when its time to remove it
> 
> Simon
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

Re: [Kicad-developers] Add a cmake option for legacy canvas

[Wayne Stambaugh]

On 9/17/2016 6:59 AM, Simon Wells wrote:
> As legacy canvas in pcbnew is legacy is it worth conditional compiling
> all the code related and only used by legacy canvas based on a cmake
> option aka something like
> 
> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
> who have no use for the legacy canvas as they have a truly functional
> opengl canvas to see in their usual workflow if anything is missing.
> 
> I realise that wayne is waiting on a replacement non-gl dependent GAL
> canvas before removing legacy, but in the interim is it worth making
> it an option making less work later on when its time to remove it
> 
> Simon
> 

I'm not sure what this would buy the project in terms of usefulness.  As
far as I am concerned there are still to far too many issues with the
gal canvas and new tool framework for me to give the go ahead to pull
the plug on the legacy canvas.  Here is my short list:

Stability: the current state of the p router stability is not what it
needs to be.  We still have users that seem to be able to crash it at
will.  It seems to me there are memory management issues at play here
that need some scrutiny.  IMO this needs to be bullet proof.  Period.

Polish: there are too many missing context menu entries from the gal
context menus which make the gal canvases less convenient to use than
the legacy canvas.  The last I heard, custom hot keys are still not
respected by gal but I could be wrong about this.  There is still no
track clearance indicator even routing a single trace.  There is a long
laundry list of these types of minor annoyances that KiCad users have
come to expect.

Speed: unfortunately there are still platforms were opengl is not usable
and the cairo canvas is unacceptably slow.  Either the cairo canvas will
need a significant increase in performance or someone will have to write
a wxDC painter object or we just keep putting it off until opengl works
well everywhere.

Once these issues are addressed, then we can start talking about
removing the legacy canvas.  Until then, it will have to stay.  I'm not
willing to force users (me) over to it until it's ready for prime time.
None of this should really surprise anyone.  We've discussed it many
times.  Now's your chance to be a hero and step up to the plate and fix
these issues.

Cheers,

Wayne


___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

Re: [Kicad-developers] Add a cmake option for legacy canvas

[Nick Østergaard]

I am not entirely sure what your intention exactly.

The message looks like a commit message rather than a request for
comments. Especially because I can not detect any questionmark, but I
will assumeit is a question anyways. It guess it requries quite some
ifdefs.

May I suggest just naming it KICAD_LEGACY_CANVAS which can be set ON
or OFF? The _BUILD seems superfluous to me. Yes, I know this is
bikeshedding, it is just a suggestion afterall.

But really, I don't see any reason start doing this before GAL support
is added everywhere. It could possible be added while enabling GAL
support everywhere, but I think it might not be worth the effort
trying to add it. I think it might be easer to just rip it out when
that is going to happen directly rather than adding ifdefs.

But in reality, I like the switching concept we have in pcbnew right
now. It makes it very easy to compare the two canvasses because it
does not require a seperate build, and you can switch instantly.


2016-09-17 12:59 GMT+02:00 Simon Wells :
> As legacy canvas in pcbnew is legacy is it worth conditional compiling
> all the code related and only used by legacy canvas based on a cmake
> option aka something like
>
> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
> who have no use for the legacy canvas as they have a truly functional
> opengl canvas to see in their usual workflow if anything is missing.
>
> I realise that wayne is waiting on a replacement non-gl dependent GAL
> canvas before removing legacy, but in the interim is it worth making
> it an option making less work later on when its time to remove it
>
> Simon
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp