Re: [Kicad-developers] Library Convention

2014-04-23 Thread John Beard
On 22/04/14 18:10, Lorenzo Marcantonio wrote:

> Tell me if you want them and where to put them;

I'd very much like to see them. Most convenient for me, personally, is
some sort of git repository, at Github or elsewhere. Others may prefer
bzr for easier integration with Launchpad?

> On Tue, Apr 22, 2014 at 07:53:53PM +0300, Vesa Solonen wrote:
>> We should continue on some thread
>> on library-committers list and I hope to see you and Jean-Paul on board
>> as well.

I'd also be willing to have a go at filling in any blanks in the 60617
set, if I can help.

> If you want me on the library list I think someone should make the
> invitation or something...

You can request team membership here:
https://launchpad.net/~kicad-lib-committers

-John

___
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] [PATCH] IPC-7351 Footprint wizard

2014-04-28 Thread John Beard
On 27/04/14 20:18, Francesco Del Degan wrote:
> 
> Yes, I will try to reuse the helper classes that was committed few days ago,
> and fix some issues with it. (I just discovered that actual BGA
> footprint uses all
> 26 letters, but only ABCDEFGHJKLMNPRTUVWY should be allowed) . 

My bad, thanks for catching it! Patch attached, which also fixed a
transposition of the row/col parameters for BGA. This patch will allow
any alphabet to be used by the underlying helper function, the BGA
wizard provides the one you gave above.

>  Chip Resistor, Capacitor and Inductor 
>  Aluminum Electrolytic Capacitors 
>  Tantalum Capacitor 
>  Molded Inductor 
>  Wire Wound Inductor 
>  Molded Resistor, Diode 
>  TO Packages
>  Oscillators and Chip Array’s
>  QFN 
>  SOJ 
>  PLCC 
>  SOT 
>  MELF 
>  LCC 
>  CFP
>  CQFP
>  BGA 
>  QFP 
>  SQFP 
>  TQFP 
>  SOIC 
>  SOP 
>  TSOP
> TSSOP 

A lot of these should be very easily doable by subclassing from the
current SOIC wizard and implementing new default values.

> On Sun, Apr 27, 2014 at 08:01:00PM +0200, jp charras wrote:
>> Be careful with wxPython when it is used in Pcbnew: today we have some
>> troubles with it:
>>- It does not work on Windows (at least for me)
>> - It has serious issues (crashes) (not yet fixed) on Linux.
>>(I do not know the status on OSX)

I did the wizard helpers on the pre-kiway branch of the repo, as the
trunk at that time (haven't tried it since) crashed immediately on
start otherwise. It seemed pretty stable on Linux Mint 15 and 16, 64
bit, FWIW.

> I'm also very interested, at the end, to bring python support more
> deeply (and stable) into kicad itself

Me too, but I have not got around to looking at the C++ side of the
plugin fence yet.

Something that would be handy would be a way to pass strings into the
parameter map. Also, a fixed list of items that is presented in KiCad as
a drop-down would be useful too.

Cheers,

John
diff --git a/pcbnew/scripting/plugins/PadArray.py b/pcbnew/scripting/plugins/PadArray.py
index 99fa70e..3c5d5f7 100644
--- a/pcbnew/scripting/plugins/PadArray.py
+++ b/pcbnew/scripting/plugins/PadArray.py
@@ -75,13 +75,14 @@ class PadGridArray(PadArray):
 
 # handy utility function 1 - A, 2 - B, 26 - AA, etc
 # aIndex = 0 for 0 - A
-def AlphaNameFromNumber(self, n, aIndex = 1):
+# alphabet = set of allowable chars if not A-Z, eg ABCDEFGHJKLMNPRTUVWY for BGA
+def AlphaNameFromNumber(self, n, aIndex = 1, alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"):
 
-div, mod = divmod(n - aIndex, 26)
-alpha = chr(65 + mod)
+div, mod = divmod(n - aIndex, len(alphabet))
+alpha = alphabet[mod]
 
 if div > 0:
-return self.AlphaNameFromNumber(div) + alpha;
+return self.AlphaNameFromNumber(div, aIndex, alphabet) + alpha;
 
 return alpha;
 
diff --git a/pcbnew/scripting/plugins/bga_wizard.py b/pcbnew/scripting/plugins/bga_wizard.py
index 7fd6258..36a86eb 100644
--- a/pcbnew/scripting/plugins/bga_wizard.py
+++ b/pcbnew/scripting/plugins/bga_wizard.py
@@ -24,7 +24,7 @@ import PadArray as PA
 class BGAPadGridArray(PA.PadGridArray):
 
 def NamingFunction(self, x, y):
-return "%s%d" % (self.AlphaNameFromNumber(y + 1), x + 1)
+return "%s%d" % (self.AlphaNameFromNumber(y + 1, alphabet="ABCDEFGHJKLMNPRTUVWY"), x + 1)
 
 
 class BGAWizard(HFPW.HelpfulFootprintWizardPlugin):
@@ -79,7 +79,7 @@ class BGAWizard(HFPW.HelpfulFootprintWizardPlugin):
 pin1Pos = pcbnew.wxPoint(-((rows - 1) * pad_pitch) / 2,
 -((cols - 1) * pad_pitch) / 2)
 
-array = BGAPadGridArray(pad, rows, cols, pad_pitch, pad_pitch, pin1Pos)
+array = BGAPadGridArray(pad, cols, rows, pad_pitch, pad_pitch, pin1Pos)
 array.AddPadsToModule()
 
 #box
___
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


[Kicad-developers] Forward-compatibility in s-expression formats

2014-05-06 Thread John Beard
I think it would be a good idea to allow unknown fields in the
s-expression formats so that an older KiCad doesn't choke on things it
doesn't understand, and doesn't need to.

For example, I was thinking that it might be helpful to add a field to
the footprint format: "part_reference", which would hold a list of
manufacturer and/or vendor IDs associated with this part. For example:

(part_reference (fci 343233222) (farnell 1234567))

Leaving discussions on the actual usefulness of this field aside,
clearly, an older KiCad does not need this field, it can be ignored
harmlessly. However, when saving a footprint, an older KiCad should not
remove this field.

Ignoring *every* unknown field might also not be the best idea, in case
there is one day a field which an older KiCad can't understand, but
should realise "no, this means I can't use this footprint". So maybe
also a "fp_version" field, which, if missing, is assumed to be "1".
Then, if a future footprint declares "(version 2)", because it knows it
contains features a version-1 KiCad will misinterpret, that version-1
KiCad can say "sorry, I'm version 1, you need version 2 or higher". In
this case, it would be nicer to not set the version to 2 if you didn't
need to, so version-1 KiCads can use the footprint.

As it currently stands, *any* KiCad would refuse to load a footprint
with either of these fields, which is what I am proposing that we
change. The "part_reference" field is just an example, and needs more
thought. I was pointed to a discussion about a "device" concept, but
that is outside the scope of this.

Cheers,

John

___
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] Forward-compatibility in s-expression formats

2014-05-06 Thread John Beard

On 06/05/14 21:47, Jean-Paul Louis wrote:

Why not collect all the field of any part being loaded, and consider
all the fields that are not known as “user defined fields”?


In future you may want you use a word for a new field, and you don't 
really want to stomp on some poor sap who thought he could hide his 
internal stock number in "sku", but now KiCad wants to use it for 
"Saalbach Kondensator Union".


I support user-definable keys, and I quite like the CSS approach to 
this, where the extension keys start with "-prefix", eg "-moz-hyphens". 
Or the HTML DOM where data elements look like "data-something", and are 
ignored by everything that is not looking for them.



I would not agree to have a part_reference to be the combination of
two of my user fields that are Supplier1 and PN_Supplier1, Supplier2
and PN_Supplier2, etc..


Part references were just an off-the-cuff example, I'm not proposing 
adding them to footprints.



Just keep those fields, that would allows the program to load ANY
legacy part without breaking the bank.


That's exactly what I mean - older versions of KiCad should be able to 
ignore, but retain, unknown data.


___
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] Forward-compatibility in s-expression formats

2014-05-06 Thread John Beard

On 06/05/14 23:18, Tomasz Wlostowski wrote:


How about about adding a generic attribute set for each part, stored as
a list of type-name-value entries:

(part xxx
(attribute "simulation_model" (string ".cir") )
(attribute "part_reference_farnell" (int 123456 ) )
)

Such scheme would not only allow adding a vendor P/N to each symbol, but
also simulation models, high speed constraints (length, impedance) and
so on.


Yes, this is a good way to do it, we'd just need to define some grammar 
of "types" to allow parsing, and everything else is self-describing. It 
does seem a little verbose, but as Lorenzo said, maybe it's not possible 
to add unknown elements without breaking everything, and we'd have to go 
down a road where new elements always say exactly what they are.


I suppose one question is in that case, why wouldn't all elements be 
self describing, rather than having the "old" elements like "at" having 
some implicit type, and new elements like "location" (say) needing to 
say "(location (coords 3.3 3.3))".


Then again, if you ever come across a data type that we didn't have the 
foresight to include at the start, you'd have to munge it into a string 
or other primitive to express it, or you're back to square one.


Cheers,

John


___
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] Forward-compatibility in s-expression formats

2014-05-06 Thread John Beard

On 06/05/14 21:47, Lorenzo Marcantonio wrote:


That said, I think that *if* the sexp reader can traverse automatically
a generic sexp (without having a grammar, only tokenising) we could
decide that it could 'skip' each subexpression starting with an unknown
token. Maybe with warnings:D


I would have thought this would exactly be done at the end of 
tokenisation - this is when the current parser says "erk, i expected one 
of 'fp_line', 'pad', , I give up now, no footprint for you". 
Instead, what it could do is go, "hmm, don't know what this is, I'll 
keep this bit - up to the matching paren - for later in case we save it 
back, and move on".


Cheers,

John


___
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] Forward-compatibility in s-expression formats

2014-05-08 Thread John Beard
On 08/05/14 15:53, Dick Hollenbeck wrote:
> 
>> That's exactly what I mean - older versions of KiCad should be able to 
>> ignore, but retain, unknown data.
> 
> This sentence has two concepts in it.  One is fools gold, the other is poorly 
> expressed.
> 
> 1) An older kicad should load a data file that is has never seen before.

Is this fools' gold? A PDF reader from 2005 would probably still load a
PDF made by newer software, even if there are new features it cannot
understand. It might say "You can't see the embedded 4D MegaContent
(TM), upgrade to a version that support PDF spec v5.6 for that", but
it will show you what it can. You'd be annoyed if it didn't, it would be
like Flash needing the latest version to watch your video, without the
excuse that is necessary to implement the latest and greatest DRM fad.

> The motivation should be that you want the flexibility to solve a specific 
> problem.

The motivation is that I want the flexibility to add a field to a
.kicad_mod file, that still allows someone else to read the footprint,
if they don't *need* to know about that field. Currently, that will
cause THROW_PARSE_ERROR(), and they get nothing.

> But it should not name future proofing as the motivation, because you don't 
> want to
> upgrade.  That's the part that gives me conceptual grief.

It's not because *I* don't want to upgrade, *I'm* to the one who's
hypothetically extended some aspect of the footprint format for my own
ends. It's because J. Random McTrunkBuild (or even his cousin,
McDistroVersion) shouldn't need to recompile with a patch from me to
read a footprint of mine which has my field "(user_sku 45443)" or
"(jedec_ref "SOT45")" in it, when he doesn't need or care about that
field.

The current state is there no way for people (not intimately familiar
with the .kicad_mod format) downloading a footprint now to know if it's
compatible with their KiCad without loading the footprint and then
guessing that the "Expected" message means the footprint contains
unknown fields, as opposed to being malformed.

Forced recompiles are only free if your time is worthless and any
problems caused by running bleeding-edge PCB software in production
environments have zero cost.

___
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] Forward-compatibility in s-expression formats

2014-05-08 Thread John Beard
On 07/05/14 07:19, Lorenzo Marcantonio wrote:
> On Wed, May 07, 2014 at 12:48:22AM +0100, John Beard wrote:
>> I would have thought this would exactly be done at the end of tokenisation
> 
> More or less, that's the idea :D
> 
> There are various ways to do this but all depends on the flexibility
> required and the current architecture of the parser.

I've had a quick look, and basically what you'd need to do (for a
module) is when you get an unknown symbol, replace the Expected() call
with a call to some new function parseUnknown(). In there, you
basically do:

int depth = 1; // we just passed a (
T token = T_SYMBOL;

while (depth > 0)
{
// we have token and CurText() here, can save them

token = NextTok();

if (token == T_LEFT)
depth ++;
else if (token == T_RIGHT)
depth --;
}

This consumes the unknown element and continues as normal. I have tested
it on (unknown abc) and (unknown (abc 3 3)) in a .kicad_mod, and the
footprint is now readable.

More work to do on keeping the structured data around, even though we
don't know what it means in KiCad, which means we can still potentially
provide it to whoever (Python?) wants it, as well as save it back
non-destructively.

John

___
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] Forward-compatibility in s-expression formats

2014-05-08 Thread John Beard
On 08/05/14 20:12, Lorenzo Marcantonio wrote:
> On Thu, May 08, 2014 at 07:31:47PM +0100, John Beard wrote:
>> it will show you what it can. You'd be annoyed if it didn't, it would be
>> like Flash needing the latest version to watch your video, without the
>> excuse that is necessary to implement the latest and greatest DRM fad.
> 
> Uhm... doesn't actually flash or java work this way? I uninstalled them
> after being tired of the weekly updates

Indeed they do, and isn't it annoying!

> I'm still convinced that a simple generic something like (attribute
> "user_sku" "45443"), like that supported by gencad would be useful for
> your needs. eeschema attributes works like that, too.

A generic attribute *would* be useful for a wide range of things, but
I'd like to see if it isn't just as easy to use arbitrarily-structured
data first.

John


___
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] Forward-compatibility in s-expression formats

2014-05-09 Thread John Beard
On 08/05/14 20:44, Dick Hollenbeck wrote:
> On 05/08/2014 02:18 PM, Dick Hollenbeck wrote:
>> On 05/08/2014 01:31 PM, John Beard wrote:
>>> On 08/05/14 15:53, Dick Hollenbeck wrote:
>> Submit a patch.
>>
>> Hope it gets accepted.
> 
> Since your interest is limited to the *.kicad_mod file.  You can use the 
> mentioned DOM
> parser in your rewrite of the KICAD_PLUGIN::Footprint*() functions.

My interest is not limited solely to .kicad_mod formats, they are just a
convenient starting place with more immediate use cases for extended
data, particularly since there is useful Python integration, which would
be a natural place to use data not handled by the KiCad core.

I'm not presuming to rewrite anything significant.

> Please include comparative benchmarks with your patch submission.  If there's 
> no
> appreciable performance hit, then you have a chance getting the patch 
> accepted.
> 

Of course. Is there existing infrastructure for benchmarking, for
example any used when designing the current parser? I see a
container_test.cpp in tools with some timing code, but no other
significant usage of GetRunningMicroSecs(). I also see
USE_INSTRUMENTATION in pcbnew/files.cpp, but that seems to be the only
use, and is not really useful for me, as I don't want to test my
network/disk/cache IO, and the setup code, I only care about the parser.

John

___
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] Forward-compatibility in s-expression formats

2014-05-17 Thread John Beard

On 09/05/14 19:49, Maciej Sumiński wrote:


It was not used for when designing the current parser, but you may find
useful include/profile.h. Have a look at common/view/view.cpp for an
example of usage (search for 'prof_counter').


Thanks, that looks useful.

I'm trying to write a separate test application that hooks into the PCB
parsing code to be able to, say, parse a module 1,000,000 times and time
it. However, as this code lives in pcbnew_kiface, which is compiled as a
module, I can't link with it. The only other test programs I can see use
common, so they can link.

What would be the right way to get test code to use pcbnew functions
like this?  Add directly into pcbnew and ifdef it out, in which case it
will bitrot in its ifdef prison? Or is there a way to do this over
KiWay, and make it buildable in normal configurations too, without
having to rebuild pcbnew do the tests?

Thanks,

John

___
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] [Kicad-lib-committers] Silk screens over pads and naming

2014-06-13 Thread John Beard
On 13/06/14 19:36, Lorenzo Marcantonio wrote:
> On Fri, Jun 13, 2014 at 06:46:37PM +0100, John Beard wrote:
>> The silk screen is put so it can be seen after soldering. The ASCII art
>> arrow is an assembly layer feature, modelled after the IPC standard.
> 
> Sorry my fault. I was thinking was a fancy silk. IPC assemblies are
> simply notched rectangles usually.

So a rectangle that covers the whole device with a missing corner? And
no indication for the wings or pin exit side? Like the attached PNG?

My only concern is that doesn't really look like the component,
especially when it comes to the pin-1 feature, which is more easily
visually registered by the pin exit side rather than the microscopic "1"
moulded into the case. Would that make it harder to interpret the
assembly drawing?

John
___
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] [Kicad-lib-committers] Silk screens over pads and naming

2014-06-13 Thread John Beard
On 13/06/14 20:44, Lorenzo Marcantonio wrote:
> On Fri, Jun 13, 2014 at 08:33:02PM +0100, John Beard wrote:
>> OK, I won't worry too much about it and get one with making more
>> footprints! Are there any more things you'd like to see changed? No
>> matter how tiny!
> 
> These are already *way* nicer than the ones I use at the moment:D

Thanks! Please, please, tell me at once if any footprints can be
improved and I can update my methods. I'm very keen to make the KiCad
libs better than anything else out there! :-)

> You know, usually is better if the circuit works as intended rather than
> having nice drawings.

Sure, but when writing a tool that can generate hundreds of components
at a time, might as well take the time to get them looking nice! It also
helps to refine the script that generates them, as the more you change,
the more refactoring you end up doing!

> Nice drawings however are a bonus, like the 3D view (never had a good
> reason to use it)

In practice, me neither, but now the mech guys can (hopefully) import
the new KiCad output, maybe I can start! I definitely think footprints
come before 3D models, but I would really like KiCad to be able to ship
with a comprehensive library of both, that would be great selling
(giving?) point.

Cheers,

John


___
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] [Kicad-lib-committers] Silk screens over pads and naming

2014-06-13 Thread John Beard
On 13/06/14 21:23, Nick Østergaard wrote:

> I like the 3D view very much, even the old simple one is great for
> verifying tracks. I often find things I want to change when looking at
> the 3D view of just the traces and silk.

I agree, I think it's having an alaternative perspective. A bit like
going away for lunch and coming back, but without having to leave your
desk!

> 3D models for parts is definitely nice, it is some nice work Cirilo has
> made. I have still to use the IDF funtionality for other than just testing.

The mechanical people here have just got a 3D printer. Getting it on
there is my next plan! Now the IDF stuff is in, maybe OpenSCAD and/or
FreeCAD + Python can be used to greater effect for scriptable
components. I still have problems colouring things nicely, but that's
another email thread!

John

___
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


[Kicad-developers] Footprint wizard changes

2014-07-05 Thread John Beard

Hi,

I have made some changes to the helpers for the footprint wizards. There
is now a matrix-based transform stack that helps simplify calculations
that footprint wizards need to do. The patch attached is based on BZR 
rev 4975.


There are also some tests in the pcbnew/scripting/tests directory
(test_board.py and test_fpw.py). These use a method I'm not very happy
with: they construct a kicad_pcb file with the relevant modules in it,
and then disassemble it into modules. I couldn't see a way to directly
export modules to .kicad_mod files using the Python interface.

It also uses fixed sized "cells" as the call to module.GetBoundingBox()
segfaults.

I am very open to suggestions for this, but as it stands, at least it's
a fast-turnaround way to write new wizards and make sure the existing
ones aren't broken. I use it like this:

$ pcbnew/scripting/tests/test_fpw.py && pcbnew /tmp/test.kicad_pcb

Thanks,

John
diff --git a/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py b/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
index d1046bd..fcd893a 100644
--- a/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
+++ b/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
@@ -14,7 +14,11 @@
 #  MA 02110-1301, USA.
 #
 
+from __future__ import division
+
 import pcbnew
+import math
+
 
 class FootprintWizardDrawingAids:
 """
@@ -24,31 +28,285 @@ class FootprintWizardDrawingAids:
 A "drawing context" is provided which can be used to set and retain
 settings such as line width and layer
 """
+
+# directions (in degrees, compass-like)
+dirN = 0
+dirNE = 45
+dirE = 90
+dirSE = 135
+dirS = 180
+dirSW = 225
+dirW = 270
+dirNW = 315
+
+# flip constants
+flipNone = 0
+flipX = 1  # flip X values, i.e. about Y
+flipY = 2  # flip Y valuersabout X
+flipBoth = 3
+
+xfrmIDENTITY = [1, 0, 0, 0, 1, 0]  # no transform
+
 def __init__(self, module):
 self.module = module
-#drawing context defaults
+# drawing context defaults
 self.dc = {
-'layer': pcbnew.SILKSCREEN_N_FRONT,
-'width': pcbnew.FromMM(0.2)
+'layer': pcbnew.F_SilkS,
+'width': pcbnew.FromMM(0.2),
+'transforms': [],
+'transform': self.xfrmIDENTITY
 }
 
+def PushTransform(self, mat):
+"""
+Add a transform to the top of the stack and recompute the
+overall transform
+"""
+self.dc['transforms'].append(mat)
+self.RecomputeTransforms()
+
+def PopTransform(self, num=1):
+"""
+Remove a transform from the top of the stack and recompute the
+overall transform
+"""
+
+for i in range(num):
+mat = self.dc['transforms'].pop()
+self.RecomputeTransforms()
+return mat
+
+def ResetTransform(self):
+"""
+Reset the transform stack to the identity matrix
+"""
+self.dc['transforms'] = []
+self.RecomputeTransforms()
+
+def _ComposeMatricesWithIdentity(self, mats):
+"""
+Compose a sequence of matrices together by sequential
+pre-mutiplciation with the identity matrix
+"""
+
+x = self.xfrmIDENTITY
+
+for mat in mats:
+#precompose with each transform in turn
+x = [
+x[0] * mat[0] + x[1] * mat[3],
+x[0] * mat[1] + x[1] * mat[4],
+x[0] * mat[2] + x[1] * mat[5] + x[2],
+x[3] * mat[0] + x[4] * mat[3],
+x[3] * mat[1] + x[4] * mat[4],
+x[3] * mat[2] + x[4] * mat[5] + x[5]]
+
+return x
+
+def RecomputeTransforms(self):
+"""
+Re-compute the transform stack into a single transform and
+store in the DC
+"""
+self.dc['transform'] = self._ComposeMatricesWithIdentity(
+self.dc['transforms'])
+
+def TransformTranslate(self, x, y, push=True):
+"""
+Set up and return a transform matrix representing a translartion
+optionally pushing onto the stack
+
+(   1  0   x  )
+(   0  1   y  )
+"""
+mat = [1, 0, x, 0, 1, y]
+
+if push:
+self.PushTransform(mat)
+return mat
+
+def TransformFlipOrigin(self, flip, push=True):
+"""
+Set up and return a transform matrix representing a horizontal,
+vertical or both flip about the origin
+"""
+mat = None
+if flip == self.flipX:
+mat = [-1, 0, 0, 0, 1, 0]
+elif flip == self.flipY:
+mat = [1, 0, 0, 0, -1, 0]
+elif flip == self.flipBoth:
+mat = [-1, 0, 0, 0, -1, 0]
+elif flip == self.flipNone:
+mat = self.xfrmIDENTITY
+else:
+raise ValueError
+
+if push:
+self.PushTransform(mat)
+return mat
+
+def TransformFlip(self, x, y,

[Kicad-developers] Use of std::vector in Python

2014-07-12 Thread John Beard

Hi,

I am trying to get fp_poly working in the footprint wizards. It seems
that I should be using pcbnew.DRAWSEGMENT.SetPolyPoints(), which takes a
std::vector as an argument. I tried to feed it a Python list
of wxPoints, but it couldn't handle that:


 File 
"/home/john/src/kicad/pcbnew/scripting/tests/../plugins/FootprintWizardDrawingAids.py",
 line 367, in FilledPolygon
   outline.SetPolyPoints(xpts)
 File "/home/john/local/kicad/usr/lib/python2.7/dist-packages/pcbnew.py", line 
5773, in SetPolyPoints
   return _pcbnew.DRAWSEGMENT_SetPolyPoints(self, *args)
TypeError: in method 'DRAWSEGMENT_SetPolyPoints', argument 2 of type 'std::vector< 
wxPoint,std::allocator< wxPoint > > const &'


I have never used SWIG before, but I tried to cargo-cult-hack something
into pcbnew/scripting/board.i like this:


 %template (wxPoint_Vector) std::vector;


But this produced the following error when run:


 File "/home/john/src/kicad/pcbnew/scripting/tests/test_board.py", line 23, in 

   import pcbnew
 File "/home/john/local/kicad/usr/lib/python2.7/dist-packages/pcbnew.py", line 12041, 
in 
   class wxPoint_Vector(_object):
 File "/home/john/local/kicad/usr/lib/python2.7/dist-packages/pcbnew.py", line 
12206, in wxPoint_Vector
   __swig_destroy__ = _pcbnew.delete_wxPoint_Vector
AttributeError: 'module' object has no attribute 'delete_wxPoint_Vector'


Does anyone know how a vector of wxPoints (or any class, more generally)
can be worked into the SWIG interface for use via Python?

Thanks,

John

___
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


[Kicad-developers] PATCH: Footprint wizard helpers

2014-09-11 Thread John Beard
Hi,

I have a patch (fpw-overhaul.diff) for the Python footprint wizard
helpers. This adds a few more drawing functions, such as for circles,
and also uses a matrix-based transform stack with greatly simplifies
constructing footprints consisting of regularly spaced elements (e.g. in
lines, grids, circles, or some list of points that you specify).

This fixes #1366299. It seems FootprintWizardDrawingAids.py has an
old-style layer define that was not patched in r4969 (see #1333268).

I also attach a follow-up patch (fpw-tests.diff) containing a really
basic test harness to run though a few footprints with pre-defined
parameters, save them to a PCB board and also optionally "rip" the
modules out to a .pretty directory. I use something like this to
generate whole .pretties programmatically.

This patch contains a few nasty hacks as some things don't work through
the Python binding, so it may not be time to put it in. Though putting
it in would at least allow Jenkins to make sure the footprint wizards
can execute without dying. Attached for comment, nevertheless.

Thanks,

John
diff --git a/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py b/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
index d1046bd..ba82ebe 100644
--- a/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
+++ b/pcbnew/scripting/plugins/FootprintWizardDrawingAids.py
@@ -14,7 +14,11 @@
 #  MA 02110-1301, USA.
 #
 
+from __future__ import division
+
 import pcbnew
+import math
+
 
 class FootprintWizardDrawingAids:
 """
@@ -24,31 +28,291 @@ class FootprintWizardDrawingAids:
 A "drawing context" is provided which can be used to set and retain
 settings such as line width and layer
 """
+
+# directions (in degrees, compass-like)
+dirN = 0
+dirNE = 45
+dirE = 90
+dirSE = 135
+dirS = 180
+dirSW = 225
+dirW = 270
+dirNW = 315
+
+# flip constants
+flipNone = 0
+flipX = 1  # flip X values, i.e. about Y
+flipY = 2  # flip Y valuersabout X
+flipBoth = 3
+
+xfrmIDENTITY = [1, 0, 0, 0, 1, 0]  # no transform
+
 def __init__(self, module):
 self.module = module
-#drawing context defaults
+# drawing context defaults
 self.dc = {
-'layer': pcbnew.SILKSCREEN_N_FRONT,
-'width': pcbnew.FromMM(0.2)
+'layer': pcbnew.F_SilkS,
+'width': pcbnew.FromMM(0.2),
+'transforms': [],
+'transform': self.xfrmIDENTITY
 }
 
+def PushTransform(self, mat):
+"""
+Add a transform to the top of the stack and recompute the
+overall transform
+"""
+self.dc['transforms'].append(mat)
+self.RecomputeTransforms()
+
+def PopTransform(self, num=1):
+"""
+Remove a transform from the top of the stack and recompute the
+overall transform
+"""
+
+for i in range(num):
+mat = self.dc['transforms'].pop()
+self.RecomputeTransforms()
+return mat
+
+def ResetTransform(self):
+"""
+Reset the transform stack to the identity matrix
+"""
+self.dc['transforms'] = []
+self.RecomputeTransforms()
+
+def _ComposeMatricesWithIdentity(self, mats):
+"""
+Compose a sequence of matrices together by sequential
+pre-mutiplciation with the identity matrix
+"""
+
+x = self.xfrmIDENTITY
+
+for mat in mats:
+#precompose with each transform in turn
+x = [
+x[0] * mat[0] + x[1] * mat[3],
+x[0] * mat[1] + x[1] * mat[4],
+x[0] * mat[2] + x[1] * mat[5] + x[2],
+x[3] * mat[0] + x[4] * mat[3],
+x[3] * mat[1] + x[4] * mat[4],
+x[3] * mat[2] + x[4] * mat[5] + x[5]]
+
+return x
+
+def RecomputeTransforms(self):
+"""
+Re-compute the transform stack into a single transform and
+store in the DC
+"""
+self.dc['transform'] = self._ComposeMatricesWithIdentity(
+self.dc['transforms'])
+
+def TransformTranslate(self, x, y, push=True):
+"""
+Set up and return a transform matrix representing a translartion
+optionally pushing onto the stack
+
+(   1  0   x  )
+(   0  1   y  )
+"""
+mat = [1, 0, x, 0, 1, y]
+
+if push:
+self.PushTransform(mat)
+return mat
+
+def TransformFlipOrigin(self, flip, push=True):
+"""
+Set up and return a transform matrix representing a horizontal,
+vertical or both flip about the origin
+"""
+mat = None
+if flip == self.flipX:
+mat = [-1, 0, 0, 0, 1, 0]
+elif flip == self.flipY:
+mat = [1, 0, 0, 0, -1, 0]
+elif flip == self.flipBoth:
+mat = [-1, 0, 0, 0, -1, 0]
+elif flip == self.flipNone:
+mat = self.xfrmID

[Kicad-developers] [PATCH] Use the main workspace background in component chooser/browser dialogues

2014-09-23 Thread John Beard

Hi,

Here is a small patch to make the component chooser dialogue and the
library browser dialogue use the main workspace background colour (white
or black, as configured) instead of white, which otherwise looks
inconsistent when the main workspace is black.

Because DIALOG_CHOOSE_COMPONENT and LIB_VIEW_FRAME both take wxWindow*
as the parents, you can't get directly to the EDA_DRAW_FRAME*. I have
gone around this with a dynamic_cast.

An alternative would be to do what DIALOG_LIB_EDIT_PIN does and take an
EDA_DRAW_FRAME* in the constructor, but pass a wxWindow* down to the
base class. Then, in the constructor, you can access the EDA_DRAW_FRAME
(and in this case, we'd have to save some state, perhaps a bool or
EDA_COLOR_T?)

Additionally, I have removed the saving and loading of the
LIBVIEW_BGCOLOR config. If it still makes sense to have the library
browser have a background colour independent of the parent, then maybe
the colour could be assigned by the first of these to be found:

   * Explicit config
   * Parent
   * White (hardcoded)

I think this logic would be a little complex if this config is not
actively useful.

John

diff --git a/eeschema/dialogs/dialog_choose_component.cpp b/eeschema/dialogs/dialog_choose_component.cpp
index 383f7fa..a518f6e 100644
--- a/eeschema/dialogs/dialog_choose_component.cpp
+++ b/eeschema/dialogs/dialog_choose_component.cpp
@@ -260,7 +260,14 @@ void DIALOG_CHOOSE_COMPONENT::OnHandlePreviewRepaint( wxPaintEvent& aRepaintEven
 void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART*  aComponent, int aUnit )
 {
 wxPaintDC dc( m_componentView );
-dc.SetBackground( *wxWHITE_BRUSH );
+
+bool white_bg = true;
+
+// Attempt to re-use the colour from the parent EDA_DRAW_FRAME
+if ( EDA_DRAW_FRAME* parent_frame = dynamic_cast( m_parent ) )
+white_bg = parent_frame->GetDrawBgColor() == WHITE;
+
+dc.SetBackground( white_bg ? *wxWHITE_BRUSH : *wxBLACK_BRUSH );
 dc.Clear();
 
 if( aComponent == NULL )
diff --git a/eeschema/viewlib_frame.cpp b/eeschema/viewlib_frame.cpp
index 0596d19..b3bf798 100644
--- a/eeschema/viewlib_frame.cpp
+++ b/eeschema/viewlib_frame.cpp
@@ -497,19 +497,19 @@ void LIB_VIEW_FRAME::ExportToSchematicLibraryPart( wxCommandEvent& event )
 #define LIBLIST_WIDTH_KEY wxT( "ViewLiblistWidth" )
 #define CMPLIST_WIDTH_KEY wxT( "ViewCmplistWidth" )
 
-// Currently, the library viewer has no dialog to change the background color
-// of the draw canvas. Therefore the background color is here just
-// in case of this option is added to some library viewer config dialog
-#define LIBVIEW_BGCOLOR   wxT( "LibviewBgColor" )
-
-
 void LIB_VIEW_FRAME::LoadSettings( wxConfigBase* aCfg )
 {
 EDA_DRAW_FRAME::LoadSettings( aCfg );
 
 wxConfigPathChanger cpc( aCfg, m_configPath );
 
-EDA_COLOR_T itmp = ColorByName( aCfg->Read( LIBVIEW_BGCOLOR, wxT( "WHITE" ) ) );
+bool white_bg = true;
+
+// Attempt to re-use the colour from the parent EDA_DRAW_FRAME
+if ( EDA_DRAW_FRAME* parent_frame = dynamic_cast( m_parent ) )
+white_bg = parent_frame->GetDrawBgColor() == WHITE;
+
+EDA_COLOR_T itmp = ColorByName( white_bg ? wxT( "WHITE" ) : wxT( "BLACK" ) );
 SetDrawBgColor( itmp );
 
 aCfg->Read( LIBLIST_WIDTH_KEY, &m_libListWidth, 100 );
@@ -538,7 +538,6 @@ void LIB_VIEW_FRAME::SaveSettings( wxConfigBase* aCfg )
 
 m_cmpListWidth = m_cmpList->GetSize().x;
 aCfg->Write( CMPLIST_WIDTH_KEY, m_cmpListWidth );
-aCfg->Write( LIBVIEW_BGCOLOR, ColorGetName( GetDrawBgColor() ) );
 }
 
 


___
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


[Kicad-developers] [PATCH] Minor tweaks for the eeschema component chooser, especially for aliases.

2014-10-06 Thread John Beard
Hello,

Attached is a small patch that does a few things in the eeschema component  
chooser:

   * Shows aliases' names, rather than the root device names in the 
 preview window
   * Doesn't paint the background if nothing is selected
   * Add the component name as a heading in the description panel
   * Add an "Alias of" field in the description panel, if the component 
 is an alias

For the insertion of the alias name over the root name, I used the same method
as found in viewlibs.cpp:257 and elsewhere, where the name is replaced and then
reverted after drawing.

Thanks,

John
---
 eeschema/dialogs/dialog_choose_component.cpp | 59 +---
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/eeschema/dialogs/dialog_choose_component.cpp 
b/eeschema/dialogs/dialog_choose_component.cpp
index 9352c94..9b00652 100644
--- a/eeschema/dialogs/dialog_choose_component.cpp
+++ b/eeschema/dialogs/dialog_choose_component.cpp
@@ -114,7 +114,7 @@ void DIALOG_CHOOSE_COMPONENT::OnInterceptSearchBoxKey( 
wxKeyEvent& aKeyStroke )
 selectIfValid( GetNextItem( *m_libraryComponentTree, sel ) );
 break;

-// The follwoing keys we can only hijack if they are not needed by the 
textbox itself.
+// The following keys we can only hijack if they are not needed by the 
textbox itself.

 case WXK_LEFT:
 if( m_searchBox->GetInsertionPoint() == 0 )
@@ -219,27 +219,55 @@ bool DIALOG_CHOOSE_COMPONENT::updateSelection()
 wxTextAttr text_attribute;
 text_attribute.SetFont(font_normal);

+const wxString name = selection->GetName();
+
+if ( !name.empty() )
+{
+m_componentDetails->SetDefaultStyle( headline_attribute );
+m_componentDetails->AppendText( name );
+}
+
 const wxString description = selection->GetDescription();

 if( !description.empty() )
 {
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
 m_componentDetails->SetDefaultStyle( headline_attribute );
 m_componentDetails->AppendText( _("Description\n") );
 m_componentDetails->SetDefaultStyle( text_attribute );
 m_componentDetails->AppendText( description );
-m_componentDetails->AppendText( wxT("\n\n") );
 }

 const wxString keywords = selection->GetKeyWords();

 if( !keywords.empty() )
 {
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
 m_componentDetails->SetDefaultStyle( headline_attribute );
 m_componentDetails->AppendText( _("Keywords\n") );
 m_componentDetails->SetDefaultStyle( text_attribute );
 m_componentDetails->AppendText( keywords );
 }

+if ( !selection->IsRoot() )
+{
+LIB_PART* root_part = selection->GetPart();
+const wxString root_component_name( root_part ?
+root_part->GetName() : _("Unknown") );
+
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
+m_componentDetails->SetDefaultStyle( headline_attribute );
+m_componentDetails->AppendText( _("Alias of ") );
+m_componentDetails->SetDefaultStyle( text_attribute );
+m_componentDetails->AppendText( root_component_name );
+}
+
 m_componentDetails->SetInsertionPoint( 0 );  // scroll up.
 m_componentDetails->Thaw();

@@ -250,18 +278,39 @@ bool DIALOG_CHOOSE_COMPONENT::updateSelection()
 void DIALOG_CHOOSE_COMPONENT::OnHandlePreviewRepaint( wxPaintEvent& 
aRepaintEvent )
 {
 int unit = 0;
-LIB_ALIAS* selection = m_search_container->GetSelectedAlias( &unit );
+LIB_ALIAS*  selection = m_search_container->GetSelectedAlias( &unit );
+LIB_PART*   part = selection ? selection->GetPart() : NULL;
+
+// Don't draw anything (not even the background) if we don't have
+// a part to show
+if( !part )
+return;
+
+if( selection->IsRoot() )
+{
+// just show the part directly
+renderPreview( part, unit );
+}
+else
+{
+// switch out the name temporarily for the alias name
+wxString tmp( part->GetName() );
+part->SetName( selection->GetName() );

-renderPreview( selection ? selection->GetPart() : NULL, unit );
+renderPreview( part, unit );
+
+part->SetName( tmp );
+}
 }


 // Render the preview in our m_componentView. If this gets more complicated, 
we should
 // probably have a derived class from wxPanel; but this keeps things local.
-void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART*  aComponent, int 
aUnit )
+void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART* aComponent, int aUnit )
 {
 wxPaintDC dc( m_componentView );
 EDA_COLOR_T bgcolor = m_parent->GetDrawBgColor();
+
 dc.SetBackground( bgcolor == BLACK ? *wxBLACK_BRUSH : *wxWHITE_BRUSH );
 dc.Clear();

--
1.8.3.2


__

Re: [Kicad-developers] [PATCH] Minor tweaks for the eeschema component chooser, especially for aliases.

2014-10-07 Thread John Beard
On 06/10/14 23:44, John Beard wrote:
> Hello,
> 
> Attached is a small patch 

Sorry, that's inlined. Here's the patch as an attachment if that is
preferred.

Cheers,

John


diff --git a/eeschema/dialogs/dialog_choose_component.cpp b/eeschema/dialogs/dialog_choose_component.cpp
index 9352c94..9b00652 100644
--- a/eeschema/dialogs/dialog_choose_component.cpp
+++ b/eeschema/dialogs/dialog_choose_component.cpp
@@ -114,7 +114,7 @@ void DIALOG_CHOOSE_COMPONENT::OnInterceptSearchBoxKey( wxKeyEvent& aKeyStroke )
 selectIfValid( GetNextItem( *m_libraryComponentTree, sel ) );
 break;
 
-// The follwoing keys we can only hijack if they are not needed by the textbox itself.
+// The following keys we can only hijack if they are not needed by the textbox itself.
 
 case WXK_LEFT:
 if( m_searchBox->GetInsertionPoint() == 0 )
@@ -219,27 +219,55 @@ bool DIALOG_CHOOSE_COMPONENT::updateSelection()
 wxTextAttr text_attribute;
 text_attribute.SetFont(font_normal);
 
+const wxString name = selection->GetName();
+
+if ( !name.empty() )
+{
+m_componentDetails->SetDefaultStyle( headline_attribute );
+m_componentDetails->AppendText( name );
+}
+
 const wxString description = selection->GetDescription();
 
 if( !description.empty() )
 {
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
 m_componentDetails->SetDefaultStyle( headline_attribute );
 m_componentDetails->AppendText( _("Description\n") );
 m_componentDetails->SetDefaultStyle( text_attribute );
 m_componentDetails->AppendText( description );
-m_componentDetails->AppendText( wxT("\n\n") );
 }
 
 const wxString keywords = selection->GetKeyWords();
 
 if( !keywords.empty() )
 {
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
 m_componentDetails->SetDefaultStyle( headline_attribute );
 m_componentDetails->AppendText( _("Keywords\n") );
 m_componentDetails->SetDefaultStyle( text_attribute );
 m_componentDetails->AppendText( keywords );
 }
 
+if ( !selection->IsRoot() )
+{
+LIB_PART* root_part = selection->GetPart();
+const wxString root_component_name( root_part ?
+root_part->GetName() : _("Unknown") );
+
+if ( !m_componentDetails->IsEmpty() )
+m_componentDetails->AppendText( wxT("\n\n") );
+
+m_componentDetails->SetDefaultStyle( headline_attribute );
+m_componentDetails->AppendText( _("Alias of ") );
+m_componentDetails->SetDefaultStyle( text_attribute );
+m_componentDetails->AppendText( root_component_name );
+}
+
 m_componentDetails->SetInsertionPoint( 0 );  // scroll up.
 m_componentDetails->Thaw();
 
@@ -250,18 +278,39 @@ bool DIALOG_CHOOSE_COMPONENT::updateSelection()
 void DIALOG_CHOOSE_COMPONENT::OnHandlePreviewRepaint( wxPaintEvent& aRepaintEvent )
 {
 int unit = 0;
-LIB_ALIAS* selection = m_search_container->GetSelectedAlias( &unit );
+LIB_ALIAS*  selection = m_search_container->GetSelectedAlias( &unit );
+LIB_PART*   part = selection ? selection->GetPart() : NULL;
+
+// Don't draw anything (not even the background) if we don't have
+// a part to show
+if( !part )
+return;
+
+if( selection->IsRoot() )
+{
+// just show the part directly
+renderPreview( part, unit );
+}
+else
+{
+// switch out the name temporarily for the alias name
+wxString tmp( part->GetName() );
+part->SetName( selection->GetName() );
 
-renderPreview( selection ? selection->GetPart() : NULL, unit );
+renderPreview( part, unit );
+
+part->SetName( tmp );
+}
 }
 
 
 // Render the preview in our m_componentView. If this gets more complicated, we should
 // probably have a derived class from wxPanel; but this keeps things local.
-void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART*  aComponent, int aUnit )
+void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART* aComponent, int aUnit )
 {
 wxPaintDC dc( m_componentView );
 EDA_COLOR_T bgcolor = m_parent->GetDrawBgColor();
+
 dc.SetBackground( bgcolor == BLACK ? *wxBLACK_BRUSH : *wxWHITE_BRUSH );
 dc.Clear();
 
___
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


[Kicad-developers] [PATCH] Signal handlers for SINGLE_TOP and KICAD

2017-01-11 Thread John Beard
Hi,

None of the KiCad programs appear to implement any signal handlers.
This means that some of them (e.g. pcbnew) don't respond to things
like SIGINT generated by a Ctrl-C in the console its running from
(very annoying when running up and down repeatedly), and others (e.g.
eeschema) terminate immediately, rather than shutting down.

The attached patch implements a graceful shutdown on SIGINT for all of
the SINGLE_TOP programs as well as KICAD.

I have put it in a separate class (static members because the handlers
have to be that way), so that 1) the same handler can be used in both
places and 2) in future if the signal handlers grow, any complexity is
contained.

I have tested this on Linux, and since it's using the standard C++
 stuff, I assume it works (or at least compiles) on all
platforms, but I can't prove it!

Cheers,

John
From f5bb565b873d08675ae63839d229970ae9021113 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 11 Jan 2017 22:48:57 +0800
Subject: [PATCH] Signal handlers for Kicad and Single-Top programs

This allows Kicad programs to be terminated with Ctrl-C (or SIGINT in
general) and still execute the correct shutodwn procedure. Previously,
Pcbnew wouldn't respond and eeschema would terminate immediately.

This is done in a new class with static functions, because C signal
handling only really works with static callbacks. This was done, rather
than just providing a single static function and wxApp pointer in
kicad.cpp and single_top.cpp, because this way allows for more code
reuse, and also hides future complexity if more signals are handled.
---
 common/CMakeLists.txt |  1 +
 common/app_signal_handler.cpp | 56 ++
 common/single_top.cpp |  4 +++
 include/app_signal_handler.h  | 63 +++
 kicad/kicad.cpp   |  6 -
 5 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 common/app_signal_handler.cpp
 create mode 100644 include/app_signal_handler.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 210f7545a..09a5a0a0b 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -195,6 +195,7 @@ set( COMMON_SRCS
 ${COMMON_DLG_SRCS}
 ${COMMON_WIDGET_SRCS}
 ${COMMON_PAGE_LAYOUT_SRCS}
+app_signal_handler.cpp
 base_struct.cpp
 basicframe.cpp
 bezier_curves.cpp
diff --git a/common/app_signal_handler.cpp b/common/app_signal_handler.cpp
new file mode 100644
index 0..b3061d6cd
--- /dev/null
+++ b/common/app_signal_handler.cpp
@@ -0,0 +1,56 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see CHANGELOG.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include 
+
+#include 
+
+
+// Initialise static members
+wxApp* AppSignalHandler::m_app = nullptr;
+
+
+void AppSignalHandler::SetSignalledApp( wxApp* app )
+{
+m_app = app;
+}
+
+
+void AppSignalHandler::RegisterSignalHandler ( int sig,
+  SigHandler handler)
+{
+signal( sig, handler );
+}
+
+
+void AppSignalHandler::FatalSignalHandler( int /*sig*/ )
+{
+if ( m_app )
+{
+m_app->ExitMainLoop();
+}
+else
+{
+exit( 1 );
+}
+}
diff --git a/common/single_top.cpp b/common/single_top.cpp
index 38ed55f89..b47c60787 100644
--- a/common/single_top.cpp
+++ b/common/single_top.cpp
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 // Only a single KIWAY is supported in this single_top top level component,
@@ -110,6 +111,9 @@ struct APP_SINGLE_TOP : public wxApp
 #if defined (__LINUX__)
 APP_SINGLE_TOP(): wxApp()
 {
+// Register for signals
+AppSignalHandler::SetSignalledApp( this );
+AppSignalHandler::RegisterSignalHandler( SIGINT, AppSignalHandler::FatalSignalHandler );
 // Disable proxy menu in Unity window manager. Only usual menubar works with wxWidgets (at least <= 3.1)
 // When the proxy menu menubar is enable, some important things for us do not work: menuite

Re: [Kicad-developers] [PATCH] Signal handlers for SINGLE_TOP and KICAD

2017-01-11 Thread John Beard
And here is the patch with the class name in capitals and a class
comment block. Sorry!

On Wed, Jan 11, 2017 at 11:26 PM, John Beard  wrote:
> Hi,
>
> None of the KiCad programs appear to implement any signal handlers.
> This means that some of them (e.g. pcbnew) don't respond to things
> like SIGINT generated by a Ctrl-C in the console its running from
> (very annoying when running up and down repeatedly), and others (e.g.
> eeschema) terminate immediately, rather than shutting down.
>
> The attached patch implements a graceful shutdown on SIGINT for all of
> the SINGLE_TOP programs as well as KICAD.
>
> I have put it in a separate class (static members because the handlers
> have to be that way), so that 1) the same handler can be used in both
> places and 2) in future if the signal handlers grow, any complexity is
> contained.
>
> I have tested this on Linux, and since it's using the standard C++
>  stuff, I assume it works (or at least compiles) on all
> platforms, but I can't prove it!
>
> Cheers,
>
> John
From 3923fc6e7e4e63c4853da9c906180e88c28ce04d Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 11 Jan 2017 22:48:57 +0800
Subject: [PATCH] Signal handlers for Kicad and Single-Top programs

This allows Kicad programs to be terminated with Ctrl-C (or SIGINT in
general) and still execute the correct shutodwn procedure. Previously,
Pcbnew wouldn't respond and eeschema would terminate immediately.

This is done in a new class with static functions, because C signal
handling only really works with static callbacks. This was done, rather
than just providing a single static function and wxApp pointer in
kicad.cpp and single_top.cpp, because this way allows for more code
reuse, and also hides future complexity if more signals are handled.
---
 common/CMakeLists.txt |  1 +
 common/app_signal_handler.cpp | 56 +++
 common/single_top.cpp |  4 +++
 include/app_signal_handler.h  | 77 +++
 kicad/kicad.cpp   |  6 +++-
 5 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 common/app_signal_handler.cpp
 create mode 100644 include/app_signal_handler.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index ca460d2b4..134a01236 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -180,6 +180,7 @@ set( COMMON_SRCS
 ${COMMON_DLG_SRCS}
 ${COMMON_WIDGET_SRCS}
 ${COMMON_PAGE_LAYOUT_SRCS}
+app_signal_handler.cpp
 base_struct.cpp
 basicframe.cpp
 bezier_curves.cpp
diff --git a/common/app_signal_handler.cpp b/common/app_signal_handler.cpp
new file mode 100644
index 0..bc91a90ad
--- /dev/null
+++ b/common/app_signal_handler.cpp
@@ -0,0 +1,56 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see CHANGELOG.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include 
+
+#include 
+
+
+// Initialise static members
+wxApp* APP_SIGNAL_HANDLER::m_app = nullptr;
+
+
+void APP_SIGNAL_HANDLER::SetSignalledApp( wxApp* app )
+{
+m_app = app;
+}
+
+
+void APP_SIGNAL_HANDLER::RegisterSignalHandler( int sig,
+SigHandler handler )
+{
+signal( sig, handler );
+}
+
+
+void APP_SIGNAL_HANDLER::FatalSignalHandler( int/*sig*/ )
+{
+if( m_app )
+{
+m_app->ExitMainLoop();
+}
+else
+{
+exit( 1 );
+}
+}
diff --git a/common/single_top.cpp b/common/single_top.cpp
index 38ed55f89..8a4ca9eca 100644
--- a/common/single_top.cpp
+++ b/common/single_top.cpp
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 // Only a single KIWAY is supported in this single_top top level component,
@@ -110,6 +111,9 @@ struct APP_SINGLE_TOP : public wxApp
 #if defined (__LINUX__)
 APP_SINGLE_TOP(): wxApp()
 {
+// Register for signals
+APP_SIGNAL_HANDLER::SetSignalledApp( this );
+APP_SIGNAL_HANDLER::RegisterSignalHandler( SIGINT, APP_SIGNAL_HANDL

[Kicad-developers] [PATCH] C++14-style std::make_unique for C++11

2017-01-21 Thread John Beard
Hi,

This is a patch to add std::make_unique to common.h when the C++
standard is C++11 (which it normally is for KiCad).

This simplifies code creating std::unique_ptr's and also means you can
generally say "never use new". It also closes a potential for
exception-unsafety concerning temporary "new"ed objects.

It's cribbed shamelessly from its proposal for C++14
(https://isocpp.org/files/papers/N3656.txt), and that's more or less
exactly how my GCC (6.3.1) implements it too. I've added the GCC
comments in for good measure.

The idea here is that when KiCad one day moves on to C++14, this file
can be removed entirely and forgetten!

Cheers,

John
From 5762fed4ae255612d244f5ab51c88c6b3c31fde4 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sun, 22 Jan 2017 00:03:01 +0800
Subject: [PATCH] Add C++14-style std::make_unique 'polyfill' for C++11

std::make_unique is a very useful part of the new C++ smart pointers
ecosystem, as it allows one to dispense entirely with "new" and also
provides exception safety in some cases when creating temporary
variables.

It also allows more concise code by avoiding repetition of the type:

std::make_unique( args );

vs

std::unique_ptr( new TYPE( args ) );

This commit adds a "polyfill" to provide std::make_unique when C++11 is
enabled. The implementation is that submitted to the C++ committee, and
is essentially how it is done in GCC for C++14.

The intention is to allow KiCad to use this implementation when using
C++11 and when C++14 or greater is a requirement, to remove this code
and use the compiler implementation.
---
 include/common.h  |  2 ++
 include/make_unique.h | 88 +++
 2 files changed, 90 insertions(+)
 create mode 100644 include/make_unique.h

diff --git a/include/common.h b/include/common.h
index 3ea8fad69..64ad39beb 100644
--- a/include/common.h
+++ b/include/common.h
@@ -44,6 +44,8 @@
 
 #include 
 
+// C++11 "polyfill" for the C++14 std::make_unique function
+#include "make_unique.h"
 
 class wxAboutDialogInfo;
 class SEARCH_STACK;
diff --git a/include/make_unique.h b/include/make_unique.h
new file mode 100644
index 0..1ef20658d
--- /dev/null
+++ b/include/make_unique.h
@@ -0,0 +1,88 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+/**
+ * @file make_unique.h
+ * @brief Implementation of std::make_unique for pre C++14 compilation
+ * environments
+ */
+
+#ifndef  MAKE_UNIQUE_H
+#define  MAKE_UNIQUE_H
+
+// Define std::make_unique if the compiler is C++11, but not C++14
+// (which provides this itself)
+// When C++14 support is a KiCad pre-requisite, this entire file
+// can be removed.
+#if __cplusplus == 201103L
+
+#include 
+#include 
+#include 
+#include 
+
+// It's a bit naughty to add things to std::, but the point is to
+// "polyfill" this function in only if std:: doesn't have it
+//
+// This implementation is the one proposed by Stephan T. Lavavej
+// in N3656: https://isocpp.org/files/papers/N3656.txt
+// This is more or less exactly how it is implemented in GCC (e.g. 6.3.1)
+// when C++14 is enabled.
+namespace std
+{
+template struct _Unique_if {
+typedef unique_ptr _Single_object;
+};
+
+template struct _Unique_if {
+typedef unique_ptr _Unknown_bound;
+};
+
+template struct _Unique_if {
+typedef void _Known_bound;
+};
+
+/// std::make_unique for single objects
+template
+typename _Unique_if::_Single_object
+make_unique(Args&&... args) {
+return unique_ptr(new T(std::forward(args)...));
+}
+
+/// std::make_unique for arrays of unknown bound
+template
+typename _Unique_if::_Unknown_bound
+make_unique(size_t n) {
+typedef typename remove_extent::type U;
+return unique_ptr(new U[n]());
+}

[Kicad-developers] [PATCH] Tidy consts in D_PAD

2017-01-21 Thread John Beard
Hi,

There are a few accessors in D_PAD that could do with const qualifiers.

Also a few functions return non-POD objects by const value. This
doesn't actually acheive anything, except forcing the compiler to
forgo move operations and use copies instead, which is just less
efficient. It doesn't actually convey any meaning about the data
itself (the calling code is the one to decide if it wants to call its
newly copied/moved object "const" or not). If the functions returned
const ref or const pointer, then it would have meaning.

I didn't change them all, as some are overrides that are part of a
larger inheriance hierarchy.

Cheers,

John
From 4f726e83fb44f5057a28b05777bfef5613b3fbf9 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sun, 22 Jan 2017 04:19:46 +0800
Subject: [PATCH 1/2] Tidy consts for class D_PAD

Some accessors should be const:

* IsFlipped
* GetRoundRectRadiusRatio

Returning a objects by value as const in these cases is not helpful, as
all it does is prevent the caller moving from the return value, it just
forces a copy.

Some of thse functions come from base class overrides, those haven't
been changed.

* ShapePos
* GetPadName
* GetPackedPadName
---
 pcbnew/class_pad.cpp |  6 +++---
 pcbnew/class_pad.h   | 10 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/pcbnew/class_pad.cpp b/pcbnew/class_pad.cpp
index cab53dbda..d4822f71f 100644
--- a/pcbnew/class_pad.cpp
+++ b/pcbnew/class_pad.cpp
@@ -120,7 +120,7 @@ LSET D_PAD::UnplatedHoleMask()
 return saved;
 }
 
-bool D_PAD::IsFlipped()
+bool D_PAD::IsFlipped() const
 {
 if( GetParent() &&  GetParent()->GetLayer() == B_Cu )
 return true;
@@ -363,7 +363,7 @@ void D_PAD::AppendConfigs( PARAM_CFG_ARRAY* aResult )
 
 
 // Returns the position of the pad.
-const wxPoint D_PAD::ShapePos() const
+wxPoint D_PAD::ShapePos() const
 {
 if( m_Offset.x == 0 && m_Offset.y == 0 )
 return m_Pos;
@@ -378,7 +378,7 @@ const wxPoint D_PAD::ShapePos() const
 }
 
 
-const wxString D_PAD::GetPadName() const
+wxString D_PAD::GetPadName() const
 {
 wxString name;
 
diff --git a/pcbnew/class_pad.h b/pcbnew/class_pad.h
index c4689630f..6072fec05 100644
--- a/pcbnew/class_pad.h
+++ b/pcbnew/class_pad.h
@@ -111,7 +111,7 @@ public:
  * @return true if the pad has a footprint parent flipped
  * (on the back/bottom layer)
  */
-bool IsFlipped();
+bool IsFlipped() const;
 
 /**
  * Set the pad name (sometimes called pad number, although
@@ -124,7 +124,7 @@ public:
  * @return the pad name
  * the pad name is limited to 4 ASCII chars
  */
-const wxString GetPadName() const;
+wxString GetPadName() const;
 
 /**
  * @return the pad name in a wxUint32 which is possible
@@ -132,7 +132,7 @@ public:
  * The packed pad name should be used only to compare 2
  * pad names, not to try to print this name
  */
-const wxUint32 GetPackedPadName() const { return m_NumPadName; }
+wxUint32 GetPackedPadName() const { return m_NumPadName; }
 
 /**
  * Function IncrementPadName
@@ -430,7 +430,7 @@ public:
 return m_boundingRadius;
 }
 
-const wxPoint ShapePos() const;
+wxPoint ShapePos() const;
 
 /**
  * has meaning only for rounded rect pads
@@ -439,7 +439,7 @@ public:
  * Cannot be > 0.5
  * the normalized IPC-7351C value is 0.25
  */
-double GetRoundRectRadiusRatio()
+double GetRoundRectRadiusRatio() const
 {
 return m_padRoundRectRadiusScale;
 }
-- 
2.11.0

___
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


[Kicad-developers] Fwd: [PATCH] C++14-style std::make_unique for C++11

2017-01-22 Thread John Beard
FWD to list

-- Forwarded message --
From: John Beard 
Date: Mon, Jan 23, 2017 at 8:36 AM
Subject: Re: [Kicad-developers] [PATCH] C++14-style std::make_unique for C++11
To: Wayne Stambaugh 


The idea with std::make_unique is essentially that when making
std::unique_ptrs, you never have to use "new" and it also makes the
code much clearer when doing so because you only need to write the
type once. Both of these aspects encourage more "modern" styles of
memory management.

I'd anticipate that make_unique would (should?) be used every time an
object is constructed into a unique_ptr, so it would save a lot of
work to do it now than wait until C++14 and change.

I have added a warning in the header for C++14. An error would cause
breakage immediately on C++14, when actually it would work, just with
an unused header. A warning will prompt for removal, but would allow
people to upgrade their own compiler standard (e.g. someone trying out
C++14/17) without having to patch the #error out.

On Mon, Jan 23, 2017 at 6:49 AM, Wayne Stambaugh  wrote:
> I'm not opposed but I agree with your assessment that it needs to not
> build when c++14 is defined.  Is this something that devs are going to
> use or is something we can push off until we are actually a c++14 project?
>
> On 1/22/2017 10:51 AM, Chris Pavlina wrote:
>> After reading about the application of std::make_unique, I like it; I am
>> not opposed to merging this. I would, however, be in favor of modifying
>> it slightly to emit a warning or error if built under C++14, so that it
>> is not forgotten if and when we do make the transition to C++14 in the
>> future.
>>
>> Is anybody else for or against this?
>>
>> On Sun, Jan 22, 2017 at 12:38:07AM +0800, John Beard wrote:
>>> Hi,
>>>
>>> This is a patch to add std::make_unique to common.h when the C++
>>> standard is C++11 (which it normally is for KiCad).
>>>
>>> This simplifies code creating std::unique_ptr's and also means you can
>>> generally say "never use new". It also closes a potential for
>>> exception-unsafety concerning temporary "new"ed objects.
>>>
>>> It's cribbed shamelessly from its proposal for C++14
>>> (https://isocpp.org/files/papers/N3656.txt), and that's more or less
>>> exactly how my GCC (6.3.1) implements it too. I've added the GCC
>>> comments in for good measure.
>>>
>>> The idea here is that when KiCad one day moves on to C++14, this file
>>> can be removed entirely and forgetten!
>>>
>>> Cheers,
>>>
>>> John
>>
>>
>> ___
>> 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
From 2a75bab89393e607ea26ef796da3ec997da7fb82 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sun, 22 Jan 2017 00:03:01 +0800
Subject: [PATCH 1/4] Add C++14-style std::make_unique 'polyfill' for C++11

std::make_unique is a very useful part of the new C++ smart pointers
ecosystem, as it allows one to dispense entirely with "new" and also
provides exception safety in some cases when creating temporary
variables.

It also allows more concise code by avoiding repetition of the type:

std::make_unique( args );

vs

std::unique_ptr( new TYPE( args ) );

This commit adds a "polyfill" to provide std::make_unique when C++11 is
enabled. The implementation is that submitted to the C++ committee, and
is essentially how it is done in GCC for C++14.

The intention is to allow KiCad to use this implementation when using
C++11 and when C++14 or greater is a requirement, to remove this code
and use the compiler implementation.
---
 include/common.h  |  2 ++
 include/make_unique.h | 93 +++
 2 files changed, 95 insertions(+)
 create mode 100644 include/make_unique.h

diff --git a/include/common.h b/include/common.h
index 3ea8fad69..64ad39beb 100644
--- a/include/common.h
+++ b/include/common.h
@@ -44,6 +44,8 @@
 
 #include 
 
+// C++11 "polyfill" for the C++14 std::make_unique function
+#include "make_unique.h"
 
 class wxAboutDialogInfo;
 class SEARCH_STACK;
diff --git a/include/make_unique.h b/include/make_unique.h
new file mode 100644
index 0

[Kicad-developers] [PATCH] Move PostCommandMenuEvent to EDA_BASE_FRAME

2017-01-22 Thread John Beard
Hi,

This is a small patch to move PostCommandMenuEvent up to
EDA_BASE_FRAME, from PCB_BASE_EDIT_FRAME.

This function has nothing intristic to PCB edit frames, it would apply
to any frame. I'd like to use it for code that works in both Pcbnew
and Cvpcb, which have EDA_BASE_FRAME as the first suitable common
ancestor (KIWAY_PLAYER is first, but that's not really suitable for
menu event code).

I'm submitting this ahead of the code that uses it, because it's
logically separate and stands alone, and it touches common headers: if
the make_unique patch goes in alongside, this will avoid a
double-recompile for anyone pulling both at once!

Cheers,

John
From f8c76db985a90fbf5e209317e04011028b95e4de Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 17 Jan 2017 13:57:53 +0800
Subject: [PATCH] Move PostCommandMenuEvent to EDA_BASE_FRAME

There is nothing PCB-frame-specific about this function,
it is equally applicable to an frame, for example
PCB_EDIT_FRAME and CVPCB_MAINFRAME, which have EDA_BASE_FRAME
as the nearest common ancestor, except KIWAY_PLAYER,
which is not really concerned with this kind of UI event method.
---
 common/basicframe.cpp  | 15 +++
 include/wxstruct.h |  9 +
 pcbnew/pcb_base_edit_frame.cpp | 15 ---
 pcbnew/pcb_base_edit_frame.h   |  2 --
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/common/basicframe.cpp b/common/basicframe.cpp
index 45ad2f4d8..417bbc6ed 100644
--- a/common/basicframe.cpp
+++ b/common/basicframe.cpp
@@ -631,3 +631,18 @@ void EDA_BASE_FRAME::CheckForAutoSaveFile( const wxFileName& aFileName,
 wxRemoveFile( autoSaveFileName.GetFullPath() );
 }
 }
+
+
+bool EDA_BASE_FRAME::PostCommandMenuEvent( int evt_type )
+{
+if( evt_type != 0 )
+{
+wxCommandEvent evt( wxEVT_COMMAND_MENU_SELECTED );
+evt.SetEventObject( this );
+evt.SetId( evt_type );
+wxPostEvent( this, evt );
+return true;
+}
+
+return false;
+}
diff --git a/include/wxstruct.h b/include/wxstruct.h
index 1292150b4..35d4af924 100644
--- a/include/wxstruct.h
+++ b/include/wxstruct.h
@@ -406,6 +406,15 @@ public:
  * redraws the menus and what not in current language.
  */
 virtual void ShowChangedLanguage();
+
+
+/**
+ * Function PostCommandMenuEvent
+ *
+ * Post a menu event to the frame, which can be used to trigger actions
+ * bound to menu items.
+ */
+bool PostCommandMenuEvent( int evt_type );
 };
 
 
diff --git a/pcbnew/pcb_base_edit_frame.cpp b/pcbnew/pcb_base_edit_frame.cpp
index 3383c5388..2580985b9 100644
--- a/pcbnew/pcb_base_edit_frame.cpp
+++ b/pcbnew/pcb_base_edit_frame.cpp
@@ -39,21 +39,6 @@ void PCB_BASE_EDIT_FRAME::SetRotationAngle( int aRotationAngle )
 }
 
 
-bool PCB_BASE_EDIT_FRAME::PostCommandMenuEvent( int evt_type )
-{
-if( evt_type != 0 )
-{
-wxCommandEvent evt( wxEVT_COMMAND_MENU_SELECTED );
-evt.SetEventObject( this );
-evt.SetId( evt_type );
-wxPostEvent( this, evt );
-return true;
-}
-
-return false;
-}
-
-
 void PCB_BASE_EDIT_FRAME::UseGalCanvas( bool aEnable )
 {
 PCB_BASE_FRAME::UseGalCanvas( aEnable );
diff --git a/pcbnew/pcb_base_edit_frame.h b/pcbnew/pcb_base_edit_frame.h
index db6f31066..ef68feebd 100644
--- a/pcbnew/pcb_base_edit_frame.h
+++ b/pcbnew/pcb_base_edit_frame.h
@@ -156,8 +156,6 @@ public:
  */
 void SetRotationAngle( int aRotationAngle );
 
-bool PostCommandMenuEvent( int evt_type );
-
 ///> @copydoc EDA_DRAW_FRAME::UseGalCanvas()
 void UseGalCanvas( bool aEnable ) override;
 
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Allow editing locked modules in modedit

2017-01-22 Thread John Beard
This patch makes it possible to edit locked modules in the module editor.

Since you have to explicitly launch modedit to be able to change the
module, and there is no change to pcbnew behaviour, editing locked
modules still required explicit effort to do so (just like moving
locked modules).

Fixes: LP 1591625

https://bugs.launchpad.net/kicad/+bug/1591625
From 317510c194c8e8c3829abb4cdabbeab773b087bc Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sun, 22 Jan 2017 01:32:45 +0800
Subject: [PATCH] Allow editing locked modules in modedit

Since you have to explicitly enter the module editor with the menu or
hotkey, allowing editing of module sub-parts once in should not cause
any unexpected changes.

Fixes: lp:1591625
* https://bugs.launchpad.net/kicad/+bug/1591625
---
 pcbnew/tools/selection_tool.cpp | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index ce40ae634..a2220f856 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -867,12 +867,23 @@ bool SELECTION_TOOL::selectable( const BOARD_ITEM* aItem ) const
 case PCB_MODULE_EDGE_T:
 case PCB_PAD_T:
 {
+// Multiple selection is only allowed in modedit mode
+// In pcbnew, you have to select subparts of modules
+// one-by-one, rather than with a drag selection.
+// This is so you can pick up items under an (unlocked)
+// module without also moving the module's sub-parts.
 if( m_multiple && !m_editModules )
 return false;
 
-MODULE* mod = static_cast( aItem )->GetParent();
-if( mod && mod->IsLocked() )
-return false;
+// When editing modules, it's allowed to select them, even when
+// locked, since you already have to explicitly activate the
+// module editor to get to this stage
+if ( !m_editModules )
+{
+MODULE* mod = static_cast( aItem )->GetParent();
+if( mod && mod->IsLocked() )
+return false;
+}
 
 break;
 }
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Init OPENGL_GAL currentTarget

2017-01-22 Thread John Beard
Hi,

This patch initialises a class member in OPENGL_GAL. It uses the
SetTarget function to make  sure the target and manager initialise in
a consistent way.

Cheers,

John
From f9a0fc626f78337165595f18f4dca73afa11a9af Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Mon, 23 Jan 2017 13:19:10 +0800
Subject: [PATCH] OPENGL_GAL: Init currentTarget

The currentTarget member of OPENGL_GAL is not initialised, but the
currentManager member is.

This commit uses SetTarget to initialise both target and manager to a
consistent state at construction.
---
 common/gal/opengl/opengl_gal.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp
index 4e64d0e04..862a783da 100644
--- a/common/gal/opengl/opengl_gal.cpp
+++ b/common/gal/opengl/opengl_gal.cpp
@@ -139,7 +139,7 @@ OPENGL_GAL::OPENGL_GAL( GAL_DISPLAY_OPTIONS& aDisplayOptions, wxWindow* aParent,
 
 gluTessProperty( tesselator, GLU_TESS_WINDING_RULE, GLU_TESS_WINDING_POSITIVE );
 
-currentManager = nonCachedManager;
+SetTarget( TARGET_NONCACHED );
 }
 
 
-- 
2.11.0

___
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


[Kicad-developers] Pad import/export/push and modedit mirror

2017-01-24 Thread John Beard
Hi,

Here are two patches for outstanding GAL functionality: pad
import/export/push and modedit mirroring.

Pad "push" is what I termed global edit, since I think that describes
the action of taking a reference and "pushing" it to a filtered subset
of other pads. I am happy to keep the old name too, if that's
preferred.

I have implemented in a new PAD_TOOL GAL tool as I think there could
be more pad-only function in future that can all stick together in
there.

Let me know if there are any suggestions!

John
From a37f96d130a5fbd86e662360e02159aad5e1ef54 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Mon, 23 Jan 2017 14:47:39 +0800
Subject: [PATCH 2/2] Add mirror tool action for GAL module editor

This is basically a simple clone of the legacy tool with a few minor
tidy-ups for GAL mode.

Fixes: lp:1619301
* https://bugs.launchpad.net/kicad/+bug/1619301
---
 pcbnew/tools/common_actions.cpp |   4 ++
 pcbnew/tools/common_actions.h   |   3 ++
 pcbnew/tools/edit_tool.cpp  | 117 
 pcbnew/tools/edit_tool.h|   7 +++
 4 files changed, 131 insertions(+)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index dc65bf6e3..da2694db5 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -125,6 +125,10 @@ TOOL_ACTION COMMON_ACTIONS::flip( "pcbnew.InteractiveEdit.flip",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_FLIP_ITEM ),
 _( "Flip" ), _( "Flips selected item(s)" ), swap_layer_xpm );
 
+TOOL_ACTION COMMON_ACTIONS::mirror( "pcbnew.InteractiveEdit.mirror",
+AS_GLOBAL, 0,
+_( "Mirror" ), _( "Mirrors selected item" ), mirror_h_xpm );
+
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
 _( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm );
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index 2cf312ad3..a878520da 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -76,6 +76,9 @@ public:
 /// Flipping of selected objects
 static TOOL_ACTION flip;
 
+/// Mirroring of selected items
+static TOOL_ACTION mirror;
+
 /// Activation of the edit tool
 static TOOL_ACTION properties;
 
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 7c831774b..1c9466fb8 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -86,6 +86,10 @@ bool EDIT_TOOL::Init()
 return false;
 }
 
+auto editingModuleCondition = [ this ] ( const SELECTION& aSelection ) {
+return m_editModules;
+};
+
 // Add context menu entries that are displayed when selection tool is active
 CONDITIONAL_MENU& menu = m_selectionTool->GetToolMenu().GetMenu();
 menu.AddItem( COMMON_ACTIONS::editActivate, SELECTION_CONDITIONS::NotEmpty );
@@ -98,6 +102,9 @@ bool EDIT_TOOL::Init()
 menu.AddItem( COMMON_ACTIONS::duplicate, SELECTION_CONDITIONS::NotEmpty );
 menu.AddItem( COMMON_ACTIONS::createArray, SELECTION_CONDITIONS::NotEmpty );
 
+// Mirror only available in modedit
+menu.AddItem( COMMON_ACTIONS::mirror, editingModuleCondition && SELECTION_CONDITIONS::NotEmpty );
+
 // Footprint actions
 menu.AddItem( COMMON_ACTIONS::editFootprintInFpEditor,
 SELECTION_CONDITIONS::OnlyType( PCB_MODULE_T ) &&
@@ -404,6 +411,115 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
 return 0;
 }
 
+/*!
+ * Mirror a point about the vertical axis passing though another point
+ */
+static wxPoint mirrorPointX( const wxPoint& aPoint, const wxPoint& aMirrorPoint )
+{
+wxPoint mirrored = aPoint;
+
+mirrored.x -= aMirrorPoint.x;
+mirrored.x = -mirrored.x;
+mirrored.x += aMirrorPoint.x;
+
+return mirrored;
+}
+
+
+/**
+ * Mirror a pad in the vertical axis passing though a point
+ */
+static void mirrorPadX( D_PAD& aPad, const wxPoint& aMirrorPoint )
+{
+wxPoint tmpPt = mirrorPointX( aPad.GetPosition(), aMirrorPoint );
+
+aPad.SetPosition( tmpPt );
+
+aPad.SetX0( aPad.GetPosition().x );
+
+tmpPt = aPad.GetOffset();
+tmpPt.x = -tmpPt.x;
+aPad.SetOffset( tmpPt );
+
+auto tmpz = aPad.GetDelta();
+tmpz.x = -tmpz.x;
+aPad.SetDelta( tmpz );
+
+aPad.SetOrientation( -aPad.GetOrientation() );
+}
+
+
+int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
+{
+const SELECTION& selection = m_selectionTool->GetSelection();
+PCB_BASE_EDIT_FRAME* editFrame = getEditFrame();
+
+// Shall the selection be cleared at the end?
+bool unselect = selection.Empty();
+
+if( !hoverSelection() || m_selectionTool->CheckLock() == SELECTION_LOCKED )
+return 0;
+
+wxPoint mirrorPoint = getModificationPoint

Re: [Kicad-developers] Pad import/export/push and modedit mirror

2017-01-24 Thread John Beard
Sorry, attached the wrong version of the "import export" patch - this
is the correct sequence.

On Wed, Jan 25, 2017 at 12:41 AM, John Beard  wrote:
> Hi,
>
> Here are two patches for outstanding GAL functionality: pad
> import/export/push and modedit mirroring.
>
> Pad "push" is what I termed global edit, since I think that describes
> the action of taking a reference and "pushing" it to a filtered subset
> of other pads. I am happy to keep the old name too, if that's
> preferred.
>
> I have implemented in a new PAD_TOOL GAL tool as I think there could
> be more pad-only function in future that can all stick together in
> there.
>
> Let me know if there are any suggestions!
>
> John
From e170571678476336da2f4074aa7b3d58e88bdbc3 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sun, 22 Jan 2017 05:06:18 +0800
Subject: [PATCH 1/2] Add pad import/export/push to GAL canvas

This implements the pad import/export to the board's master pad setting
in the GAL canvases.

Implemented as a new GAL tool: PAD_TOOL.

It uses the same dialog, which has been split out into its own files in
pcbnew/dialogs, rather than along with frame methods in
pcbnew/globaleditpad.cpp.

Fixes: lp:1619304
* https://bugs.launchpad.net/kicad/+bug/1619304
---
 pcbnew/CMakeLists.txt |   2 +
 pcbnew/dialogs/dialog_global_pads_edition.cpp |  91 ++
 pcbnew/dialogs/dialog_global_pads_edition.h   |  56 
 pcbnew/globaleditpad.cpp  |  91 +-
 pcbnew/moduleframe.cpp|   3 +
 pcbnew/tools/common_actions.cpp   |  18 ++
 pcbnew/tools/common_actions.h |  10 +
 pcbnew/tools/pad_tool.cpp | 395 ++
 pcbnew/tools/pad_tool.h   |  64 +
 pcbnew/tools/tools_common.cpp |   3 +
 10 files changed, 643 insertions(+), 90 deletions(-)
 create mode 100644 pcbnew/dialogs/dialog_global_pads_edition.cpp
 create mode 100644 pcbnew/dialogs/dialog_global_pads_edition.h
 create mode 100644 pcbnew/tools/pad_tool.cpp
 create mode 100644 pcbnew/tools/pad_tool.h

diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt
index f6f702353..4b10ca678 100644
--- a/pcbnew/CMakeLists.txt
+++ b/pcbnew/CMakeLists.txt
@@ -87,6 +87,7 @@ set( PCBNEW_DIALOGS
 dialogs/dialog_global_modules_fields_edition.cpp
 dialogs/dialog_global_modules_fields_edition_base.cpp
 dialogs/dialog_global_pads_edition_base.cpp
+dialogs/dialog_global_pads_edition.cpp
 dialogs/dialog_graphic_items_options.cpp
 dialogs/dialog_graphic_items_options_base.cpp
 dialogs/dialog_graphic_item_properties.cpp
@@ -292,6 +293,7 @@ set( PCBNEW_CLASS_SRCS
 tools/placement_tool.cpp
 tools/common_actions.cpp
 tools/grid_helper.cpp
+tools/pad_tool.cpp
 tools/picker_tool.cpp
 tools/zoom_tool.cpp
 tools/tools_common.cpp
diff --git a/pcbnew/dialogs/dialog_global_pads_edition.cpp b/pcbnew/dialogs/dialog_global_pads_edition.cpp
new file mode 100644
index 0..c2c449b5d
--- /dev/null
+++ b/pcbnew/dialogs/dialog_global_pads_edition.cpp
@@ -0,0 +1,91 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+
+#include 
+
+#include 
+#include 
+
+
+DIALOG_GLOBAL_PADS_EDITION::DIALOG_GLOBAL_PADS_EDITION( PCB_BASE_FRAME* aParent, D_PAD* aPad ) :
+DIALOG_GLOBAL_PADS_EDITION_BASE( aParent )
+{
+m_parent = aParent;
+m_currentPad = aPad;
+
+// Pad filter selection.
+m_Pad_Shape_Filter_CB->SetValue( m_Pad_Shape_Filter );
+m_Pad_Layer_Filter_CB->SetValue( m_Pad_Layer_Filter );
+m_Pad_Orient_Filter_CB->SetValue( m_Pad_Orient_Filter );
+
+SetFocus();
+
+GetSizer()->Fit( this );
+Centre();
+}
+
+
+// Class DIALOG_GLOBAL_PADS_EDITION static variables
+bool DIALOG_GLOBAL_PADS_EDITION::m_Pad_Shape_Filter  = true;
+bool DIALOG_GLOBAL_PADS_EDITI

[Kicad-developers] [PATCH] Consts in BOARD

2017-01-27 Thread John Beard
Here is a patch for a couple of consts in class BOARD, plus a
suspicious const return-by-value (which does nothing but disable move
operations).

I looked at GetBoardPolygonOutlines too, but that needs the
GetBoardPolygonOutlines function in specctra.cpp to be constified, and
theres (at least) a member, "pcb", that gets conditionally initialised
in that function, so it's impossible without a mutable, which I'm not
sure would be correct or not.

Cheers,

John
From 5d745e925631107ec0bd462907955345cacae2fd Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 27 Jan 2017 23:32:44 +0800
Subject: [PATCH 1/2] Tidy consts in class BOARD

Add some consts to "getters" in this class.

Remove const from return type of static return-by-value getter - this
has no effect on the caller except disabling move operations and forcing
copy operations.
---
 pcbnew/class_board.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pcbnew/class_board.h b/pcbnew/class_board.h
index 0ad5e9d89..e43fa5aae 100644
--- a/pcbnew/class_board.h
+++ b/pcbnew/class_board.h
@@ -353,7 +353,7 @@ public:
  * Function GetHighLightNetCode
  * @return netcode of net to highlight (-1 when no net selected)
  */
-int GetHighLightNetCode() { return m_highLight.m_netCode; }
+int GetHighLightNetCode() const { return m_highLight.m_netCode; }
 
 /**
  * Function SetHighLightNet
@@ -368,7 +368,7 @@ public:
  * Function IsHighLightNetON
  * @return true if a net is currently highlighted
  */
-bool IsHighLightNetON() { return m_highLight.m_highLightOn; }
+bool IsHighLightNetON() const { return m_highLight.m_highLightOn; }
 
 /**
  * Function HighLightOFF
@@ -645,7 +645,7 @@ public:
  * @return const wxString - containing the layer name or "BAD INDEX" if aLayerId
  *  is not legal
  */
-static const wxString GetStandardLayerName( LAYER_ID aLayerId )
+static wxString GetStandardLayerName( LAYER_ID aLayerId )
 {
 // a BOARD's standard layer name is the LAYER_ID fixed name
 return LSET::Name( aLayerId );
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Pre-select highlighted net in zone create dialog

2017-01-28 Thread John Beard
Here's a patch for pre-selecting the currently highlighted net in the
zone create dialog.

This fixes https://bugs.launchpad.net/kicad/+bug/1482866 (the last 14*
bug with the missing-gal-tool tag!).

The actual behaviour, as mentioned in the LP bug thread, is a bit hard
to discover, but that's pre-existing and a different issue.

Due to the use of a const-reference BOARD&, this patch relies on
yesterday's "Consts in BOARD" patch.

Cheers,

John
From 97cdca03faefe694bc49297db40ba000ca0025f8 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 27 Jan 2017 23:44:32 +0800
Subject: [PATCH 2/2] Pre-select highlighted net in zone create dialog

Applies in GAL mode (legacy mode already did this).

Fixes: lp:1482866
* https://bugs.launchpad.net/kicad/+bug/1482866
---
 pcbnew/tools/drawing_tool.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp
index d2b8ba7ae..c0b897361 100644
--- a/pcbnew/tools/drawing_tool.cpp
+++ b/pcbnew/tools/drawing_tool.cpp
@@ -1286,9 +1286,12 @@ int DRAWING_TOOL::drawZone( bool aKeepout )
 {
 if( numPoints == 0 )// it's the first click
 {
+const auto& board = *getModel();
+
 // Get the current default settings for zones
 ZONE_SETTINGS zoneInfo = m_frame->GetZoneSettings();
 zoneInfo.m_CurrentZone_Layer = m_frame->GetScreen()->m_Active_Layer;
+zoneInfo.m_NetcodeSelection = board.GetHighLightNetCode();
 zoneInfo.SetIsKeepout( aKeepout );
 
 m_controls->SetAutoPan( true );
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Add exchange footprints to GAL

2017-01-31 Thread John Beard
Hi,

The attached patches add the "exchange footprints" command to the GAL edit tool.

This is 1 out of 3 parts of
https://bugs.launchpad.net/kicad/+bug/1541460. I'm not sure how to put
this as a commit comment exactly.

This is implemented as two segments:

* Put the dialog/exchange FP code into pcbnew/dialogs directory (as a
header/implementation pair) where it can be reached by bother
pcbframe.cpp and GAL tools. This also makes it easier to remove legacy
calling code in future. No change is made to the functionality of the
dialog.
* Add the tool to the GAL, calling the dialog as needed.

The third patch is optional, but tidies up a copy-pasted selection
routine into a named function and adds a little commentary to a couple
of related existing functions.

Cheers,

John
From fd30b0465e0b627a4f69fa574f077396ea3f7861 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 31 Jan 2017 17:54:54 +0800
Subject: [PATCH 3/3] Factor an EDIT_TOOL selection routine

Also add some commentary to other EDIT_TOOL selection functions.
---
 pcbnew/tools/edit_tool.cpp | 10 ++---
 pcbnew/tools/edit_tool.h   | 51 +-
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index d2af32568..651ea5bab 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -783,10 +783,7 @@ int EDIT_TOOL::CreateArray( const TOOL_EVENT& aEvent )
 
 int EDIT_TOOL::ExchangeFootprints( const TOOL_EVENT& aEvent )
 {
-if( !hoverSelection() )
-return 0;
-
-MODULE* mod = uniqueSelected();
+MODULE* mod = uniqueHoverSelection();
 
 if( !mod )
 return 0;
@@ -895,10 +892,7 @@ int EDIT_TOOL::editFootprintInFpEditor( const TOOL_EVENT& aEvent )
 SELECTION& selection = m_selectionTool->GetSelection();
 bool unselect = selection.Empty();
 
-if( !hoverSelection() )
-return 0;
-
-MODULE* mod = uniqueSelected();
+MODULE* mod = uniqueHoverSelection();
 
 if( !mod )
 return 0;
diff --git a/pcbnew/tools/edit_tool.h b/pcbnew/tools/edit_tool.h
index 83f2b493e..2cf158636 100644
--- a/pcbnew/tools/edit_tool.h
+++ b/pcbnew/tools/edit_tool.h
@@ -151,15 +151,34 @@ private:
 ///> selected items.
 wxPoint getModificationPoint( const SELECTION& aSelection );
 
-///> If there are no items currently selected, it tries to choose the item that is under
-///> the cursor or displays a disambiguation menu if there are multiple items.
-bool hoverSelection( bool aSanitize = true );
-
 int editFootprintInFpEditor( const TOOL_EVENT& aEvent );
 
 bool invokeInlineRouter();
 
-template T* uniqueSelected()
+/**
+ * Function hoverSelection()
+ *
+ * If there are no items currently selected, it tries to choose the
+ * item that is under he cursor or displays a disambiguation menu
+ * if there are multiple items.
+ *
+ * @param aSanitize sanitize selection using SanitizeSelection()
+ * @return true if the eventual selection contains any items, or
+ * false if it fails to select any items.
+ */
+bool hoverSelection( bool aSanitize = true );
+
+/**
+ * Function uniqueSelected()
+ *
+ * Get a single selected item of a certain type
+ *
+ * @tparam T type of item to select
+ * @return pointer to the item (of type T), or nullptr if there isn't
+ * a single selected item, or it's not of the right type.
+ */
+template
+T* uniqueSelected()
 {
 const SELECTION& selection = m_selectionTool->GetSelection();
 
@@ -170,6 +189,28 @@ private:
 return dyn_cast( item );
 }
 
+/**
+ * Function uniqueHoverSelection()
+ *
+ * Get a single unique selection of an item, either from the
+ * current selection, or from the items under cursor via
+ * hoverSelection()
+ *
+ * @tparam T type of item to select
+ * @return pointer to a selected item, or nullptr if none could
+ * be found.
+ */
+template
+T* uniqueHoverSelection( bool aSanitize = true )
+{
+if( !hoverSelection( aSanitize ) )
+return nullptr;
+
+T* item = uniqueSelected();
+
+return item;
+}
+
 std::unique_ptr m_commit;
 };
 
-- 
2.11.0

From 60ef3d4789c8abcf8e8808a236c7f661f49d0de3 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 31 Jan 2017 16:03:07 +0800
Subject: [PATCH 2/3] Add exchange modules GAL tool (in EDIT_TOOL)

Includes tool action and the transistion binding in EDIT_TOOL.

Partial fix for lp:1541460 (1 of 3 items)
* https://bugs.launchpad.net/kicad/+bug/1541460
---
 pcbnew/tools/common_actions.cpp |  5 +
 pcbnew/tools/common_actions.h   |  3 +++
 pcbnew/tools/edit_tool.cpp  | 42 +++--
 pcbnew/tools/edit_tool.h|  8 
 4 files changed, 56 insertions(+), 2 delet

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

2017-02-04 Thread 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: https://launchpad.net/~kicad-developers
Post to  

Re: [Kicad-developers] Pad import/export/push and modedit mirror

2017-02-05 Thread John Beard
On Sat, Feb 4, 2017 at 8:10 PM, Nick Østergaard  wrote:
> I have attached a patch that renames this. It contains three commits
> for easier review.

Sounds good to me.

At the (considerable) risk of bikeshedding, I might also suggest
something like "stash" or "store" instead of "copy", since the user
doesn't really "get" a copy of the settings as such, but I also think
what you have here is fine, and better than import/export, since there
is no "external" destination like a file that the user is aware of.

Cheers,

John

___
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


[Kicad-developers] [PATCH] CCW Rotation in GAL

2017-02-05 Thread John Beard
Hi,

Here is a patch for adding CCW rotation to the GAL canvas.

Possibly controversial is the new namespace in tool_action_utils.h for
dumping generic free function helpers that act on TOOL_ACTIONS and on
other classes in concert. I feel that this is a way to isolate the
these routines, giving access to whichever GAL tool functions needed
them, while emphasising that they act via public interfaces of the
relevant classes, but I defer to the GAL wizards for comment!

Fixes: https://bugs.launchpad.net/kicad/+bug/1660731 (but it's more
extensive than just that, as it adds bidirectional rotation to
everywhere there is unidirectional rotation in GAL now).

Cheers,

John
From 77bc4b3b42d64137d915aecdfef0139150c0844b Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sat, 4 Feb 2017 13:09:50 +0800
Subject: [PATCH] Add CCW rotation to GAL canvas

This makes "rotate" into two separate TOOL_EVENTs, which each have a
"multiplier" parameter.

Also added is a namespace for 'free functions' that use TOOL_EVENT
public interfaces (perhaps with other inputs too) to centralise some
decision-making and calculations.

Fixes: lp:1660731
* https://bugs.launchpad.net/kicad/+bug/1660731
---
 pcbnew/CMakeLists.txt|  1 +
 pcbnew/tools/common_actions.cpp  | 10 --
 pcbnew/tools/common_actions.h|  7 ++--
 pcbnew/tools/drawing_tool.cpp| 21 +++
 pcbnew/tools/edit_tool.cpp   | 11 --
 pcbnew/tools/module_editor_tools.cpp | 14 +---
 pcbnew/tools/pcb_editor_control.cpp  |  8 +++--
 pcbnew/tools/tool_event_utils.cpp| 48 +
 pcbnew/tools/tool_event_utils.h  | 68 
 9 files changed, 168 insertions(+), 20 deletions(-)
 create mode 100644 pcbnew/tools/tool_event_utils.cpp
 create mode 100644 pcbnew/tools/tool_event_utils.h

diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt
index eedb3a6f0..7c36570c9 100644
--- a/pcbnew/CMakeLists.txt
+++ b/pcbnew/CMakeLists.txt
@@ -297,6 +297,7 @@ set( PCBNEW_CLASS_SRCS
 tools/picker_tool.cpp
 tools/zoom_tool.cpp
 tools/tools_common.cpp
+tools/tool_event_utils.cpp
 tools/tool_menu.cpp
 
 tools/grid_menu.cpp
diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 498d86355..604201fd5 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -117,9 +117,15 @@ TOOL_ACTION COMMON_ACTIONS::createArray( "pcbnew.InteractiveEdit.createArray",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_CREATE_ARRAY ),
 _( "Create array" ), _( "Create array" ), array_module_xpm, AF_ACTIVATE );
 
-TOOL_ACTION COMMON_ACTIONS::rotate( "pcbnew.InteractiveEdit.rotate",
+TOOL_ACTION COMMON_ACTIONS::rotateCw( "pcbnew.InteractiveEdit.rotateCw",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_ROTATE_ITEM ),
-_( "Rotate" ), _( "Rotates selected item(s)" ), rotate_cw_xpm );
+_( "Rotate clockwise" ), _( "Rotates selected item(s) clockwise" ),
+rotate_cw_xpm, AF_NONE, (void*) 1 );
+
+TOOL_ACTION COMMON_ACTIONS::rotateCcw( "pcbnew.InteractiveEdit.rotateCcw",
+AS_GLOBAL, MD_SHIFT + 'R',
+_( "Rotate counter-clockwise" ), _( "Rotates selected item(s) counter-clockwise" ),
+rotate_ccw_xpm, AF_NONE, (void*) -1 );
 
 TOOL_ACTION COMMON_ACTIONS::flip( "pcbnew.InteractiveEdit.flip",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_FLIP_ITEM ),
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index b8167cf3c..b9800dcd4 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -70,8 +70,11 @@ public:
 /// Activation of the edit tool
 static TOOL_ACTION editActivate;
 
-/// Rotation of selected objects
-static TOOL_ACTION rotate;
+/// Rotation of selected objects clockwise
+static TOOL_ACTION rotateCw;
+
+/// Rotation of selected objects counter-clockwise
+static TOOL_ACTION rotateCcw;
 
 /// Flipping of selected objects
 static TOOL_ACTION flip;
diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp
index c0b897361..c2196070d 100644
--- a/pcbnew/tools/drawing_tool.cpp
+++ b/pcbnew/tools/drawing_tool.cpp
@@ -53,7 +53,7 @@
 #include 
 
 #include 
-
+#include 
 
 using SCOPED_DRAW_MODE = SCOPED_SET_RESET;
 
@@ -236,12 +236,14 @@ int DRAWING_TOOL::PlaceText( const TOOL_EVENT& aEvent )
 
 else if( text && evt->Category() == TC_COMMAND )
 {
-if( evt->IsAction( &COMMON_ACTIONS::rotate ) )
+if( TOOL_EVT_UTILS::IsRotateToolEvt( *evt ) )
 {
-text->Rotate( text->GetPosition(), m_frame->GetRotationAngle() );
+const auto rotationAngle = TOOL_EVT_UTILS::GetEventRotationAngle(
+

[Kicad-developers] [PATCH] Fix arc selection over-enthusiastic bounding boxes

2017-02-05 Thread John Beard
Hi,

Here is a patch to fix https://bugs.launchpad.net/kicad/+bug/1492734,
where a small-angle arc has an unexpectedly large bounding box,
causing odd selection behaviour.

The code was actually already there with a TODO on it - this patch
basically just uses that and replaces the TODO with a comment (one
more TODO down!)

Cheers,

John
From 42a706a236276f88a06a1202ab01326614c3e2e9 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sat, 4 Feb 2017 09:42:37 +0800
Subject: [PATCH] Do not include arc centre in bounding box

Arc centres don't land in the bounding box when the arc angle is small.
Currently, there are added to the BB, which leads to surprising
selection beheviour of arc segments (the BB can be much larger than
expected).

This commit omits the arc centre from the calculation.

Fixes: lp:1492734
* https://bugs.launchpad.net/kicad/+bug/1492734
---
 pcbnew/class_drawsegment.cpp | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pcbnew/class_drawsegment.cpp b/pcbnew/class_drawsegment.cpp
index 37d776a0b..50ddf984a 100644
--- a/pcbnew/class_drawsegment.cpp
+++ b/pcbnew/class_drawsegment.cpp
@@ -608,9 +608,10 @@ const BOX2I DRAWSEGMENT::ViewBBox() const
 
 void DRAWSEGMENT::computeArcBBox( EDA_RECT& aBBox ) const
 {
-aBBox.Merge( m_End );
-// TODO perhaps the above line can be replaced with this one, so we do not include the center
-//aBBox.SetOrigin( m_End );
+// Do not include the center, which is not necessarily
+// inside the BB of a arc with a small angle
+aBBox.SetOrigin( m_End );
+
 wxPoint end = m_End;
 RotatePoint( &end, m_Start, -m_Angle );
 aBBox.Merge( end );
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Delete whole tracks in GAL

2017-02-05 Thread John Beard
Hi,

These patches implement deletion of whole tracks (actually whole
connections, it won't span an unrouted gap).

Fixes: https://bugs.launchpad.net/kicad/+bug/1517213

This in in two parts:

* Expand the current connection/track/net selection to act on multiple
selections, not just the first item in the selection list and refactor
the core of the traversal code in each case.
* Register the pre-existing "remove alternate" tool action, add a
concept of "delete flags" to COMMON_TOOLS, and when triggered, use the
flag to conditionally select whole tracks to delete.

The idea of the flags is to allow flexible mappings of modified delete
actions, in much the same way as modifier keys, but without having to
commit to a hardcoded key for each flag, so the mappings can be more
eaisly user defined.

This patch also reverses the mappings of the delete and backspace
keys, so that backspace is "normal remove" and "delete" is "remove
alternate". This keeps the mapping the same between legacy and GAL, as
per the LP issue. Other than tracks, this won't change anything else,
as there is as yet no other effect to alt remove, which wasn't used
until now.

Cheers,

John
From 9fc80d8c5fc971cab9f756bc87a7383b732dfeba Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sat, 4 Feb 2017 14:45:52 +0800
Subject: [PATCH 2/2] Allow delete whole track in GAL

This commit wires up the as-yet-unused "remove alternate" tool action
and uses it to select copper connections (normally 'U') before deleting
segments.

THis also reverses the sense of Delete and Backspace (Delete used to be
'remove' and Backpace was 'remove alt', now it is reversed). This means
that backspace is the key that removes a segment and Delete removes the
track. This is the same as legacy behaviour. Other delete actions are,
for now, the same between Delete and Backspace.

Fixes: lp:1517213
* https://bugs.launchpad.net/kicad/+bug/1517213
---
 pcbnew/tools/common_actions.cpp | 10 ++
 pcbnew/tools/common_actions.h   |  3 +++
 pcbnew/tools/edit_tool.cpp  | 13 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 604201fd5..fec8c683e 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -136,12 +136,14 @@ TOOL_ACTION COMMON_ACTIONS::mirror( "pcbnew.InteractiveEdit.mirror",
 _( "Mirror" ), _( "Mirrors selected item" ), mirror_h_xpm );
 
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
-_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
+_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::NORMAL );
 
 TOOL_ACTION COMMON_ACTIONS::removeAlt( "pcbnew.InteractiveEdit.removeAlt",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
-_( "Remove (alterative)" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
+_( "Remove (alternative)" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::ALT );
 
 TOOL_ACTION COMMON_ACTIONS::exchangeFootprints( "pcbnew.InteractiveEdit.ExchangeFootprints",
 AS_GLOBAL, 0,
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index b9800dcd4..33d0d7ab9 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -356,6 +356,9 @@ public:
 ///> Cursor control event types
 enum CURSOR_EVENT_TYPE { CURSOR_UP, CURSOR_DOWN, CURSOR_LEFT, CURSOR_RIGHT,
  CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
+
+///> Remove event modifier flags
+enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
 };
 
 void registerAllTools( TOOL_MANAGER* aToolManager );
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 031f2db4f..35cace213 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -569,6 +569,18 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
 if( !hoverSelection() || m_selectionTool->CheckLock() == SELECTION_LOCKED )
 return 0;
 
+// is this "alternative" remove?
+const bool isAlt = aEvent.Parameter() ==
+(int) COMMON_ACTIONS::REMOVE_FLAGS::ALT;
+
+// in "alternative" mode, deletion is not just a simple list
+// of selected items, it is:
+//   - whole tracks, not just segments
+if( isAlt )
+{
+m_toolMgr->RunAction( COMMON_ACTIONS::selectConnection, true );
+}
+
 // Get a copy instea

Re: [Kicad-developers] [PATCH] Delete whole tracks in GAL

2017-02-05 Thread John Beard
Apparently those patches don't apply because the diff context lines
have a line of a different non-mainline patch in them (from the rotate
CCW patch).

Hopefully, these will apply to the current master.

On Sun, Feb 5, 2017 at 9:23 PM, John Beard  wrote:
> Hi,
>
> These patches implement deletion of whole tracks (actually whole
> connections, it won't span an unrouted gap).
>
> Fixes: https://bugs.launchpad.net/kicad/+bug/1517213
>
> This in in two parts:
>
> * Expand the current connection/track/net selection to act on multiple
> selections, not just the first item in the selection list and refactor
> the core of the traversal code in each case.
> * Register the pre-existing "remove alternate" tool action, add a
> concept of "delete flags" to COMMON_TOOLS, and when triggered, use the
> flag to conditionally select whole tracks to delete.
>
> The idea of the flags is to allow flexible mappings of modified delete
> actions, in much the same way as modifier keys, but without having to
> commit to a hardcoded key for each flag, so the mappings can be more
> eaisly user defined.
>
> This patch also reverses the mappings of the delete and backspace
> keys, so that backspace is "normal remove" and "delete" is "remove
> alternate". This keeps the mapping the same between legacy and GAL, as
> per the LP issue. Other than tracks, this won't change anything else,
> as there is as yet no other effect to alt remove, which wasn't used
> until now.
>
> Cheers,
>
> John
From 7bf00fa7d5eca17cc14d8aa1ccae57ede8a73f6a Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sat, 4 Feb 2017 14:45:52 +0800
Subject: [PATCH 2/2] Allow delete whole track in GAL

This commit wires up the as-yet-unused "remove alternate" tool action
and uses it to select copper connections (normally 'U') before deleting
segments.

THis also reverses the sense of Delete and Backspace (Delete used to be
'remove' and Backpace was 'remove alt', now it is reversed). This means
that backspace is the key that removes a segment and Delete removes the
track. This is the same as legacy behaviour. Other delete actions are,
for now, the same between Delete and Backspace.

Fixes: lp:1517213
* https://bugs.launchpad.net/kicad/+bug/1517213
---
 pcbnew/tools/common_actions.cpp | 10 ++
 pcbnew/tools/common_actions.h   |  3 +++
 pcbnew/tools/edit_tool.cpp  | 13 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 498d86355..fe485cd1b 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -130,12 +130,14 @@ TOOL_ACTION COMMON_ACTIONS::mirror( "pcbnew.InteractiveEdit.mirror",
 _( "Mirror" ), _( "Mirrors selected item" ), mirror_h_xpm );
 
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
-_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
+_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::NORMAL );
 
 TOOL_ACTION COMMON_ACTIONS::removeAlt( "pcbnew.InteractiveEdit.removeAlt",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
-_( "Remove (alterative)" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
+_( "Remove (alternative)" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::ALT );
 
 TOOL_ACTION COMMON_ACTIONS::exchangeFootprints( "pcbnew.InteractiveEdit.ExchangeFootprints",
 AS_GLOBAL, 0,
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index b8167cf3c..fe8d0f67b 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -353,6 +353,9 @@ public:
 ///> Cursor control event types
 enum CURSOR_EVENT_TYPE { CURSOR_UP, CURSOR_DOWN, CURSOR_LEFT, CURSOR_RIGHT,
  CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
+
+///> Remove event modifier flags
+enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
 };
 
 void registerAllTools( TOOL_MANAGER* aToolManager );
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index a344b3110..60adc3326 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -565,6 +565,18 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
 if( !hoverSelection() || m_selectionTool->CheckLock() == SELECTION_LOCKED )
 return 0;
 
+// is this "

[Kicad-developers] [PATCH] Fix warp-to-nearest pad bug with T hotkey

2017-02-06 Thread John Beard
Hi,

This patch fixes a GAL usability quirk with the T hotkey (find and move).

Fixes: https://bugs.launchpad.net/kicad/+bug/1571214

Cheers,

John
From 191412b93ba263f69d69e75cabb7758b175607bd Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Mon, 6 Feb 2017 18:16:36 +0800
Subject: [PATCH] Warp to module when 'find-moving' modules in GAL

Perviously, the generic snapping code would choose the nearest of the
module origin and origin of each module pad when selecting a module
using the "Find module" tool (T hotkey).

This is unlikely to be expected unless the cursor is already near the
correct pad or the module centre.

New behaviour is to warp the cursor to the module centre first, then
select the module. This means that the cursor is already nearest the
main module origin anchor, so that is what will be used.

Fixes: lp:1571214
* https://bugs.launchpad.net/kicad/+bug/1571214
---
 pcbnew/tools/selection_tool.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index ccb9d7252..26e36bbd2 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -714,6 +714,10 @@ int SELECTION_TOOL::findMove( const TOOL_EVENT& aEvent )
 
 if( module )
 {
+// place cursor on module origin first, so the generic anchor snap
+// doesn't just choose the closest pin for us
+getViewControls()->WarpCursor( module->GetPosition(), true, true );
+
 clearSelection();
 toggleSelection( module );
 m_toolMgr->InvokeTool( "pcbnew.InteractiveEdit" );
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Add duplicate zone onto layer tool

2017-02-06 Thread John Beard
Hi,

This patch adds a missing tool to GAL: duplicate zone onto layer.

It's accessed through the zones context menu and does the same as the
legacy mode version.

Cheers,

John
From 031b15406695e60b17af6bb9e8b984e662c728d8 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Mon, 6 Feb 2017 11:45:56 +0800
Subject: [PATCH] Add zone duplicate onto layer to GAL

This adds it into the PCB_EDITOR_CONTROL tool, alongside the zone merge
tool.
---
 pcbnew/tools/common_actions.cpp |  5 +++
 pcbnew/tools/common_actions.h   |  3 ++
 pcbnew/tools/pcb_editor_control.cpp | 66 +
 pcbnew/tools/pcb_editor_control.h   |  3 ++
 4 files changed, 77 insertions(+)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index d6cfd3b74..78102f576 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -415,6 +415,11 @@ TOOL_ACTION COMMON_ACTIONS::zoneMerge( "pcbnew.EditorControl.zoneMerge",
 AS_GLOBAL, 0,
 _( "Merge zones" ), _( "Merge zones" ) );
 
+TOOL_ACTION COMMON_ACTIONS::zoneDuplicate( "pcbnew.EditorControl.zoneDuplicate",
+AS_GLOBAL, 0,
+_( "Duplicate zone onto layer" ), _( "Duplicate zone outline onto a different layer" ),
+zone_duplicate_xpm );
+
 
 TOOL_ACTION COMMON_ACTIONS::placeTarget( "pcbnew.EditorControl.placeTarget",
 AS_GLOBAL, 0,
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index 42a3f1528..c81b9e073 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -262,6 +262,9 @@ public:
 static TOOL_ACTION zoneUnfillAll;
 static TOOL_ACTION zoneMerge;
 
+/// Duplicate zone onto another layer
+static TOOL_ACTION zoneDuplicate;
+
 // Module editor tools
 /// Activation of the drawing tool (placing a PAD)
 static TOOL_ACTION placePad;
diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp
index 614bd14f5..0c2fdd57e 100644
--- a/pcbnew/tools/pcb_editor_control.cpp
+++ b/pcbnew/tools/pcb_editor_control.cpp
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -68,7 +69,11 @@ public:
 Add( COMMON_ACTIONS::zoneFillAll );
 Add( COMMON_ACTIONS::zoneUnfill );
 Add( COMMON_ACTIONS::zoneUnfillAll );
+
+AppendSeparator();
+
 Add( COMMON_ACTIONS::zoneMerge );
+Add( COMMON_ACTIONS::zoneDuplicate );
 }
 
 protected:
@@ -82,6 +87,13 @@ private:
 {
 SELECTION_TOOL* selTool = getToolManager()->GetTool();
 
+// enable zone actions that act on a single zone
+bool singleZoneActionsEnabled = ( SELECTION_CONDITIONS::Count( 1 )
+  && SELECTION_CONDITIONS::OnlyType( PCB_ZONE_AREA_T )
+)( selTool->GetSelection() );
+
+Enable( getMenuId( COMMON_ACTIONS::zoneDuplicate), singleZoneActionsEnabled );
+
 // enable zone actions that ably to a specific set of zones (as opposed to all of them)
 bool nonGlobalActionsEnabled = ( SELECTION_CONDITIONS::MoreThan( 0 ) )( selTool->GetSelection() );
 
@@ -721,6 +733,59 @@ int PCB_EDITOR_CONTROL::ZoneMerge( const TOOL_EVENT& aEvent )
 }
 
 
+int PCB_EDITOR_CONTROL::ZoneDuplicate( const TOOL_EVENT& aEvent )
+{
+auto selTool = m_toolMgr->GetTool();
+const auto& selection = selTool->GetSelection();
+
+// because this pops up the zone editor, it would be confusing
+// to handle multiple zones, so just handle single selections
+// containing exactly one zone
+if ( selection.Size() != 1 )
+return 0;
+
+auto oldZone = dyn_cast( selection[0] );
+
+if ( !oldZone )
+return 0;
+
+auto newZone = std::make_unique( *oldZone );
+newZone->UnFill();
+ZONE_SETTINGS zoneSettings;
+zoneSettings << *oldZone;
+
+bool success = false;
+
+if( oldZone->GetIsKeepout() )
+success = InvokeKeepoutAreaEditor( m_frame, &zoneSettings );
+else if( oldZone->IsOnCopperLayer() )
+success = InvokeCopperZonesEditor( m_frame, &zoneSettings );
+else
+success = InvokeNonCopperZonesEditor( m_frame, oldZone, &zoneSettings );
+
+// If the new zone is on the same layer as the the initial zone,
+// do nothing
+if( success && ( oldZone->GetLayer() == zoneSettings.m_CurrentZone_Layer ) )
+{
+DisplayError( m_frame,
+_( "The duplicated zone cannot be on the same layer as the original zone." ) );
+success = false;
+}
+
+// duplicate the zone
+if( success )
+{
+BOARD_COMMIT commit( m_frame );
+zoneSettings.ExportSetting( *newZone );
+
+commit.Add( newZone.release() );
+commit.Push( _( "Duplicate zone" ) );
+}
+
+return 0;
+}

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

2017-02-06 Thread 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 u

Re: [Kicad-developers] [PATCH] Delete whole tracks in GAL

2017-02-07 Thread John Beard
The first of these patches has a mistake in the selection code, which
used to clear the selection and replace it, and should now augment,
not replace.

Please use the attached patches instead.

Thanks,

John

On Mon, Feb 6, 2017 at 12:37 AM, John Beard  wrote:
> Apparently those patches don't apply because the diff context lines
> have a line of a different non-mainline patch in them (from the rotate
> CCW patch).
>
> Hopefully, these will apply to the current master.
>
> On Sun, Feb 5, 2017 at 9:23 PM, John Beard  wrote:
>> Hi,
>>
>> These patches implement deletion of whole tracks (actually whole
>> connections, it won't span an unrouted gap).
>>
>> Fixes: https://bugs.launchpad.net/kicad/+bug/1517213
>>
>> This in in two parts:
>>
>> * Expand the current connection/track/net selection to act on multiple
>> selections, not just the first item in the selection list and refactor
>> the core of the traversal code in each case.
>> * Register the pre-existing "remove alternate" tool action, add a
>> concept of "delete flags" to COMMON_TOOLS, and when triggered, use the
>> flag to conditionally select whole tracks to delete.
>>
>> The idea of the flags is to allow flexible mappings of modified delete
>> actions, in much the same way as modifier keys, but without having to
>> commit to a hardcoded key for each flag, so the mappings can be more
>> eaisly user defined.
>>
>> This patch also reverses the mappings of the delete and backspace
>> keys, so that backspace is "normal remove" and "delete" is "remove
>> alternate". This keeps the mapping the same between legacy and GAL, as
>> per the LP issue. Other than tracks, this won't change anything else,
>> as there is as yet no other effect to alt remove, which wasn't used
>> until now.
>>
>> Cheers,
>>
>> John
From 3beb35ba9a3c391acb8de2e57a03002ab5686d53 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Sat, 4 Feb 2017 14:45:52 +0800
Subject: [PATCH 2/2] Allow delete whole track in GAL

This commit wires up the as-yet-unused "remove alternate" tool action
and uses it to select copper connections (normally 'U') before deleting
segments.

THis also reverses the sense of Delete and Backspace (Delete used to be
'remove' and Backpace was 'remove alt', now it is reversed). This means
that backspace is the key that removes a segment and Delete removes the
track. This is the same as legacy behaviour. Other delete actions are,
for now, the same between Delete and Backspace.

Fixes: lp:1517213
* https://bugs.launchpad.net/kicad/+bug/1517213
---
 pcbnew/tools/common_actions.cpp | 10 ++
 pcbnew/tools/common_actions.h   |  3 +++
 pcbnew/tools/edit_tool.cpp  | 13 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 498d86355..fe485cd1b 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -130,12 +130,14 @@ TOOL_ACTION COMMON_ACTIONS::mirror( "pcbnew.InteractiveEdit.mirror",
 _( "Mirror" ), _( "Mirrors selected item" ), mirror_h_xpm );
 
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
-_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
+_( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::NORMAL );
 
 TOOL_ACTION COMMON_ACTIONS::removeAlt( "pcbnew.InteractiveEdit.removeAlt",
-AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
-_( "Remove (alterative)" ), _( "Deletes selected item(s)" ), delete_xpm );
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
+_( "Remove (alternative)" ), _( "Deletes selected item(s)" ), delete_xpm,
+AF_NONE, (void*) REMOVE_FLAGS::ALT );
 
 TOOL_ACTION COMMON_ACTIONS::exchangeFootprints( "pcbnew.InteractiveEdit.ExchangeFootprints",
 AS_GLOBAL, 0,
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index b8167cf3c..fe8d0f67b 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -353,6 +353,9 @@ public:
 ///> Cursor control event types
 enum CURSOR_EVENT_TYPE { CURSOR_UP, CURSOR_DOWN, CURSOR_LEFT, CURSOR_RIGHT,
  CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
+
+///> Remove event modifier flags
+enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
 };
 
 void regis

[Kicad-developers] [PATCH] Show zero thickness lines in GAL

2017-02-07 Thread John Beard
Hi,

Here is a patch to show zero-thickness lines in GAL using the "outline width".

This addressess https://bugs.launchpad.net/kicad/+bug/1501749

Cheers,

John
From 60dfc34c70493bf05f77ecd4dca7594b46798c8e Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 7 Feb 2017 18:21:26 +0800
Subject: [PATCH] Show lines of zero thickness in GAL

If a line has zero thickness, use the outline thickness to draw it.

This avoids having invisible items on the PCB that could still end up in
outputs, or "losing" an item by setting thickness to 0.

This only affects GAL drawing routines, the PCB data structures are not
affected, so any outputs will be the same.

Fixes: lp:1501749
* https://bugs.launchpad.net/kicad/+bug/1501749
---
 pcbnew/pcb_painter.cpp | 22 +-
 pcbnew/pcb_painter.h   |  9 +
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp
index f46bf85fd..20f84a0fc 100644
--- a/pcbnew/pcb_painter.cpp
+++ b/pcbnew/pcb_painter.cpp
@@ -247,6 +247,18 @@ PCB_PAINTER::PCB_PAINTER( GAL* aGal ) :
 }
 
 
+int PCB_PAINTER::getLineThickness( int aActualThickness ) const
+{
+// if items have 0 thickness, draw them with the outline
+// width, otherwise respect the set value (which, no matter
+// how small will produce something)
+if( aActualThickness == 0 )
+return m_pcbSettings.m_outlineWidth;
+
+return aActualThickness;
+}
+
+
 bool PCB_PAINTER::Draw( const VIEW_ITEM* aItem, int aLayer )
 {
 const EDA_ITEM* item = static_cast( aItem );
@@ -784,7 +796,7 @@ void PCB_PAINTER::draw( const DRAWSEGMENT* aSegment, int aLayer )
 if( m_pcbSettings.m_sketchMode[aLayer] )
 m_gal->SetLineWidth( m_pcbSettings.m_outlineWidth );// Outline mode
 else
-m_gal->SetLineWidth( aSegment->GetWidth() );// Filled mode
+m_gal->SetLineWidth( getLineThickness( aSegment->GetWidth() ) ); // Filled mode
 
 switch( aSegment->GetShape() )
 {
@@ -871,7 +883,7 @@ void PCB_PAINTER::draw( const TEXTE_PCB* aText, int aLayer )
 else
 {
 // Filled mode
-m_gal->SetLineWidth( aText->GetThickness() );
+m_gal->SetLineWidth( getLineThickness( aText->GetThickness() ) );
 }
 
 m_gal->SetStrokeColor( color );
@@ -899,7 +911,7 @@ void PCB_PAINTER::draw( const TEXTE_MODULE* aText, int aLayer )
 else
 {
 // Filled mode
-m_gal->SetLineWidth( aText->GetThickness() );
+m_gal->SetLineWidth( getLineThickness( aText->GetThickness() ) );
 }
 
 m_gal->SetStrokeColor( color );
@@ -1021,7 +1033,7 @@ void PCB_PAINTER::draw( const DIMENSION* aDimension, int aLayer )
 m_gal->SetStrokeColor( strokeColor );
 m_gal->SetIsFill( false );
 m_gal->SetIsStroke( true );
-m_gal->SetLineWidth( aDimension->GetWidth() );
+m_gal->SetLineWidth( getLineThickness( aDimension->GetWidth() ) );
 
 // Draw an arrow
 m_gal->DrawLine( VECTOR2D( aDimension->m_crossBarO ), VECTOR2D( aDimension->m_crossBarF ) );
@@ -1050,7 +1062,7 @@ void PCB_PAINTER::draw( const PCB_TARGET* aTarget )
 VECTOR2D position( aTarget->GetPosition() );
 double   size, radius;
 
-m_gal->SetLineWidth( aTarget->GetWidth() );
+m_gal->SetLineWidth( getLineThickness( aTarget->GetWidth() ) );
 m_gal->SetStrokeColor( strokeColor );
 m_gal->SetIsFill( false );
 m_gal->SetIsStroke( true );
diff --git a/pcbnew/pcb_painter.h b/pcbnew/pcb_painter.h
index c86991862..718f619a1 100644
--- a/pcbnew/pcb_painter.h
+++ b/pcbnew/pcb_painter.h
@@ -228,6 +228,15 @@ protected:
 void draw( const DIMENSION* aDimension, int aLayer );
 void draw( const PCB_TARGET* aTarget );
 void draw( const MARKER_PCB* aMarker );
+
+/**
+ * Function getLineThickness()
+ * Get the thickness to draw for a line (e.g. 0 thickness lines
+ * get a minimum value).
+ * @param aActualThickness line own thickness
+ * @return the thickness to draw
+ */
+int getLineThickness( int aActualThickness ) const;
 };
 } // namespace KIGFX
 
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Dragging with tracks in GAL

2017-02-07 Thread John Beard
Hi,

Here is a really basic concept patch for component dragging with
tracks ('G') in pcbnew GAL.

It uses the same dragging mechanism as legacy for modules and pads. If
it's not desired to continue to use that code for the time being, at
least this provides the TOOL_ACTION hook and outlines where new code
would have to be added.

However, the drag.h interface is pretty small, so I don't think using
it is too objectionable for the time being.

For dragging traces, the much nicer PNS dragging is used, but this
doesn't work with a block selection. I'm not that familiar with the
PNS, so I'm not sure where to start to do that.

Cheers,

John
From 7e493417cf1f04912a0f47681a40f30f2a9e96e3 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Thu, 2 Feb 2017 00:14:18 +0800
Subject: [PATCH] Add drag tool to GAL edit tool

This adds segment endpoint dragging when moving modules/pads and calls
the existing PNS interactive router for dragging tracks.

Drag tool is activated using the 'G' hotkey, same as legacy.

Fixes: lp:1661962
* https://bugs.launchpad.net/kicad/+bug/1661962
---
 pcbnew/tools/common_actions.cpp |  8 -
 pcbnew/tools/common_actions.h   |  6 
 pcbnew/tools/edit_tool.cpp  | 65 +++--
 pcbnew/tools/edit_tool.h| 26 -
 4 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 850d8b2fa..783c8005f 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -98,7 +98,13 @@ TOOL_ACTION COMMON_ACTIONS::globalEditPads( "pcbnew.InteractiveEdit.globalPadEdi
 
 TOOL_ACTION COMMON_ACTIONS::editActivate( "pcbnew.InteractiveEdit",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_MOVE_ITEM ),
-_( "Move" ), _( "Moves the selected item(s)" ), move_xpm, AF_ACTIVATE );
+_( "Move" ), _( "Moves the selected item(s)" ),
+move_xpm, AF_ACTIVATE, (void*) MOVE_FLAGS::NORMAL );
+
+TOOL_ACTION COMMON_ACTIONS::drag( "pcbnew.InteractiveEdit.drag",
+AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DRAG_ITEM ),
+_( "Drag" ), _( "Drags the selected item(s) along with connected tracks" ),
+drag_module_xpm, AF_ACTIVATE, (void*) MOVE_FLAGS::DRAG_TRACKS );
 
 TOOL_ACTION COMMON_ACTIONS::duplicate( "pcbnew.InteractiveEdit.duplicate",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DUPLICATE_ITEM ),
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index 61f1a4ec0..643252ee0 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -70,6 +70,9 @@ public:
 /// Activation of the edit tool
 static TOOL_ACTION editActivate;
 
+/// Activation of the drag tool
+static TOOL_ACTION drag;
+
 /// Rotation of selected objects clockwise
 static TOOL_ACTION rotateCw;
 
@@ -359,6 +362,9 @@ public:
 
 ///> Remove event modifier flags
 enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
+
+///> Move event modifier flags
+enum class MOVE_FLAGS { NORMAL = 0x00, DRAG_TRACKS = 0x01 };
 };
 
 void registerAllTools( TOOL_MANAGER* aToolManager );
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 35cace213..ed2bad730 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -125,7 +126,7 @@ bool EDIT_TOOL::Init()
 }
 
 
-bool EDIT_TOOL::invokeInlineRouter()
+bool EDIT_TOOL::invokeInlineRouter( bool aForce )
 {
 TRACK* track = uniqueSelected();
 VIA* via = uniqueSelected();
@@ -135,7 +136,7 @@ bool EDIT_TOOL::invokeInlineRouter()
 ROUTER_TOOL* theRouter = static_cast( m_toolMgr->FindTool( "pcbnew.InteractiveRouter" ) );
 assert( theRouter );
 
-if( !theRouter->PNSSettings().InlineDragEnabled() )
+if( !aForce && !theRouter->PNSSettings().InlineDragEnabled() )
 return false;
 
 m_toolMgr->RunAction( COMMON_ACTIONS::routerInlineDrag, true, track ? track : via );
@@ -145,6 +146,50 @@ bool EDIT_TOOL::invokeInlineRouter()
 return false;
 }
 
+void EDIT_TOOL::setupDragList( const SELECTION& aSelection )
+{
+EraseDragList();
+DRAG_LIST drglist( getModel() );
+
+for( auto item : aSelection )
+{
+switch( item->Type() )
+{
+case PCB_PAD_T:
+{
+auto pad = static_cast( item );
+drglist.BuildDragListe( pad );
+break;
+}
+case PCB_MODULE_T:
+{
+auto module = static_cast( item );
+drglist.BuildDragListe( module );
+break;
+}
+default:
+break;
+}
+}
+
+// save modifications to dragged segments
+for( unsigned ii = 0;

Re: [Kicad-developers] [PATCH] Dragging with tracks in GAL

2017-02-07 Thread John Beard
This needs with some other patches on the list. To use this patch, you
need the "CCW rotate" patch and the "delete tracks" patch to be
applied first.

There is a branch with just this patch (which will conflict with the
above mentioned patches) at
https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/drag_tracks

On Wed, Feb 8, 2017 at 9:47 AM, John Beard  wrote:
> Hi,
>
> Here is a really basic concept patch for component dragging with
> tracks ('G') in pcbnew GAL.
>
> It uses the same dragging mechanism as legacy for modules and pads. If
> it's not desired to continue to use that code for the time being, at
> least this provides the TOOL_ACTION hook and outlines where new code
> would have to be added.
>
> However, the drag.h interface is pretty small, so I don't think using
> it is too objectionable for the time being.
>
> For dragging traces, the much nicer PNS dragging is used, but this
> doesn't work with a block selection. I'm not that familiar with the
> PNS, so I'm not sure where to start to do that.
>
> Cheers,
>
> John

___
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] Issues in right click context menu in GAL mode on Windows.

2017-02-08 Thread John Beard
On Thu, Feb 9, 2017 at 2:01 AM, Kristoffer Ödmark
 wrote:
> I am seeing the "Pads" menu entry when right-clicking empty in linux, "push
> pads settings" is enabled in the submenus and if I press that one and get a
> small dialog, the button "Pad Editor" gives me a segfault. I am guessing
> that is because there is no footprint selected.

I think that's more likely that there is no global pad setting to use
and a missing null check, but either way, that's bad.

There is a branch "pad_menus" at
https://git.launchpad.net/~john-j-beard/kicad that adjusts some of the
PAD_TOOL behaviour:

* Pad push only works from a single selected pad, no longer able to
use the global pad setting from an empty selection, which is a bit
"secret".
* Pad import/apply is only available when there is a global pad
setting, even if a pad is selected.
* The whole menu is hidden if none of the options are available, or of
there are no footprints (e.g. blank module editor)
* The enumerate pads action is now in the Pads submenu (and therefore
hidden until a footprint is loaded

Cheers,

John

___
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] Issues in right click context menu in GAL mode on Windows.

2017-02-08 Thread John Beard
Here is a Launchpad bug for tracking the broken Windows submenus:
https://bugs.launchpad.net/kicad/+bug/1663101

On Wed, Feb 8, 2017 at 10:37 PM, jp charras  wrote:
> Recently, some enhancements were added to the right click popup context menus.
>
> On windows, most are broken:
>
> For instance, Board editor:
> context menu for zones: submenus are not working.
> context menu with no tool: the pad export import menu is always shown.
> context menu for tracks: the title "interactive router" has a sub menu "zoom"
> the next menuitem is the grid submenu (zoom and grid submenus arae 
> also at the bottom of the popup
> menu)
>
> Footprint editor:
> the context menu also always shows the "pads" and export import menuitem, 
> even when no footprint is
> loaded.
>
> --
> Jean-Pierre CHARRAS
>
> ___
> 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] [PATCH] Delete whole tracks in GAL

2017-02-08 Thread John Beard
On Wed, Feb 8, 2017 at 6:48 AM, Nick Østergaard  wrote:
> Mm, it seems this also reverses it such that you have to use backspace
> to delete a footprint whereas in that case the delete key should be
> used.

Hi Nick,

The latest patch (Feb 7) should not do that - for me, both 'delete' and
'backspace' can be used to delete footprints.

The only difference should be that 'delete' also adds in the connected
tracks of any segments in the selection.

Cheers,

John

___
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


[Kicad-developers] [PATCH] common.h tidyups

2017-02-09 Thread John Beard
Hi,

I have a big set of patches on a branch that are mostly to do with
stripping seldom-used functions out of common.h into more targeted
headers:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/tidy_ups

The idea here is to reduce the visibility of these functions from
almost every file in KiCad to just those that need them. This also
means that changing the signatures (or even just adding comments)
doesn't force a huge recompile of the whole codebase, just the files
that use them. It also makes common.h less of a "dumping ground" for
random stuff.

There were several old functions or variables that weren't used,
weren't defined, or weren't declared too, and these were tidied up.

I have used std::unique_ptr in a couple of places, at one was leaking
an object before.

I only meant to do a few, but since any big work in common.h is as bad
as small work, I did quite a few.

The patches should mostly be cherry-pickable in case not all of them are wanted.

Cheers,

John

___
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] [PATCH] common.h tidyups

2017-02-09 Thread John Beard
Hi Wayne,

Yes, first in the list is "8f192fe12 OPENGL_GAL: Init currentTarget"

That particular one is actually an older patch I had on my tidy-up
branch and isn't really in the common.h set, but it keeps my IDE
quieter! 5e6a022b5 is the first common.h one.

Cheers,

John

On Thu, Feb 9, 2017 at 9:56 PM, Wayne Stambaugh  wrote:
> John,
>
> I like this idea.  Anything that prevents unnecessary compiling is a
> good thing.  Looking at this repo, I'm guessing that everything from
> commit 8f192fe12eb52efd6d024c82d1416db5e62b345f on is what needs to be
> merged.  I'll try to take a look at it over the weekend.  Keep up the
> great work.
>
> Thanks,
>
> Wayne
>
> On 2/9/2017 8:44 AM, John Beard wrote:
>> Hi,
>>
>> I have a big set of patches on a branch that are mostly to do with
>> stripping seldom-used functions out of common.h into more targeted
>> headers:
>>
>> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/tidy_ups
>>
>> The idea here is to reduce the visibility of these functions from
>> almost every file in KiCad to just those that need them. This also
>> means that changing the signatures (or even just adding comments)
>> doesn't force a huge recompile of the whole codebase, just the files
>> that use them. It also makes common.h less of a "dumping ground" for
>> random stuff.
>>
>> There were several old functions or variables that weren't used,
>> weren't defined, or weren't declared too, and these were tidied up.
>>
>> I have used std::unique_ptr in a couple of places, at one was leaking
>> an object before.
>>
>> I only meant to do a few, but since any big work in common.h is as bad
>> as small work, I did quite a few.
>>
>> The patches should mostly be cherry-pickable in case not all of them are 
>> wanted.
>>
>> Cheers,
>>
>> John
>>
>> ___
>> 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: 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] [PATCH] Signal handlers for SINGLE_TOP and KICAD

2017-02-09 Thread John Beard
On Fri, Jan 27, 2017 at 12:44 AM, Wayne Stambaugh  wrote:
> Simon,
>
> Thanks for testing this.  It was on my list but it looks like it has to
> be fixed before we can commit it.
>
> Cheers,
>
> Wayne

Thanks for testing on OSX, Simon.

On Linux, I never see this, which must mean somehow that Python isn't
"eating" the SIGINT.

I do, however, see a segfault when Ctrl-C'ing out of the OpenGL canvas
which is to do with exiting in the "wrong" way from the wxApp. I tried
a different way, but it would only take effect after the window gets
focus after the Ctrl-C.

I'll keep looking into it.

Cheers,

John

___
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] [PATCH] Delete whole tracks in GAL

2017-02-09 Thread John Beard
The branch has been rebased to avoid conflicts with recent GAL commits:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/deletetrack

On Thu, Feb 9, 2017 at 2:51 PM, Nick Østergaard  wrote:
> Ahh ok, I see that is working alright.
>
> 2017-02-09 5:44 GMT+01:00 John Beard :
>> On Wed, Feb 8, 2017 at 6:48 AM, Nick Østergaard  wrote:
>>> Mm, it seems this also reverses it such that you have to use backspace
>>> to delete a footprint whereas in that case the delete key should be
>>> used.
>>
>> Hi Nick,
>>
>> The latest patch (Feb 7) should not do that - for me, both 'delete' and
>> 'backspace' can be used to delete footprints.
>>
>> The only difference should be that 'delete' also adds in the connected
>> tracks of any segments in the selection.
>>
>> Cheers,
>>
>> John

___
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] Issues in right click context menu in GAL mode on Windows.

2017-02-10 Thread John Beard
On Fri, Feb 10, 2017 at 3:44 PM, Maciej Suminski
 wrote:
>
> On 02/08/2017 03:37 PM, jp charras wrote:
>>
>> For instance, Board editor:
>> context menu for zones: submenus are not working.
>> context menu with no tool: the pad export import menu is always shown.
>> context menu for tracks: the title "interactive router" has a sub menu "zoom"
>>   the next menuitem is the grid submenu (zoom and grid submenus arae 
>> also at the bottom of the popup
>> menu)
>>
>> Footprint editor:
>> the context menu also always shows the "pads" and export import menuitem, 
>> even when no footprint is
>> loaded.
>
> Thank you for the report, the problem should be fixed now.

Hi Orson,

The patches for adjusting the pad too menu enablements (points 2 and 4
in JP's original email) are on this branch:

https://git.launchpad.net/~john-j-beard/kicad branch pad_menus

I added a function for the menus to check if they have any enabled
items and used it to only put the PAD_TOOL menu in the main context
menu if there are enabled items. Do you think that approch makes
sense?

Cheers,

John

___
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


[Kicad-developers] [PATCH] GAL: Add similar and cutout zone tools

2017-02-10 Thread John Beard
Hi,

This branch has patches for the Add Similar Zone and Add Zone Cutout
tools in GAL:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/zone_tools

They are preceded by a refactor which makes the tools themselves into
relatively straightforward modifications of the existing interactive
zone tool.

Cheers,

John

___
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


[Kicad-developers] [PATCH] Select filter dialog in GAL

2017-02-11 Thread John Beard
Hi,

Here is a branch with a GAL action to invoke the block select dialog
and filter the current selection based on the results:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/select_filter

The first commit refactors the dialog so it can be reached by the GAL
too, the second plumbs it in to the GAL selection tool. I have used
'Shift-F' to invoke the dialog.

Possibly controversial: use of a compilation firewall (PImpl
idiom/opaque pointer) to allow SELECTION_TOOL to keep an instance of
the dialog options for persistance between invocations. As the class
is a nested class, it can't be simply forward declared.

If it is preferred to not use PImpl in this way (since it's not
commonly done in Kicad), I can redo it so the options class is not
nested and forward declare it.

Cheers,

John

___
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] {PATCH] Icon tweaks

2017-02-11 Thread John Beard
Hi Piotr,

It's a nice idea, and I certainly think it would be good not to have
to compile in the icons, but instead be able to load from files.
However, as far as I know, there are some technical problems with
that, to do with WX icons only being able to use XPM on all platforms,

If nothing else, it might reduce the horrible recompile that happens
if bitmaps.h gets touched.

Cheers,

John

On Sat, Feb 11, 2017 at 4:05 PM, Piotr Esden-Tempski  wrote:
> Did anyone consider adding the ability for “themes”/“resource packs"? So
> that one could load and switch packs that contain icons, colors and fonts. A
> good default is very important, but an easy way of testing out modifications
> to icons and color themes might be a nice thing to have.
>
> Cheers,
> Piotr
>
> On Feb 10, 2017, at 11:38 PM, Oliver Walters
>  wrote:
>
> I like the use of the book to signify the libraries but why the return to
> using the pencil?
>
> The icons are way better without the pencils in my opinion and if they
> feature in every icon then what differentiating purpose do they serve?
>
> On 11 Feb 2017 6:06 PM, "Константин Барановский"
>  wrote:
>>
>> I'm also working on new icons for a long time. You can find them in my
>> branch:
>> https://code.launchpad.net/~baranovskiykonstantin/kicad/+git/kicad
>>
>>
>> 2017-02-11 1:10 GMT+02:00 Ben Hest :
>>>
>>> Not entirely sure which OS you are using, but LightShot [1] is fantastic
>>> at being able take pictures of menus on Windows. (It works on Mac also)
>>>
>>> [1] https://app.prntscr.com/en/index.html
>>>
>>> On Fri, Feb 10, 2017 at 3:55 PM, John Beard 
>>> wrote:
>>>>
>>>> And here are the same images with the changed icons highlighted, maybe
>>>> not everyone loves spot-the-difference puzzles that much!
>>>>
>>>> Some icons are hidden in menus which I don't seem to be able to
>>>> screenshot.
>>>>
>>>> Cheers,
>>>>
>>>> John
>>>>
>>>> On Sat, Feb 11, 2017 at 5:37 AM, John Beard 
>>>> wrote:
>>>> > Hi,
>>>> >
>>>> > I have a few tweaks to some icons in Pcbnew and Eeschema. These are
>>>> > mostly minor tweaks that are intended to make the icons match a little
>>>> > better and give a more uniform look and feel:
>>>> >
>>>> > https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/icons
>>>> >
>>>> > * Pixel align a few icons to make then crisper when displayed
>>>> > * Redraw all the zoom icons to be crisper and flatter. The zoom to fit
>>>> > (Home) icon is now a sort of reticle rather than a pair of square
>>>> > brackets as text.
>>>> > * Redraw console icon to be flat to match other existing icons.
>>>> > * Mute some very bright colours, especially the green-legged module
>>>> > icons - use the zone icons as a guide for the colour brightness.
>>>> >
>>>> > Attached are screen shots of pcbnew and eeschema, where you can see
>>>> > some of the icons.
>>>> >
>>>> > I know icons and UI subjects in general are very subjective and
>>>> > personal, so I'd welcome input!
>>>> >
>>>> > Cheers,
>>>> >
>>>> > John
>>>>
>>>> ___
>>>> 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
>>>>
>>>
>>>
>>>
>>> --
>>>
>>> -Ben
>>>
>>> ___
>>> 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: 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: 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


[Kicad-developers] Fwd: {PATCH] Icon tweaks

2017-02-12 Thread John Beard
On Sat, Feb 11, 2017 at 5:54 AM, Nick Østergaard  wrote:
> While we are at it, what about making the line graphic icon such that
> it matches the style of the arc and circle?  That is making it not
> dashed, and add anchor point indicators?

I think that the "Graphic Line" tool in pcbnew should actually be the
existing add_polygon icon, as this is what you get if you use the tool
with more than 2 nodes: a connected series of lines.

On the other hand, the add_polygon icon should possibly be closed
polygon - what is shown is actually a polyline.

___
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] {PATCH] Icon tweaks

2017-02-12 Thread John Beard
Hi Fabrizio,

The changes are on the tip of this branch:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/icons

I have deliberately tried not made drastic changes to the icons such
that they would be unrecognisable to users of older documentation,
it's mostly alignment and colour tweaks.

If they get merged, I should update the kicad-doc documentation before
v5 gets cut.

Updating the documentation is easy to replace the icons themselves
(copy/paste from source repo to docs repo), but will also need some
new screenshots too.

Cheers,

John

On Mon, Feb 13, 2017 at 1:27 AM, Fabrizio Tappero
 wrote:
> Hi John,
> these improved icons look great! thanks for the effort. I think this work
> should be included. Could you please submit a patch for this to Wayne or JP.
>
> A lot of icons could be improved and as Wayne mentioned it is important to
> kind of coordinate us so that we do not create great icons that do not get
> submitted to the main kiCad stream. John if you think you want contribute to
> this, there is a lot of new and great icons from Konstantin that could be
> picked and submitted. Mind that we cannot start redesigning tons of new
> icons because the documentation will then be heavily out of sync.
>
> thanks a lot
> Fabrizio
>
>
> On Fri, Feb 10, 2017 at 10:55 PM, John Beard  wrote:
>>
>> And here are the same images with the changed icons highlighted, maybe
>> not everyone loves spot-the-difference puzzles that much!
>>
>> Some icons are hidden in menus which I don't seem to be able to
>> screenshot.
>>
>> Cheers,
>>
>> John
>>
>> On Sat, Feb 11, 2017 at 5:37 AM, John Beard 
>> wrote:
>> > Hi,
>> >
>> > I have a few tweaks to some icons in Pcbnew and Eeschema. These are
>> > mostly minor tweaks that are intended to make the icons match a little
>> > better and give a more uniform look and feel:
>> >
>> > https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/icons
>> >
>> > * Pixel align a few icons to make then crisper when displayed
>> > * Redraw all the zoom icons to be crisper and flatter. The zoom to fit
>> > (Home) icon is now a sort of reticle rather than a pair of square
>> > brackets as text.
>> > * Redraw console icon to be flat to match other existing icons.
>> > * Mute some very bright colours, especially the green-legged module
>> > icons - use the zone icons as a guide for the colour brightness.
>> >
>> > Attached are screen shots of pcbnew and eeschema, where you can see
>> > some of the icons.
>> >
>> > I know icons and UI subjects in general are very subjective and
>> > personal, so I'd welcome input!
>> >
>> > Cheers,
>> >
>> > John
>>
>> ___
>> 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] RFC: Change UX for item selection clarification

2017-02-12 Thread John Beard
On Mon, Feb 13, 2017 at 6:39 AM, Jon Evans  wrote:
>
> The core of the issue is how KiCad thinks about selection of objects
> compared to most other software that deals with graphical objects that the
> user interacts with.
>
> Most applications have the concept of a "selection set" that contains 0 or
> more objects.  You can select one or more objects, and then separately from
> the action of selecting, you can do things to the objects.  KiCad uses a
> more "immediate" concept of selection -- you select an object at the same
> time as doing something to it.

Hi Jon,

Are you talking about the GAL canvases, or legacy? The GAL canvases do
have a concept of the "current selection", which is what is used by
actions, and the drag selection works a lot like a "normal" graphical
program like Inkscape, for example.

You can also use Shift-click to add items to a selection, before
invoking an action.

Invoking an action without a selection often has an additional special
behaviour - select whatever is currently under the cursor and act on
that. Generally, you see this in hotkeys, e.g. 'M', and this saves a
lot of clicking if you only want to deal with one item.

Cheers,

John

___
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] {PATCH] Icon tweaks

2017-02-14 Thread John Beard
Hi,

I think Konstantin's icons are a more complete and homegenous set -
I'd rather see those go in first. If there is still any use for my
icons after that, I can rebase on top of that.

He and I have talked about rebasing his branch into a simpler set of
smaller patches (eg. just zoom icons) for easier review, since I think
icons can easily attracts lots of comment, and getting bogged down in
talking about colours of vias when the zoom icons are fine would be
frustrating!

Cheers,

John

On Wed, Feb 15, 2017 at 12:37 AM, Fabrizio Tappero
 wrote:
> Hi Wayne,
> John metion that his changes are on the tip of this branch:
>
> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/icons
>
> Also, would you please be so kind to include this icon in attachment. I have
> some problem is generating the patch for it.
>
> thank you
> Fabrizio
>
>
>
> On Tue, Feb 14, 2017 at 3:39 PM, Wayne Stambaugh 
> wrote:
>>
>> Where do we stand on this?  I think these changes should be committed.
>> If there are no objections, please send a patch to the dev list so I can
>> merge it.
>>
>> On 2/10/2017 4:37 PM, John Beard wrote:
>> > Hi,
>> >
>> > I have a few tweaks to some icons in Pcbnew and Eeschema. These are
>> > mostly minor tweaks that are intended to make the icons match a little
>> > better and give a more uniform look and feel:
>> >
>> > https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/icons
>> >
>> > * Pixel align a few icons to make then crisper when displayed
>> > * Redraw all the zoom icons to be crisper and flatter. The zoom to fit
>> > (Home) icon is now a sort of reticle rather than a pair of square
>> > brackets as text.
>> > * Redraw console icon to be flat to match other existing icons.
>> > * Mute some very bright colours, especially the green-legged module
>> > icons - use the zone icons as a guide for the colour brightness.
>> >
>> > Attached are screen shots of pcbnew and eeschema, where you can see
>> > some of the icons.
>> >
>> > I know icons and UI subjects in general are very subjective and
>> > personal, so I'd welcome input!
>> >
>> > Cheers,
>> >
>> > John
>> >
>> >
>> >
>> > ___
>> > 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: 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


[Kicad-developers] [PATCH] Setting grid line sizes and density

2017-02-15 Thread John Beard
Hi,

Here is a branch with a set of patches to add the ability to set the
grid line/dot size and the minimum grid spacing (to avoid very dense
grids when using thicker lines).

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/grid_dot_size

This also includes moving the grid display settings our of the Set
Grid dialog in to the Display Preferences dialog. As an item in the
Dimensions menu, Set Grid isn't really a suitable place, especially as
there are now more options.

This was a fairly annoying change to make in the code, as the current
GAL properties structure was only consumed by the OpenGL GAL canvas.
However, now, all GAL canvases have access to it, which opens the way
for more display options to be added easily (though if the Display
Options dialog gets more options, we might need to put some tabs in,
like Eeschema).

There are also some supporting infrastructure changes:

* A class to bind a text entry and spin buttons together without a lot
of verbose repeating of event bindings.
* Utility functions to use a mapping table to decouple internal enum
values from the values written into config files and radio button
selections
* Over-visible header causing slow recompilation, hidden a bit better

Cheers,

John

___
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


[Kicad-developers] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-02-16 Thread John Beard
Hi,

I was trying to profile the eeschema slow library loads, and I got a
bit distracted by RICHIO's FILE_LINE_READER.

Internally, it uses a very tight loop of reading single chars at a
time from a file descriptor, which looks inefficient. I wrote a
benchmarker to compare RICHIO against std::ifstream and a new
LINE_READER implementation, backed by std::ifstream. operf confirms
that most of the time in RICHIO burned in the ReadLine() function
itself.

The results were that RICHIO (in debug mode) is consistently 4-7 times
slower than using std::ifstream, when reading eeschema library text
files (so relatively short lines). Compiling the release version
improved RICHIOs speed more than std::ifstream's, but it is still
around 3 times slower than std::ifstream.

For files with 1k lines, the slowdown is about 30 times (!) in debug
and 14 times in release mode, so significantly worse. Few files read
line-wise by Kicad look like that, however.

Avoiding reconstructing the stream/LINE_READER each time doesn't have
much of an effect in any case.

Is there a particular reason why STL streams are not used in RICHIO?
The only thing I think the example ifstream implementation can't do is
catching over long lines, but that's only used in one place: the VRML
parser, which hardcodes an 8MB limit. ifstream could do this, but not
with the simple getline function.

This performance doesn't appear to be a major bottleneck for me, but
it does seem a shame to throw away (charitably) two thirds of file
read speeds (and uncharitably, up to 97% in odd cases) if there is no
particular reason to do so.

As an aside, RICHIO appears to allocate twice as many times as
std::ifstream when reading the same data, for the roughly the same
amount of memory in total.

Anyway, I thought I'd share this finding! Please find attached the
benchmark program, such as it is.

Cheers,

John
From 9030890b9ad86ff2601a0488ef6e0003dfbb2e66 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 15 Feb 2017 16:47:41 +0800
Subject: [PATCH] IO benchmark program: RICHIO vs std::fstream

Also vs fstream-backed LINE_READER.
---
 qa/CMakeLists.txt|   2 +
 qa/io_benchmark/CMakeLists.txt   |  17 
 qa/io_benchmark/fstream_io.cpp   |  77 +++
 qa/io_benchmark/fstream_io.h |  55 +++
 qa/io_benchmark/io_benchmark.cpp | 199 +++
 5 files changed, 350 insertions(+)
 create mode 100644 qa/io_benchmark/CMakeLists.txt
 create mode 100644 qa/io_benchmark/fstream_io.cpp
 create mode 100644 qa/io_benchmark/fstream_io.h
 create mode 100644 qa/io_benchmark/io_benchmark.cpp

diff --git a/qa/CMakeLists.txt b/qa/CMakeLists.txt
index bd579a597..a56f15f51 100644
--- a/qa/CMakeLists.txt
+++ b/qa/CMakeLists.txt
@@ -9,3 +9,5 @@ if( KICAD_SCRIPTING_MODULES )
 )
 
 endif()
+
+add_subdirectory( io_benchmark )
diff --git a/qa/io_benchmark/CMakeLists.txt b/qa/io_benchmark/CMakeLists.txt
new file mode 100644
index 0..fde89c9d2
--- /dev/null
+++ b/qa/io_benchmark/CMakeLists.txt
@@ -0,0 +1,17 @@
+
+include_directories( BEFORE ${INC_BEFORE} )
+
+set( IOBENCHMARK_SRCS
+io_benchmark.cpp
+fstream_io.cpp
+)
+
+add_executable( io_benchmark
+${IOBENCHMARK_SRCS}
+)
+
+target_link_libraries( io_benchmark
+common
+${wxWidgets_LIBRARIES}
+)
+
diff --git a/qa/io_benchmark/fstream_io.cpp b/qa/io_benchmark/fstream_io.cpp
new file mode 100644
index 0..420f57348
--- /dev/null
+++ b/qa/io_benchmark/fstream_io.cpp
@@ -0,0 +1,77 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 1992-2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include "fstream_io.h"
+
+#include 
+
+FSTREAM_LINE_READER::FSTREAM_LINE_READER( const wxString& aFileName,
+unsigned aStartingLineNumber,
+unsigned aMaxLineLength ) throw( IO_ERROR ) :
+LINE_READER( aMaxLineLength ),
+m_stream( aFileName )
+{
+line = &m_buffer[0];
+
+if( !m_stream.is_ope

Re: [Kicad-developers] [PATCH] Setting grid line sizes and density

2017-02-16 Thread John Beard
Hi Orson,

Thank you very much for checking! I see you found
mydeliberate...last minute mistakes!

That's very strage about the enter thing, I certainly tried it before
but now I see a segfault as well. I will try to work out how to unbind
from a lambda, as I suspect you are right.

Cheers,

John

On Fri, Feb 17, 2017 at 12:23 AM, Maciej Sumiński
 wrote:
> Hi John,
>
> Thank you very much, I see lots of great improvements here. Your branch
> is a good candidate for merge, but there is one problem. If I type a
> value in INCREMENT_TEXT_CTRL and hit Enter, pcbnew segfaults. I suspect
> that kill focus event handler is executed after the window is gone, but
> I am not fully sure.
>
> There are also a few minor patches, see the attachments.
>
> Cheers,
> Orson
>
> On 02/16/2017 07:01 AM, John Beard wrote:
>> Hi,
>>
>> Here is a branch with a set of patches to add the ability to set the
>> grid line/dot size and the minimum grid spacing (to avoid very dense
>> grids when using thicker lines).
>>
>> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/grid_dot_size
>>
>> This also includes moving the grid display settings our of the Set
>> Grid dialog in to the Display Preferences dialog. As an item in the
>> Dimensions menu, Set Grid isn't really a suitable place, especially as
>> there are now more options.
>>
>> This was a fairly annoying change to make in the code, as the current
>> GAL properties structure was only consumed by the OpenGL GAL canvas.
>> However, now, all GAL canvases have access to it, which opens the way
>> for more display options to be added easily (though if the Display
>> Options dialog gets more options, we might need to put some tabs in,
>> like Eeschema).
>>
>> There are also some supporting infrastructure changes:
>>
>> * A class to bind a text entry and spin buttons together without a lot
>> of verbose repeating of event bindings.
>> * Utility functions to use a mapping table to decouple internal enum
>> values from the values written into config files and radio button
>> selections
>> * Over-visible header causing slow recompilation, hidden a bit better
>>
>> Cheers,
>>
>> John
>>
>> ___
>> 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: 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] [PATCH] Setting grid line sizes and density

2017-02-16 Thread John Beard
Hi Orson,

I have modified the incrementer control to unbind from the kill focus
event. This meant I couldn't use a lambda - if you know how to unbind
from a lambda without a lot of messing around, please let me know!

I have also made it so the increment GetValue will update the value
before returning, otherwise things like Enter in a dialog can do an
end run and not sanitise/update the value before returning.

I have integrated your mods into the patch sequence in the appropriate
places and rebased the branch onto master.

Cheers,

John

On Fri, Feb 17, 2017 at 12:27 AM, John Beard  wrote:
> Hi Orson,
>
> Thank you very much for checking! I see you found
> mydeliberate...last minute mistakes!
>
> That's very strage about the enter thing, I certainly tried it before
> but now I see a segfault as well. I will try to work out how to unbind
> from a lambda, as I suspect you are right.
>
> Cheers,
>
> John
>
> On Fri, Feb 17, 2017 at 12:23 AM, Maciej Sumiński
>  wrote:
>> Hi John,
>>
>> Thank you very much, I see lots of great improvements here. Your branch
>> is a good candidate for merge, but there is one problem. If I type a
>> value in INCREMENT_TEXT_CTRL and hit Enter, pcbnew segfaults. I suspect
>> that kill focus event handler is executed after the window is gone, but
>> I am not fully sure.
>>
>> There are also a few minor patches, see the attachments.
>>
>> Cheers,
>> Orson
>>
>> On 02/16/2017 07:01 AM, John Beard wrote:
>>> Hi,
>>>
>>> Here is a branch with a set of patches to add the ability to set the
>>> grid line/dot size and the minimum grid spacing (to avoid very dense
>>> grids when using thicker lines).
>>>
>>> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/grid_dot_size
>>>
>>> This also includes moving the grid display settings our of the Set
>>> Grid dialog in to the Display Preferences dialog. As an item in the
>>> Dimensions menu, Set Grid isn't really a suitable place, especially as
>>> there are now more options.
>>>
>>> This was a fairly annoying change to make in the code, as the current
>>> GAL properties structure was only consumed by the OpenGL GAL canvas.
>>> However, now, all GAL canvases have access to it, which opens the way
>>> for more display options to be added easily (though if the Display
>>> Options dialog gets more options, we might need to put some tabs in,
>>> like Eeschema).
>>>
>>> There are also some supporting infrastructure changes:
>>>
>>> * A class to bind a text entry and spin buttons together without a lot
>>> of verbose repeating of event bindings.
>>> * Utility functions to use a mapping table to decouple internal enum
>>> values from the values written into config files and radio button
>>> selections
>>> * Over-visible header causing slow recompilation, hidden a bit better
>>>
>>> Cheers,
>>>
>>> John
>>>
>>> ___
>>> 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: 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] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-02-16 Thread John Beard
Hi Wayne,

I added some new profiles for the INPUTSTREAM_LINE_READER.

The results are very surprising to me. In debug and release mode,
using INPUTSTREAM_LINE_READER with a wxInputFileStream is around 200
times (:-O) slower than a straight std::ifstream, taking over two
seconds to read a 6.5MB short-lined file that std::ifstream can do in
<10ms.

I wonder if there's something I've missed here, as I can't believe
it's truly that slow.

I've pushed the benchmark to Launchpad for those who are interested:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/io_benchmark

As for your note about having a generic stream version, yes, that's
more flexible and we should aim for that, if we were to provide a
std::istream LINE_READER. I just did ifstream as a test to keep things
clear and ensure a sensible comparison.

As I said, the current performance is "OK", and if we want to limit
line lengths, we probably can't get that for free, anyway.

I understand the desire to not read infinite lines, but at least in my
tests, the std:ifstream method, which has no limit for that, can deal
with a 1GB file of a single line in about 300ms. Obviously it's all in
disk cache, and you have to pay the allocation for it when reading
into the buffer.

All the existing LINE_READER explode with IO_ERROR on that file since
it's too long for them.

Cheers,

John

On Fri, Feb 17, 2017 at 5:56 AM, Wayne Stambaugh  wrote:
> John,
>
> It would have been nice if you would have benchmarked wxFileInputStream
> as well.  There already is an INPUTSTREAM_LINE_READER object which takes
> a pointer to wxInputStream object.  I'm curious how it stacks up against
> the std::ifstream.  There are some interesting wxInputStream objects
> that could prove useful.
>
> I think ifstream wasn't used in case there are really long lines which
> there can be if you have text objects with lots of long multiple line
> strings in your files.  I'm ok with adding a LINE_READER the wraps
> istream objects.  It's fairly trivial to change LINE_READER types.  It
> might be a bit more flexible if you just provided an ISTREAM_LINE_READER
> that take any istream derived object rather than write a separate
> LINE_READER for each istream derivative.
>
> Cheers,
>
> Wayne
>
> On 2/16/2017 8:43 AM, John Beard wrote:
>> Hi,
>>
>> I was trying to profile the eeschema slow library loads, and I got a
>> bit distracted by RICHIO's FILE_LINE_READER.
>>
>> Internally, it uses a very tight loop of reading single chars at a
>> time from a file descriptor, which looks inefficient. I wrote a
>> benchmarker to compare RICHIO against std::ifstream and a new
>> LINE_READER implementation, backed by std::ifstream. operf confirms
>> that most of the time in RICHIO burned in the ReadLine() function
>> itself.
>>
>> The results were that RICHIO (in debug mode) is consistently 4-7 times
>> slower than using std::ifstream, when reading eeschema library text
>> files (so relatively short lines). Compiling the release version
>> improved RICHIOs speed more than std::ifstream's, but it is still
>> around 3 times slower than std::ifstream.
>>
>> For files with 1k lines, the slowdown is about 30 times (!) in debug
>> and 14 times in release mode, so significantly worse. Few files read
>> line-wise by Kicad look like that, however.
>>
>> Avoiding reconstructing the stream/LINE_READER each time doesn't have
>> much of an effect in any case.
>>
>> Is there a particular reason why STL streams are not used in RICHIO?
>> The only thing I think the example ifstream implementation can't do is
>> catching over long lines, but that's only used in one place: the VRML
>> parser, which hardcodes an 8MB limit. ifstream could do this, but not
>> with the simple getline function.
>>
>> This performance doesn't appear to be a major bottleneck for me, but
>> it does seem a shame to throw away (charitably) two thirds of file
>> read speeds (and uncharitably, up to 97% in odd cases) if there is no
>> particular reason to do so.
>>
>> As an aside, RICHIO appears to allocate twice as many times as
>> std::ifstream when reading the same data, for the roughly the same
>> amount of memory in total.
>>
>> Anyway, I thought I'd share this finding! Please find attached the
>> benchmark program, such as it is.
>>
>> Cheers,
>>
>> John
>>
>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : http

Re: [Kicad-developers] Pushed new component selector

2017-02-19 Thread John Beard
On Sun, Feb 19, 2017 at 10:46 AM, Chris Pavlina  wrote:
> I just pushed the revamped component selector.
>
> Please, test thoroughly.

Hi Chris,

I get a bunch of asserts and then a crash on trying to place a part on
an empty sheet. This on Linux on a few pull from LP.

I have opened a bug for it to track it separately from any general
discussion of the new feature:

https://bugs.launchpad.net/kicad/+bug/1665982

Cheers,

John

___
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] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-02-19 Thread John Beard
On Sat, Feb 18, 2017 at 1:08 AM, Nox  wrote:
> What about wxFFileInputStream instead of wxFileInputStream?
>

wxFFileInputStream appears to be about 5-6 times faster than
wxFileInputStream, but that's still much much slower than RICHIO or
std::ifstream.

$ qa/io_benchmark/io_benchmark /tmp/all.lib 2
IO Bench Mark Util
  Benchmark file: /tmp/all.lib
  Repetitions:2
std::fstream  317858 lines, acc: 25103384 in 16 ms
std::fstream, reused  317858 lines, acc: 25103384 in 16 ms
RICHIO317858 lines, acc: 25103384 in 91 ms
RICHIO, reused317858 lines, acc: 25103384 in 90 ms
New fstream IO317858 lines, acc: 25103384 in 19 ms
New fstream IO, reused317858 lines, acc: 25103384 in 19 ms
wxIStream 317858 lines, acc: 25103384 in 3558 ms
wxIStream, reused 317858 lines, acc: 25103384 in 3429 ms
wxFFIStream   317858 lines, acc: 25103384 in 589 ms
wxFFIStream, reused   317858 lines, acc: 25103384 in 602 ms

___
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] [PATCH] Refactor COMMON_ACTIONS into a base and derived class

2017-02-20 Thread John Beard
HI Orson,

I think that sounds like a sensible idea. Having a huge central list
of actions has a bit of a code smell for me, as it's a big header than
then needs including everywhere. Smaller lists that are included along
with their tool's headers (if needed), or even actions that are
totally hidden in implementations for when the action only works
during an interactive tool feels much nicer.

The file now called common_actions.cpp (maybe pcb_actions.cpp or
something in future) would need to include most of these headers so it
can still map legacy event IDs, but that's how it should be - a file
that needs lots, includes lots.

Cheers,

John


On Mon, Feb 20, 2017 at 4:58 PM, Maciej Sumiński
 wrote:
> Hi Jon,
>
> I see the point of your patch, as COMMON_ACTIONS are now a bit misused.
> They should not keep majority of the TOOL_ACTIONs, as many of them are
> pcbnew specific, but there are still actions that will be shared with
> other applications (e.g. zoom & grid control, move/rotate/flip).
>
> For some time I was also wondering whether it would not be better to
> move the actions to their corresponding tools, as is done e.g. in
> pcbnew/router/router_tool.cpp (ACT_* objects), and leave only truly
> generic actions in {COMMON,PCB}_ACTIONS.
>
> What do you think about splitting the current set to PCB_ACTIONS and
> COMMON_ACTIONS, perhaps moving some of them to the tools source files?
>
> Regards,
> Orson
>
> On 02/17/2017 04:56 AM, Jon Evans wrote:
>> Hi all,
>>
>> More preparation for GerbView GAL port: this patch pulls a virtual ACTIONS
>> class out of pcbnew and renames the COMMON_ACTIONS to PCB_ACTIONS for
>> clarity.
>>
>> Best,
>> Jon
>>
>>
>>
>> ___
>> 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: 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


[Kicad-developers] [PATCH] Move bitmaps.h out of base_struct.h

2017-02-20 Thread John Beard
Hi,

Here is a patch to prevent bitmaps.h being included by nearly every
file in KiCad via base_struct.h.

Prior to this commit, simply touching bitmaps.h would require the
recompilation of over 400 files for pcbnew alone, not counting the
bitmap files themselves, which are fairly quick to build.

After this commit, only 166 files are rebuilt, for all targets, and 46
for just pcbnew and its deps, which is a pretty handy speedup of just
under 90% by file count!

Many of the new includes of bitmaps.h will disappear along with legacy
which will further shrink the impact of touching bitmaps.h.

An additional benefit is that the icon list is now in its own header,
which makes it easier to automate in future.

Cheers,

John
From b72cbf3523c31e6e8af42d514beaa54b6ef5dd2e Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Mon, 20 Feb 2017 20:20:39 +0800
Subject: [PATCH] Move bitmaps.h out of base_struct.h

bitmaps.h was included in nearly every file in the project due to it
being included by base_struct.h

Only about 130 files actually use the XPM definitions defined there, and
many of those already included bitmaps.h themselves, or via
menu_helpers.h. However, touching bitmaps.h would result in over 400
rebuilt files for pcbnew alone.

This commit moves the bitmap-related types like BITMAT_DEF out to a new
header, which is still included by base_struct.h, which is less
avoidable for now, it's it's used in the interface.

The icon list is still in bitmaps.h. This has the side effect that's
it's now easier to automatically generate this file.

Many classes in pcbnew and eeschema needed some functions moved
to the implementaitons from the headers too.
---
 3d-viewer/3d_cache/dialogs/panel_prev_model.cpp|  1 +
 3d-viewer/3d_canvas/eda_3d_canvas.cpp  |  1 +
 .../3d_viewer/dialogs/dialog_3D_view_option.cpp|  1 +
 3d-viewer/3d_viewer/eda_3d_viewer.cpp  |  1 +
 cvpcb/cvpcb_mainframe.cpp  |  1 +
 eeschema/controle.cpp  |  1 +
 eeschema/dialogs/dialog_annotate.cpp   |  1 +
 eeschema/dialogs/dialog_erc.cpp|  1 +
 eeschema/dialogs/dialog_lib_edit_pin.cpp   |  1 +
 eeschema/hierarch.cpp  |  1 +
 eeschema/lib_arc.cpp   |  7 +++
 eeschema/lib_arc.h |  2 +-
 eeschema/lib_circle.cpp|  7 +++
 eeschema/lib_circle.h  |  2 +-
 eeschema/lib_field.cpp |  7 +++
 eeschema/lib_field.h   |  2 +-
 eeschema/lib_polyline.cpp  |  7 +++
 eeschema/lib_polyline.h|  2 +-
 eeschema/lib_rectangle.cpp |  7 +++
 eeschema/lib_rectangle.h   |  2 +-
 eeschema/lib_text.cpp  |  7 +++
 eeschema/lib_text.h|  2 +-
 eeschema/sch_bitmap.cpp|  8 +++
 eeschema/sch_bitmap.h  |  2 +-
 eeschema/sch_bus_entry.cpp |  6 ++
 eeschema/sch_bus_entry.h   |  2 +-
 eeschema/sch_component.cpp |  7 +++
 eeschema/sch_component.h   |  2 +-
 eeschema/sch_field.cpp |  1 +
 eeschema/sch_junction.cpp  |  6 ++
 eeschema/sch_junction.h|  2 +-
 eeschema/sch_marker.cpp|  7 +++
 eeschema/sch_marker.h  |  2 +-
 eeschema/sch_no_connect.cpp|  7 +++
 eeschema/sch_no_connect.h  |  2 +-
 eeschema/sch_sheet.cpp |  6 ++
 eeschema/sch_sheet.h   |  4 +-
 eeschema/sch_sheet_pin.cpp |  6 ++
 eeschema/sch_text.cpp  | 25 
 eeschema/sch_text.h|  8 +--
 eeschema/tool_lib.cpp  |  1 +
 eeschema/tool_sch.cpp  |  1 +
 eeschema/viewlib_frame.cpp |  1 +
 gerbview/gerbview_frame.cpp|  1 +
 gerbview/toolbars_gerber.cpp   |  1 +
 include/base_struct.h  |  2 +-
 include/bitmap_types.h | 72 ++
 include/bitmaps.h  | 42 +
 kicad/mainframe.cpp|  1 +
 pagelayout_editor/pl_editor_frame.cpp  |  1 +
 pcbnew/class_dimension.cpp |  7 +++
 pcbnew/class_dimension.h   |  2 +-
 pcbnew/class_drawsegment.cpp   |  7 +++
 pcbnew/class_drawsegment.h |  2 +-
 pcbne

[Kicad-developers] [PATCH] Fix layer color swatches in Linux

2017-02-21 Thread John Beard
Hi,

These patches fix the layer/render widget swatches in Linux under new
GTK+ toolkits. Fix for: https://bugs.launchpad.net/kicad/+bug/1605411

The patches remove the "button" nature of the swatches, since they
weren't actually actuated by a single click, so the button affordance
was misleading anyway. Also on OSX, the button was invisible, so it
just looked like a flat swatch anyway.

* Linux as it was:
https://drive.google.com/file/d/0BxVhl5qZbpYoZlZPeXV1Q0ttT2s/view
* OSX as it was:
https://launchpadlibrarian.net/274428737/Screen%20Shot%202016-07-22%20at%2019.40.14.png
* Linux after this patch: see attachment

This is followed by a refactor to pull the swatch logic out of the
layer widget into common, where it can be used by other clients, for
example the eeschema display color dialog, if wanted.

Patch #3 is a simple replacement of old WX Connect with Bind for
consistency in that file.

Cheers,

John
From 92185ffb58305a7975d96c8a80c641ccb6cf80a4 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 21 Feb 2017 20:43:02 +0800
Subject: [PATCH 3/3] Use Bind rather than Connect in LAYER_WIDGET

Bind is safer than Connect. Bind is more flexible and has a more concise
signature.
---
 pcbnew/layer_widget.cpp | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index 0027a99d3..372be5b9d 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -342,7 +342,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
 wxStaticBitmap* sbm = new wxStaticBitmap( m_LayerScrolledWindow, encodeId( col, aSpec.id ),
 useAlternateBitmap(aRow) ? *m_BlankAlternateBitmap : *m_BlankBitmap,
 wxDefaultPosition, m_BitmapSize );
-sbm->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
+sbm->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
 m_LayersFlexGridSizer->wxSizer::Insert( index+col, sbm, 0, flags );
 
 // column 1 (COLUMN_COLORBM)
@@ -357,7 +357,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
 col = COLUMN_COLOR_LYR_CB;
 wxCheckBox* cb = new wxCheckBox( m_LayerScrolledWindow, encodeId( col, aSpec.id ), wxEmptyString );
 cb->SetValue( aSpec.state );
-cb->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( LAYER_WIDGET::OnLayerCheckBox ), NULL, this );
+cb->Bind( wxEVT_COMMAND_CHECKBOX_CLICKED, &LAYER_WIDGET::OnLayerCheckBox, this );
 cb->SetToolTip( _( "Enable this for visibility" ) );
 m_LayersFlexGridSizer->wxSizer::Insert( index+col, cb, 0, flags );
 
@@ -365,7 +365,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
 col = COLUMN_COLOR_LYRNAME;
 wxStaticText* st = new wxStaticText( m_LayerScrolledWindow, encodeId( col, aSpec.id ), aSpec.rowName );
 shrinkFont( st, m_PointSize );
-st->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
+st->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
 st->SetToolTip( aSpec.tooltip );
 m_LayersFlexGridSizer->wxSizer::Insert( index+col, st, 0, flags );
 }
@@ -403,8 +403,7 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
 aSpec.rowName, wxDefaultPosition, wxDefaultSize, wxALIGN_LEFT );
 shrinkFont( cb, m_PointSize );
 cb->SetValue( aSpec.state );
-cb->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED,
-wxCommandEventHandler( LAYER_WIDGET::OnRenderCheckBox ), NULL, this );
+cb->Bind( wxEVT_COMMAND_CHECKBOX_CLICKED, &LAYER_WIDGET::OnRenderCheckBox, this );
 cb->SetToolTip( aSpec.tooltip );
 m_RenderFlexGridSizer->wxSizer::Insert( index+col, cb, 0, flags );
 }
@@ -501,8 +500,7 @@ LAYER_WIDGET::LAYER_WIDGET( wxWindow* aParent, wxWindow* aFocusOwner, int aPoint
 m_BitmapSize = wxSize(m_BlankBitmap->GetWidth(), m_BlankBitmap->GetHeight());
 
 // trap the tab changes so that we can call passOnFocus().
-m_notebook->Connect( -1, wxEVT_COMMAND_NOTEBOOK_PAGE_CHANGED,
-wxNotebookEventHandler( LAYER_WIDGET::OnTabChange ), NULL, this );
+m_notebook->Bind( wxEVT_COMMAND_NOTEBOOK_PAGE_CHANGED, &LAYER_WIDGET::OnTabChange, this );
 
 Layout();
 }
-- 
2.11.0

From 5230a50e22cddd52dceddd9fdc98c16b7f74f5cf Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 21 Feb 2017 20:31:19 +0800
Subject: [PATCH 2/3] Move layer/render swatches to own class

This introduces COLOR_SWATCH, which is a reusable
widget that shows a color swatch and can invoke the colour picker
when duble/middle clicked.

It uses it's own wxCommandEvent to signal the change.

This makes the layer widget simpler internally, and also allows other
code to show identical swatches if needed.
---
 common/CMakeL

Re: [Kicad-developers] [PATCH] Fix layer color swatches in Linux

2017-02-21 Thread John Beard
wer will give also the answer to "Why can't ours".
>> > > > >
>> > > > > For me the answer is not obvious, but my opinion is:
>> > > > > the primary purpose of the layer manager is to select the
>> > working layer.
>> > > > > Therefore the single left click should change the layer,
>> > whatever you are clicking.
>> > > > >
>> > > > > If the primary purpose of the layer manager is to select the
>> > color, then a single left click should
>> > > > > open the color dialog.
>> > > >
>> > > > This isn't "the layer manager", it's a color well IN the layer
>> > manager.
>> > > >
>> > > > >
>> > > > > If the single left click opens the dialog, be sure many times,
>> > when clicking inside the layer
>> > > > > manager, you will open the dialog, instead of changing the
>> > active layer, especially if you are too
>> > > > > busy clicking on the window to verify the exact position of
>> > the mouse.
>> > > > > (It frequently happened to me when the single click was
>> > opening the dialog)
>> > > > >
>> > > > > A wise decision is never obvious.
>> > > > > (A reason like "other applications do that" is not necessary a
>> > good reason.
>> > > > > Each application has its constraints and its compromises)
>> > > >
>> > > > YES IT IS. Consistency is the single most important way to make an
>> > > > interface understandable. A box containing a color is a color
>> > well, and
>> > > > color wells behave the exact same way in everything that isn't
>> > kicad.
>> > > >
>> > > > Next you'll tell me "File->Open opening a file in other
>> > applications is
>> > > > not a good reason it shouldn't display a photo of a llama in 
>> > KiCad".
>> > > >
>> > > > >
>> > > > > >
>> > > > > >>
>> > > > > >> On 2/21/2017 8:04 AM, John Beard wrote:
>> > > > > >>> Hi,
>> > > > > >>>
>> > > > > >>> These patches fix the layer/render widget swatches in
>> > Linux under new
>> > > > > >>> GTK+ toolkits. Fix for:
>> > https://bugs.launchpad.net/kicad/+bug/1605411
>> > <https://bugs.launchpad.net/kicad/+bug/1605411>
>> > > > > >>>
>> > > > > >>> The patches remove the "button" nature of the swatches,
>> > since they
>> > > > > >>> weren't actually actuated by a single click, so the button
>> > affordance
>> > > > > >>> was misleading anyway. Also on OSX, the button was
>> > invisible, so it
>> > > > > >>> just looked like a flat swatch anyway.
>> > > > > >>>
>> > > > > >>> * Linux as it was:
>> > > > > >>>
>> > https://drive.google.com/file/d/0BxVhl5qZbpYoZlZPeXV1Q0ttT2s/view
>> > <https://drive.google.com/file/d/0BxVhl5qZbpYoZlZPeXV1Q0ttT2s/view>
>> > > > > >>> * OSX as it was:
>> > > > > >>>
>> > 
>> > https://launchpadlibrarian.net/274428737/Screen%20Shot%202016-07-22%20at%2019.40.14.png
>> > 
>> > <https://launchpadlibrarian.net/274428737/Screen%20Shot%202016-07-22%20at%2019.40.14.png>
>> > > > > >>> * Linux after this patch: see attachment
>> > > > > >>>
>> > > > > >>> This is followed by a refactor to pull the swatch logic
>> > out of the
>> > > > > >>> layer widget into common, where it can be used by other
>> > clients, for
>> > > > > >>> example the eeschema display color dialog, if wanted.
>> > > > > >>>
>> > > > > >>> Patch #3 is a simple replacement of old WX Connect with
>> > Bind f

Re: [Kicad-developers] [PATCH] Select filter dialog in GAL

2017-02-22 Thread John Beard
Hi,

I have rebased this branch over the new PCB_ACTION changes.

Cheers,

John

On Sat, Feb 11, 2017 at 5:14 PM, John Beard  wrote:
> Hi,
>
> Here is a branch with a GAL action to invoke the block select dialog
> and filter the current selection based on the results:
>
> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/select_filter
>
> The first commit refactors the dialog so it can be reached by the GAL
> too, the second plumbs it in to the GAL selection tool. I have used
> 'Shift-F' to invoke the dialog.
>
> Possibly controversial: use of a compilation firewall (PImpl
> idiom/opaque pointer) to allow SELECTION_TOOL to keep an instance of
> the dialog options for persistance between invocations. As the class
> is a nested class, it can't be simply forward declared.
>
> If it is preferred to not use PImpl in this way (since it's not
> commonly done in Kicad), I can redo it so the options class is not
> nested and forward declare it.
>
> Cheers,
>
> John

___
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] [PATCH] Select filter dialog in GAL

2017-02-22 Thread John Beard
Hi Orson,

Yes, I am aware that the existing dialog is not very advanced, but it
was a fairly simple job to use it in GAL, as you see in the patch, and
the code required is not extensive, and is easily replaced by a proper
filtering framework in future.
It certainly needs some more thought put into it in future, but at
least this way GAL is at least as capable as legacy. I was under no
illusions that this code is anything more than a temporary solution!

As for the menu, I think we could just use the NotEmpty
SELECTION_CONDITION to show the sub-menu on any selection, and the
items in the menu that don't apply will be greyed out as currently?
Perhaps the CONTEXT_MENU::HasEnabledItems could work too to delegate
the decision to the enablement functions of each item?

Cheers,

John

On Thu, Feb 23, 2017 at 1:11 AM, Maciej Sumiński
 wrote:
> Hi Jon,
>
> Thank you very much for you help. Actually we had another idea for the
> selection filter [1]. We would like to have a panel in the layer widget
> area and filter the selection all the time, instead of after invoking a
> dialog.
>
> As your patch is ready at the moment, we can merge it unless there are
> objections. It is likely that is going to be replaced in the future, but
> for now it should be fine. The most important point is the patch will
> reduce complaints about features missing in GAL, and for that I am
> sincerely thankful.
>
> One problem with the current implementation is that the Selection Filter
> dialog can be invoked only when the user right-clicks on a connected
> item, so he gets the options for selecting connected items.
>
> The same problem applies to 'select items in the same sheet', so perhaps
> these two entries could be moved somewhere else.
>
> Regards,
> Orson
>
> 1. http://www.ohwr.org/attachments/4646/selection_filter.pdf
>
> On 02/22/2017 03:41 PM, John Beard wrote:
>> Hi,
>>
>> I have rebased this branch over the new PCB_ACTION changes.
>>
>> Cheers,
>>
>> John
>>
>> On Sat, Feb 11, 2017 at 5:14 PM, John Beard  wrote:
>>> Hi,
>>>
>>> Here is a branch with a GAL action to invoke the block select dialog
>>> and filter the current selection based on the results:
>>>
>>> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/select_filter
>>>
>>> The first commit refactors the dialog so it can be reached by the GAL
>>> too, the second plumbs it in to the GAL selection tool. I have used
>>> 'Shift-F' to invoke the dialog.
>>>
>>> Possibly controversial: use of a compilation firewall (PImpl
>>> idiom/opaque pointer) to allow SELECTION_TOOL to keep an instance of
>>> the dialog options for persistance between invocations. As the class
>>> is a nested class, it can't be simply forward declared.
>>>
>>> If it is preferred to not use PImpl in this way (since it's not
>>> commonly done in Kicad), I can redo it so the options class is not
>>> nested and forward declare it.
>>>
>>> Cheers,
>>>
>>> John
>>
>> ___
>> 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: 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] [PATCH] Fix layer color swatches in Linux

2017-02-22 Thread John Beard
Hi Wayne,

I rebased the patches onto the COLOR4D work.

I've also added a further refactor than generalises the row indicator
icons, which removes a bit more logic from the layer widgets, and can
be reused as a generic switchable icon for other purposes if needed (I
had an Inkscape-style "eye" for the visibility checkbox in mind, but
it's not limited to two states, or to just the layer widget).

I've put both the COLOR_SWATCH and INDICATOR_ICONS in common/widgets.

There's a lot more scope for tidying this widget, and other UI ideas
abound. This should help a little to tidying up, and at least makes it
usable and consistent across platforms for v5.

Cheers,

John

On Tue, Feb 21, 2017 at 11:05 PM, John Beard  wrote:
> On Tue, Feb 21, 2017 at 10:41 PM, Wayne Stambaugh  
> wrote:
>> I forgot to mention that this will likely clash with the COLOR4D work
>> that is already in progress so we will have to coordinate the changes.
>
> Happy to rebase as needed.
>
>> On 2/21/2017 9:35 AM, Wayne Stambaugh wrote:
>>> My only issue with this change is that the tooltip letting the user know
>>> that a left button double click or a middle button click would allow
>>> them to change the color is gone.
>
> The tooltip is still there for me (Linux). I specifically checked it,
> as I think it's a useful thing, since there's no other affordance on
> the swatches to tell you what to do. I hope it's not platform specfic
> as to whether the swatch wxStaticBitmap or the COLOR_SWATCH panel
> provide the tooltip.
>
> Cheers,
>
> John
From 8dc00c8c102b644768c8fda411daa41b6af075e5 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 22 Feb 2017 20:34:23 +0800
Subject: [PATCH 4/4] Break row indicators out to own class

The introduces INDICATOR_ICON, which is a very simple class holding a
bitmap that can toggle on or off.

The ICON_PROVIDER class then provides icons to INDICATOR_ICONS, which
means the class can be used for more than just row indicators.

A default row icon provider is also provided for use in the standard row
selector.
---
 common/CMakeLists.txt|   1 +
 common/widgets/indicator_icon.cpp| 191 +++
 gerbview/class_gerbview_layer_widget.cpp |  21 
 gerbview/class_gerbview_layer_widget.h   |   7 --
 include/widgets/indicator_icon.h | 140 ++
 pcbnew/layer_widget.cpp  | 152 ++--
 pcbnew/layer_widget.h|  13 ++-
 7 files changed, 375 insertions(+), 150 deletions(-)
 create mode 100644 common/widgets/indicator_icon.cpp
 create mode 100644 include/widgets/indicator_icon.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index f2c6f04b3..349f6bc01 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -166,6 +166,7 @@ set( COMMON_WIDGET_SRCS
 widgets/widget_hotkey_list.cpp
 widgets/two_column_tree_list.cpp
 widgets/footprint_preview_panel.cpp
+widgets/indicator_icon.cpp
 )
 
 set( COMMON_PAGE_LAYOUT_SRCS
diff --git a/common/widgets/indicator_icon.cpp b/common/widgets/indicator_icon.cpp
new file mode 100644
index 0..c365c390c
--- /dev/null
+++ b/common/widgets/indicator_icon.cpp
@@ -0,0 +1,191 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include 
+
+
+INDICATOR_ICON::INDICATOR_ICON( wxWindow* aParent,
+  ICON_PROVIDER& aIconProvider,
+  ICON_ID aInitialIcon, int aID ):
+wxPanel( aParent, aID ),
+m_iconProvider( aIconProvider ),
+m_currentId( aInitialIcon )
+{
+auto sizer = new wxBoxSizer( wxHORIZONTAL );
+SetSizer( sizer );
+
+const wxBitmap& initBitmap = m_iconProvider.GetIndicatorIcon( m_currentId );
+
+m_bitmap = new wxStaticBitmap( this, aID,
+   

Re: [Kicad-developers] [PATCH] Fix layer color swatches in Linux

2017-02-22 Thread John Beard
Hi,

Please use this branch instead:

https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/swatches

The patches previously omitted the new colour picker in the rebase
(which is kind of important!).

Cheers,

John

On Thu, Feb 23, 2017 at 2:18 AM, John Beard  wrote:
> Hi Wayne,
>
> I rebased the patches onto the COLOR4D work.
>
> I've also added a further refactor than generalises the row indicator
> icons, which removes a bit more logic from the layer widgets, and can
> be reused as a generic switchable icon for other purposes if needed (I
> had an Inkscape-style "eye" for the visibility checkbox in mind, but
> it's not limited to two states, or to just the layer widget).
>
> I've put both the COLOR_SWATCH and INDICATOR_ICONS in common/widgets.
>
> There's a lot more scope for tidying this widget, and other UI ideas
> abound. This should help a little to tidying up, and at least makes it
> usable and consistent across platforms for v5.
>
> Cheers,
>
> John
>
> On Tue, Feb 21, 2017 at 11:05 PM, John Beard  wrote:
>> On Tue, Feb 21, 2017 at 10:41 PM, Wayne Stambaugh  
>> wrote:
>>> I forgot to mention that this will likely clash with the COLOR4D work
>>> that is already in progress so we will have to coordinate the changes.
>>
>> Happy to rebase as needed.
>>
>>> On 2/21/2017 9:35 AM, Wayne Stambaugh wrote:
>>>> My only issue with this change is that the tooltip letting the user know
>>>> that a left button double click or a middle button click would allow
>>>> them to change the color is gone.
>>
>> The tooltip is still there for me (Linux). I specifically checked it,
>> as I think it's a useful thing, since there's no other affordance on
>> the swatches to tell you what to do. I hope it's not platform specfic
>> as to whether the swatch wxStaticBitmap or the COLOR_SWATCH panel
>> provide the tooltip.
>>
>> Cheers,
>>
>> John

___
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


[Kicad-developers] [PATCH] Update tool framework documention

2017-02-22 Thread John Beard
Hi,

In light of recent changes to the tool actions, here is an update to
the tool framework developer docs.

There are some minor changes to the tool itself, correcting a few
mistakes, and better TOC markdown.

Cheers,

John
From 24894cd080e98c6591e6b2ddaed1e02b88965404 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Thu, 23 Feb 2017 10:47:19 +0800
Subject: [PATCH] Update GAL tool documentation

Update to reference new PCB_ACTIONs (used to be COMMON_ACTION)

Also expand on use of BOARD_COMMIT.

Add doxygen TOC markup
---
 Documentation/development/tool-framework.md | 186 +---
 1 file changed, 144 insertions(+), 42 deletions(-)

diff --git a/Documentation/development/tool-framework.md b/Documentation/development/tool-framework.md
index 6211debb0..b7ede166a 100644
--- a/Documentation/development/tool-framework.md
+++ b/Documentation/development/tool-framework.md
@@ -7,8 +7,11 @@ GAL canvases.
 
 # Introduction # {#intro}
 
-The GAL framework provides a powerful method of easily adding tools to
-KiCad. A GAL "tool" is a class which provides one or more "actions"
+The GAL (Graphics Abstraction Layer) framework provides a powerful
+method of easily adding tools to KiCad. Compared to the older "legacy"
+canvas, GAL tools are more flexible, powerful and much easier to write.
+
+A GAL "tool" is a class which provides one or more "actions"
 to perform. An action can be a simple one-off action (e.g. "zoom in"
 or "flip object"), or an interactive process (e.g. "manually edit
 polygon points").
@@ -29,11 +32,11 @@ Some examples of tools in the Pcbnew GAL are:
   (pcbnew/tools/drawing_tool.cpp,pcbnew/tools/drawing_tool.h)
 * The zoom tool - allows the user to zoom in and out
 
-## Major parts of a tool
+# Major parts of a tool # {#major-parts}
 
 There are two main aspects to tools: the actions and the the tool class.
 
-### Tool actions
+## Tool actions {#tool-actions}
 
 The `TOOL_ACTION` class acts as a handle for the GAL framework to
 call on actions provided by tools. Generally, every action, interactive
@@ -61,7 +64,7 @@ or not, has a `TOOL_ACTION` instance. This provides:
 * A parameter, which allows different actions to call the same function
   with different effects, for example "step left" and "step right".
 
-### The tool class
+## The tool class {#tool-class}
 
 GAL tools inherit the `TOOL_BASE` class. A Pcbnew tool will generally
 inherit from `PCB_TOOL`, which is a `TOOL_INTERACTIVE`, which is
@@ -72,12 +75,17 @@ The tool class for a tool can be fairly lightweight - much of the
 functionality is inherited from the tool's base classes. These base
 classes provide access to several things, particularly:
 
-* Access to the `PCB_EDIT_FRAME`, which can be used to modify the
-  viewport, set cursors and status bar content, etc.
+* Access to the parent frame (a `wxWindow`, which can be used to
+  modify the viewport, set cursors and status bar content, etc.
+* Use the function `getEditFrame()`, where `T` is the frame
+  subclass you want. In `PCB_TOOL`, this is likely `PCB_EDIT_FRAME`.
 * Access to the `TOOL_MANAGER` which can be used to access other tools'
   actions.
-* Access to the `BOARD` object which is used to modify the PCB content.
-* Access to the `KIGFX::VIEW`, which is used to manipulate the GAL canvas.
+* Access to the "model" (some sort of `EDA_ITEM`) which backs the tool.
+* Access with  `getModel()`. In `PCB_TOOL`, the model type `T` is
+ `BOARD`, which can be used to access and modify the PCB content.
+* Access to the `KIGFX::VIEW` and `KIGFX::VIEW_CONTROLS`, which are
+  used to manipulate the GAL canvas.
 
 The major parts of tool's implementation are the functions used by the
 `TOOL_MANAGER` to set up and manage the tool:
@@ -107,7 +115,7 @@ The major parts of tool's implementation are the functions used by the
   by any other code, but are invoked by the tool manager's coroutine
   framework according to the `SetTransitions()` map.
 
- Interactive actions
+### Interactive actions {#interactive-actions}
 
 The action handlers for an interactive actions handle repeated actions
 from the tool manager in a loop, until an action indicating that the
@@ -153,7 +161,7 @@ a cursor change and by setting a status string.
 return 0;
 }
 
-### The tool menu
+## The tool menu {#tool-menu}
 
 Top level tools, i.e. tools that the user enters directly, usually
 provide their own context menu. Tools that are called only from other
@@ -184,7 +192,58 @@ this menu will trigger the action - there is no further action
 needed in your tool's event loop.
 
 
-# Tutorial: Adding a new tool
+## Commit objects {#commits}
+
+The `COMMIT` class manages changes to `EDA_ITEMS`, which combines
+changes on any number of items into a single undo/redo action.
+When editing PCBs, changes to the PCB are managed b

[Kicad-developers] [PATCH] Fix grid sizes in GAL modedit

2017-02-23 Thread John Beard
Hi,

The COMMON_TOOLS GAL tool wasn't registered in modedit, so the grid
setting didn't work.

Here's a patch. This fixes lp:1667264, possibly the tersest bug report
ever, with a grand total of 6 words and a colon, plus the copy-paste
version info.

Any bug reporters out there, pretty please use your words when
reporting bugs - in this case mentioning that it was GAL only would
have been a good pointer for a start!

Cheers,

John
From 8911c539669832d5b4e261acbbe48ca9ffa25b98 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Thu, 23 Feb 2017 22:02:18 +0800
Subject: [PATCH] Register COMMON_TOOLS in modedit

Without this, no actions from that tool will work, which includes things
like settings the grid size.

Fixes: lp:1667264
* https://bugs.launchpad.net/kicad/+bug/1667264
---
 pcbnew/moduleframe.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pcbnew/moduleframe.cpp b/pcbnew/moduleframe.cpp
index 1e6e7daed..99c45e249 100644
--- a/pcbnew/moduleframe.cpp
+++ b/pcbnew/moduleframe.cpp
@@ -58,7 +58,9 @@
 #include 
 
 #include 
+#include 
 #include 
+
 #include "tools/selection_tool.h"
 #include "tools/zoom_tool.h"
 #include "tools/edit_tool.h"
@@ -950,6 +952,7 @@ void FOOTPRINT_EDIT_FRAME::setupTools()
 
 drawPanel->SetEventDispatcher( m_toolDispatcher );
 
+m_toolManager->RegisterTool( new COMMON_TOOLS );
 m_toolManager->RegisterTool( new SELECTION_TOOL );
 m_toolManager->RegisterTool( new ZOOM_TOOL );
 m_toolManager->RegisterTool( new EDIT_TOOL );
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Add cancel interactive tool to GAL

2017-02-23 Thread John Beard
Hi,

This adds a new action to the common ACTIONS tools: cancelInteractive.

This is an action which can be called to terminate an ongoing
interactive tool, for example a drawing tool. The action is can be
used pretty much everywhere that the generic IsCancel() event can, but
also allows adding an entry to tool menus.

The router tool has the current behaviour with this action - it just
ends and rips up back to the last placed node - same as pressing Esc.
Undoing the whole track is a different issue entirely.

Cheers,

John
From c8d91e1012d039edeb0d862859748f4ee5f4451e Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 7 Feb 2017 00:00:51 +0800
Subject: [PATCH] Add cancel interactive tool action to GAL

This is used to provide menu entries that allows cancellation of
interactive drawing and routing tools without needing the keyboard.

It is provided in the drawing tools and the router tool.

The cancel event doesn't have any new functionality (e.g. track rip-up
for the PNS router - lp:1448460), this just adds it to the menu, where
it behaves the same as an Escape keypress.
---
 common/tool/actions.cpp   |  6 ++
 include/tool/actions.h|  3 +++
 pcbnew/router/router_tool.cpp | 14 +++---
 pcbnew/tools/drawing_tool.cpp | 24 +---
 pcbnew/tools/tool_event_utils.cpp |  6 ++
 pcbnew/tools/tool_event_utils.h   |  9 +
 6 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/common/tool/actions.cpp b/common/tool/actions.cpp
index ad7cbfb6e..2bd8661d3 100644
--- a/common/tool/actions.cpp
+++ b/common/tool/actions.cpp
@@ -4,6 +4,12 @@
 
 // These members are static in class ACTIONS: Build them here:
 
+// Generic Actions
+TOOL_ACTION ACTIONS::cancelInteractive( "common.Interactive.cancel",
+AS_GLOBAL, 0,   // ESC key is handled in the dispatcher
+_( "Cancel" ), _( "Cancel current tool" ),
+cancel_xpm, AF_NONE );
+
 // View Controls
 TOOL_ACTION ACTIONS::zoomIn( "common.Control.zoomIn",
 AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_ZOOM_IN ),
diff --git a/include/tool/actions.h b/include/tool/actions.h
index 8747ea1e1..114c7f0a7 100644
--- a/include/tool/actions.h
+++ b/include/tool/actions.h
@@ -44,6 +44,9 @@ public:
 
 virtual ~ACTIONS() {};
 
+// Generic actions
+static TOOL_ACTION cancelInteractive;
+
 // View controls
 static TOOL_ACTION zoomIn;
 static TOOL_ACTION zoomOut;
diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp
index e568f5ab7..024e12e86 100644
--- a/pcbnew/router/router_tool.cpp
+++ b/pcbnew/router/router_tool.cpp
@@ -52,6 +52,7 @@ using namespace std::placeholders;
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -257,6 +258,11 @@ public:
 m_widthMenu( aBoard ), m_zoomMenu( &aFrame ), m_gridMenu( &aFrame )
 {
 SetTitle( _( "Interactive Router" ) );
+
+Add( ACTIONS::cancelInteractive );
+
+AppendSeparator();
+
 Add( ACT_NewTrack );
 Add( ACT_EndTrack );
 //Add( ACT_AutoEndRoute );  // fixme: not implemented yet. Sorry.
@@ -636,7 +642,8 @@ void ROUTER_TOOL::performRouting()
 still_routing = m_router->FixRoute( m_endSnapPoint, m_endItem );
 break;
 }
-else if( evt->IsCancel() || evt->IsActivate() || evt->IsUndoRedo() )
+else if( TOOL_EVT_UTILS::IsCancelInteractive( *evt )
+ || evt->IsUndoRedo() )
 break;
 }
 
@@ -731,7 +738,7 @@ int ROUTER_TOOL::mainLoop( PNS::ROUTER_MODE aMode )
 // Main loop: keep receiving events
 while( OPT_TOOL_EVENT evt = Wait() )
 {
-if( evt->IsCancel() || evt->IsActivate() )
+if( TOOL_EVT_UTILS::IsCancelInteractive( *evt ) )
 {
 break; // Finish
 }
@@ -816,7 +823,8 @@ void ROUTER_TOOL::performDragging()
 if( m_router->FixRoute( m_endSnapPoint, m_endItem ) )
 break;
 }
-else if( evt->IsCancel() || evt->IsActivate() || evt->IsUndoRedo() )
+else if( TOOL_EVT_UTILS::IsCancelInteractive( *evt )
+ || evt->IsUndoRedo() )
 break;
 
 handleCommonEvents( *evt );
diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp
index 093af47f7..fea754ecd 100644
--- a/pcbnew/tools/drawing_tool.cpp
+++ b/pcbnew/tools/drawing_tool.cpp
@@ -136,6 +136,16 @@ DRAWING_TOOL::~DRAWING_TOOL()
 
 bool DRAWING_TOOL::Init()
 {
+auto activeToolFunctor = [ this ] ( const SELECTION& aSel ) {
+return m_mode != MODE::NONE;
+};
+
+auto& ctxMenu = m_menu.GetMenu();
+
+// cancel current toool goes in main context menu at the top if present
+ctxMenu.AddItem( ACTIONS::cancelInteractive, activeToolFunctor, 1000 );
+ctxMenu.AddSeparator( activeToolFunctor, 1000 );
+
 // Drawing type-specific options will be 

[Kicad-developers] [PATCH] Undo/redo points for zone fill actions (GAL)

2017-02-24 Thread John Beard
Hi,

Thiis is a patch to allow the zone fill/unfill actions to get undo and
redo points in GAL.

These functions could do with a refactor, perhaps, but this commit
doesn't attempt that, in order to keep the change clear.

Legacy doesn't have this function either, but once GAL makes the
points, legacy can use them as normal.

Cheers,

John
From d9d18cd02e54ac3098e5b3ca7af667aeed3ef46b Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 24 Feb 2017 20:26:43 +0800
Subject: [PATCH 1/4] When filling/unfilling zones, create undo points in GAL

This is a feature which is apparently not available in legacy, but the
undo points created in GAL do work in legacy mode.
---
 pcbnew/tools/pcb_editor_control.cpp | 28 
 1 file changed, 28 insertions(+)

diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp
index 76d118e7a..68692a79f 100644
--- a/pcbnew/tools/pcb_editor_control.cpp
+++ b/pcbnew/tools/pcb_editor_control.cpp
@@ -573,17 +573,24 @@ int PCB_EDITOR_CONTROL::ZoneFill( const TOOL_EVENT& aEvent )
 const auto& selection = selTool->GetSelection();
 RN_DATA* ratsnest = getModel()->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( auto item : selection )
 {
 assert( item->Type() == PCB_ZONE_AREA_T );
 
 ZONE_CONTAINER* zone = static_cast ( item );
+
+commit.Modify( zone );
+
 m_frame->Fill_Zone( zone );
 zone->SetIsFilled( true );
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Fill Zone" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -595,15 +602,22 @@ int PCB_EDITOR_CONTROL::ZoneFillAll( const TOOL_EVENT& aEvent )
 BOARD* board = getModel();
 RN_DATA* ratsnest = board->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( int i = 0; i < board->GetAreaCount(); ++i )
 {
 ZONE_CONTAINER* zone = board->GetArea( i );
+
+commit.Modify( zone );
+
 m_frame->Fill_Zone( zone );
 zone->SetIsFilled( true );
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Fill All Zones" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -616,17 +630,24 @@ int PCB_EDITOR_CONTROL::ZoneUnfill( const TOOL_EVENT& aEvent )
 const auto& selection = selTool->GetSelection();
 RN_DATA* ratsnest = getModel()->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( auto item : selection )
 {
 assert( item->Type() == PCB_ZONE_AREA_T );
 
 ZONE_CONTAINER* zone = static_cast( item );
+
+commit.Modify( zone );
+
 zone->SetIsFilled( false );
 zone->ClearFilledPolysList();
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Unfill Zone" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -638,15 +659,22 @@ int PCB_EDITOR_CONTROL::ZoneUnfillAll( const TOOL_EVENT& aEvent )
 BOARD* board = getModel();
 RN_DATA* ratsnest = board->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( int i = 0; i < board->GetAreaCount(); ++i )
 {
 ZONE_CONTAINER* zone = board->GetArea( i );
+
+commit.Modify( zone );
+
 zone->SetIsFilled( false );
 zone->ClearFilledPolysList();
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Unfill All Zones" ) );
+
 ratsnest->Recalculate();
 
 return 0;
-- 
2.11.0

___
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


[Kicad-developers] [PATCH] Draw zone hatching in GAL

2017-02-26 Thread John Beard
Hi,

Here's a patch to draw the zone hatching in GAL.

The hatching is already contained within the ZONE_CONTAINER objects,
so this patch just regenerates the hatching at opportune times, and
renders it when the zone is drawn.

Cheers,

John
From 9c9a512d0490c9b7776c4f54b625cd42acfbe070 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 24 Feb 2017 20:26:25 +0800
Subject: [PATCH] Draw zone hatching in GAL

This just uses the same hatch lines that are built into the zone as
legacy.

They are always drawn, even when there is a fill, as if the fill
doesn't reach the corners, the hatches can be seen.

Fixes: lp:1487043
* https://bugs.launchpad.net/kicad/+bug/1487043
---
 pcbnew/pcb_painter.cpp| 6 ++
 pcbnew/tools/drawing_tool.cpp | 1 +
 pcbnew/tools/point_editor.cpp | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp
index 24fa470f1..b0b170a8c 100644
--- a/pcbnew/pcb_painter.cpp
+++ b/pcbnew/pcb_painter.cpp
@@ -966,6 +966,12 @@ void PCB_PAINTER::draw( const ZONE_CONTAINER* aZone )
 m_gal->DrawPolyline( corners );
 corners.clear();
 }
+
+for( unsigned ic = 0; ic < polygon->m_HatchLines.size(); ic++ )
+{
+auto& hatchLine = polygon->m_HatchLines[ic];
+m_gal->DrawLine( hatchLine.m_Start, hatchLine.m_End );
+}
 }
 
 // Draw the filling
diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp
index fea754ecd..5bb24a429 100644
--- a/pcbnew/tools/drawing_tool.cpp
+++ b/pcbnew/tools/drawing_tool.cpp
@@ -1440,6 +1440,7 @@ int DRAWING_TOOL::drawZone( bool aKeepout, ZONE_MODE aMode )
 
 zone->Outline()->CloseLastContour();
 zone->Outline()->RemoveNullSegments();
+zone->Outline()->Hatch();
 
 if( !aKeepout )
 static_cast( m_frame )->Fill_Zone( zone.get() );
diff --git a/pcbnew/tools/point_editor.cpp b/pcbnew/tools/point_editor.cpp
index 7e91c7626..b62e30528 100644
--- a/pcbnew/tools/point_editor.cpp
+++ b/pcbnew/tools/point_editor.cpp
@@ -469,6 +469,8 @@ void POINT_EDITOR::updateItem() const
 outline->SetY( i, point.y );
 }
 
+outline->Hatch();
+
 break;
 }
 
-- 
2.11.0

___
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


[Kicad-developers] Ubuntu build commit IDs

2017-02-26 Thread John Beard
Hi,

Would it be possible to get the ubuntu builds tagged with a commit
hash? Version strings like  201702251416+7735~57~ubuntu16.10.1 are not
very useful if you want to know exactly which commit a build relates
to.

The exact commit is very useful when looking at a bug report from a
nightly. If a regression arises between one nightly and the next, the
date only approximately tells you where to start bisecting.

The Bzr-style rev number is almost entirely useless especially when
the build is also tagged with a date, and Bzr is dead and gone.
Date+git rev give you both an incrementing number for quick
comparisons of age of build, and the git commit is an unambiguous
pointer to the exact commit.

Nearly all "nightly" and "git tracker" pacakges (PPA, AUR, etc) put
the git hash in the build number, pretty much for this exact reason.

Cheers,

John

___
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] [PATCH] Undo/redo points for zone fill actions (GAL)

2017-02-26 Thread John Beard
Hi,

Sorry, that patch had a dependency on a previous commit in my patch stack.

Here's an updated one. PCB_EDITOR_CONTROL now inherits PCB_TOOL, not
TOOL_INTERACTIVE, so it can use the relevant BOARD_COMMIT constructor.

Cheers,

John

On Sat, Feb 25, 2017 at 2:50 PM, John Beard  wrote:
> Hi,
>
> Thiis is a patch to allow the zone fill/unfill actions to get undo and
> redo points in GAL.
>
> These functions could do with a refactor, perhaps, but this commit
> doesn't attempt that, in order to keep the change clear.
>
> Legacy doesn't have this function either, but once GAL makes the
> points, legacy can use them as normal.
>
> Cheers,
>
> John
From e30af04456d1607e69f21afddce0b2b170bd6d2c Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 24 Feb 2017 20:26:43 +0800
Subject: [PATCH] When filling/unfilling zones, create undo points in GAL

This is a feature which is apparently not available in legacy, but the
undo points created in GAL do work in legacy mode.
---
 pcbnew/tools/pcb_editor_control.cpp | 30 +-
 pcbnew/tools/pcb_editor_control.h   |  4 ++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp
index 7be6dcd4e..b9b95bde9 100644
--- a/pcbnew/tools/pcb_editor_control.cpp
+++ b/pcbnew/tools/pcb_editor_control.cpp
@@ -228,7 +228,7 @@ public:
 
 
 PCB_EDITOR_CONTROL::PCB_EDITOR_CONTROL() :
-TOOL_INTERACTIVE( "pcbnew.EditorControl" ),
+PCB_TOOL( "pcbnew.EditorControl" ),
 m_frame( nullptr )
 {
 m_placeOrigin = new KIGFX::ORIGIN_VIEWITEM( KIGFX::COLOR4D( 0.8, 0.0, 0.0, 1.0 ),
@@ -645,17 +645,24 @@ int PCB_EDITOR_CONTROL::ZoneFill( const TOOL_EVENT& aEvent )
 const auto& selection = selTool->GetSelection();
 RN_DATA* ratsnest = getModel()->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( auto item : selection )
 {
 assert( item->Type() == PCB_ZONE_AREA_T );
 
 ZONE_CONTAINER* zone = static_cast ( item );
+
+commit.Modify( zone );
+
 m_frame->Fill_Zone( zone );
 zone->SetIsFilled( true );
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Fill Zone" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -667,15 +674,22 @@ int PCB_EDITOR_CONTROL::ZoneFillAll( const TOOL_EVENT& aEvent )
 BOARD* board = getModel();
 RN_DATA* ratsnest = board->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( int i = 0; i < board->GetAreaCount(); ++i )
 {
 ZONE_CONTAINER* zone = board->GetArea( i );
+
+commit.Modify( zone );
+
 m_frame->Fill_Zone( zone );
 zone->SetIsFilled( true );
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Fill All Zones" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -688,17 +702,24 @@ int PCB_EDITOR_CONTROL::ZoneUnfill( const TOOL_EVENT& aEvent )
 const auto& selection = selTool->GetSelection();
 RN_DATA* ratsnest = getModel()->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( auto item : selection )
 {
 assert( item->Type() == PCB_ZONE_AREA_T );
 
 ZONE_CONTAINER* zone = static_cast( item );
+
+commit.Modify( zone );
+
 zone->SetIsFilled( false );
 zone->ClearFilledPolysList();
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Unfill Zone" ) );
+
 ratsnest->Recalculate();
 
 return 0;
@@ -710,15 +731,22 @@ int PCB_EDITOR_CONTROL::ZoneUnfillAll( const TOOL_EVENT& aEvent )
 BOARD* board = getModel();
 RN_DATA* ratsnest = board->GetRatsnest();
 
+BOARD_COMMIT commit( this );
+
 for( int i = 0; i < board->GetAreaCount(); ++i )
 {
 ZONE_CONTAINER* zone = board->GetArea( i );
+
+commit.Modify( zone );
+
 zone->SetIsFilled( false );
 zone->ClearFilledPolysList();
 ratsnest->Update( zone );
 getView()->Update( zone );
 }
 
+commit.Push( _( "Unfill All Zones" ) );
+
 ratsnest->Recalculate();
 
 return 0;
diff --git a/pcbnew/tools/pcb_editor_control.h b/pcbnew/tools/pcb_editor_control.h
index 3684a1b1e..5f8ba280b 100644
--- a/pcbnew/tools/pcb_editor_control.h
+++ b/pcbnew/tools/pcb_editor_control.h
@@ -25,7 +25,7 @@
 #ifndef PCB_EDITOR_CONTROL_H
 #define PCB_EDITOR_CONTROL_H
 
-#include 
+#include 
 
 namespace KIGFX {
 class ORIGIN_VIEWITEM;
@@ -38,7 +38,7 @@ class PCB_EDIT_FRAME;
  *
  * Handles actions specific to the board editor in pcbnew.
  */
-class PCB_EDITOR_CONTROL : public TOOL_INTERACTIVE
+class PCB_EDITOR_CONTROL : public PCB_TOOL
 {
 public:
 PCB_EDITOR_CONTRO

Re: [Kicad-developers] [PATCH] Fix crash when switching from dragging to routine (PNS router)

2017-02-27 Thread John Beard
Hi Julius,

The TOOL_EVENT_UTILS functions are for generic event decision logic.
That PCB_ACTIONS::routerInlineDrag acts as to cancel an in progress
routing is specific to the ROUTER_TOOL event loop, so it should
probably be OR'ed in in ROUTER_TOOL::performRouting(), like this:

else if( TOOL_EVT_UTILS::IsCancelInteractive( *evt )
 || evt->IsAction( &PCB_ACTIONS::routerInlineDrag )
 || evt->IsUndoRedo() )

As you can see UndoRedo also has a special handling in the router tool
that is not shared with all other tools by inclusion in
IsCancelInteractive.

Cheers,

John

On Tue, Feb 28, 2017 at 9:10 AM, Julius Schmidt  wrote:
> The attached patch fixes a bug where triggering InlineDrag while routing
> is in progress will crash pcbnew.  The problem is that the InlineDrag
> event does not terminate performRouting.  Once InlineDrag is finished
> it will call StopRouting which deletes the m_placer.  The Wait() in
> performRouting will then return and it will crash as soon as it tries
> to access the m_placer.
>
> My fix is to add the InlineDrag action to IsCancelInteractive. I also
> added a wxCHECK2 call in case there are other ways for this to happen
> (at least it won't crash).
>
> To reproduce the bug (assuming standard hotkeys):
> 1. Press X to enter route mode.
> 2. Click on a trace.
> 3. With the cursor over a trace, press D to enter drag mode.
> 4. Click somewhere to quit drag mode.
> 5. (Back in routing mode) Click again to crash pcbnew.
>
> ---
>  pcbnew/router/router_tool.cpp | 3 +++
>  pcbnew/tools/tool_event_utils.cpp | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp
> index 8f79cd0..5bfcc4c 100644
> --- a/pcbnew/router/router_tool.cpp
> +++ b/pcbnew/router/router_tool.cpp
> @@ -600,6 +600,9 @@ void ROUTER_TOOL::performRouting()
>
>  while( OPT_TOOL_EVENT evt = Wait() )
>  {
> +// Don't crash if IsCancelInteractive missed an operation that
> cancelled routing.
> +wxCHECK2( m_router->RoutingInProgress(), break );
> +
>  if( evt->IsMotion() )
>  {
>  m_router->SetOrthoMode( evt->Modifier( MD_CTRL ) );
> diff --git a/pcbnew/tools/tool_event_utils.cpp
> b/pcbnew/tools/tool_event_utils.cpp
> index 7abe83a..7d4558c 100644
> --- a/pcbnew/tools/tool_event_utils.cpp
> +++ b/pcbnew/tools/tool_event_utils.cpp
> @@ -31,7 +31,8 @@ bool TOOL_EVT_UTILS::IsCancelInteractive( const
> TOOL_EVENT& aEvt )
>  {
>  return aEvt.IsAction( &ACTIONS::cancelInteractive )
>  || aEvt.IsActivate()
> -|| aEvt.IsCancel();
> +|| aEvt.IsCancel()
> +|| aEvt.IsAction( &PCB_ACTIONS::routerInlineDrag );
>  }
>
>  bool TOOL_EVT_UTILS::IsRotateToolEvt( const TOOL_EVENT& aEvt )
> --
> 2.10.2
>
> ___
> 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] [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog

2017-02-28 Thread John Beard
On Tue, Feb 28, 2017 at 9:00 PM, Robbert Lagerweij
 wrote:
> Hi Thomas,
>
> Thank you for your feedback. The duplication of code is indeed not how I
> would usually approach this but I thought it the lesser of two evils given
> the fact that the legacy stuff will most likely be deprecated immediately
> after the 5.0 release. This means that long term maintainability is not
> really affected since there will be limited chance of structural changes
> needed to that part of the code (presumably only 5.x bug fixes) .

Since the work is done already in Legacy, I'd vote to leave it in.
Duplicating code is paradoxically probably the cleanest way, as when
legacy gets the chop, the GAL code will be unaffected. Combining the
code is probably more likely to need tidying up in future that isn't
just deleting one of the call sites.

> If we go the GAL only route, since legacy and GAL use the same dialog, I
> could either create a new dialog which we only use in GAL or add additional
> logic to disable/hide the functionality in legacy.

I think a new dialog is overkill and not worth the effort since the
legacy work is done. At most, a constructor parameter to hide relevant
UI controls would suffice, and be easy to rip out later.

Cheers,

John

___
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] [PATCH] Fix layer color swatches in Linux

2017-02-28 Thread John Beard
Hi,

Sorry to bump, but this patchset fixes a fairly annoying usability bug
on Linux/GTK+.

Are there any comments about it? It seems to work OK my my development
branch. There have been no conflicts in the last week, but I rebased
it up to master anyway.

Cheers,

John

On Thu, Feb 23, 2017 at 8:38 AM, John Beard  wrote:
> Hi,
>
> Please use this branch instead:
>
> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/swatches
>
> The patches previously omitted the new colour picker in the rebase
> (which is kind of important!).
>
> Cheers,
>
> John
>
> On Thu, Feb 23, 2017 at 2:18 AM, John Beard  wrote:
>> Hi Wayne,
>>
>> I rebased the patches onto the COLOR4D work.
>>
>> I've also added a further refactor than generalises the row indicator
>> icons, which removes a bit more logic from the layer widgets, and can
>> be reused as a generic switchable icon for other purposes if needed (I
>> had an Inkscape-style "eye" for the visibility checkbox in mind, but
>> it's not limited to two states, or to just the layer widget).
>>
>> I've put both the COLOR_SWATCH and INDICATOR_ICONS in common/widgets.
>>
>> There's a lot more scope for tidying this widget, and other UI ideas
>> abound. This should help a little to tidying up, and at least makes it
>> usable and consistent across platforms for v5.
>>
>> Cheers,
>>
>> John
>>
>> On Tue, Feb 21, 2017 at 11:05 PM, John Beard  wrote:
>>> On Tue, Feb 21, 2017 at 10:41 PM, Wayne Stambaugh  
>>> wrote:
>>>> I forgot to mention that this will likely clash with the COLOR4D work
>>>> that is already in progress so we will have to coordinate the changes.
>>>
>>> Happy to rebase as needed.
>>>
>>>> On 2/21/2017 9:35 AM, Wayne Stambaugh wrote:
>>>>> My only issue with this change is that the tooltip letting the user know
>>>>> that a left button double click or a middle button click would allow
>>>>> them to change the color is gone.
>>>
>>> The tooltip is still there for me (Linux). I specifically checked it,
>>> as I think it's a useful thing, since there's no other affordance on
>>> the swatches to tell you what to do. I hope it's not platform specfic
>>> as to whether the swatch wxStaticBitmap or the COLOR_SWATCH panel
>>> provide the tooltip.
>>>
>>> Cheers,
>>>
>>> John

___
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] [PATCH] Fix layer color swatches in Linux

2017-02-28 Thread John Beard
Hi Wayne,

On Wed, Mar 1, 2017 at 2:06 AM, Wayne Stambaugh  wrote:
> Sorry John.  I was going to recommend we apply your patch as is
> (assuming you didn't change the click behavior) to fix the gtk issue.
> We can discuss other changes to the behavior later.

I haven't changed any behaviour or positioning. These are orthogonal
to this patch, and clearly have unresolved issues around them!

I did try (separately) to put the colour wells on the right, and had
some success, but getting a wxFlexGridSizer to do the right thing even
for long strings that go off the end (think gerbview) is non-trivial,
so I put it aside for now. Perhaps a wxGrid would work better.

Another thing we probably should fix is to put this widget in
common/widgets or something, because it's currently sneakily included
in gerbview's CMake, but it lives in pcbnews's directory.

Cheers,

John

___
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] Ubuntu build commit IDs

2017-02-28 Thread John Beard
Hi Jean-Samuel,

Thank you, a bug report on a nightly now is tagged
no-vcs-found-be10de8~57~ubuntu16.10.1, which is much clearer!

Cheers,

John

On Mon, Feb 27, 2017 at 8:22 PM, Jean-Samuel Reynaud
 wrote:
> Hi all,
>
> In your example 7735 is the revno form the git repo (It is not related
> to bzr). I had changed it with the git-commit keywork. I should be useful.
>
> Le 26/02/2017 à 16:46, John Beard a écrit :
>> Hi,
>>
>> Would it be possible to get the ubuntu builds tagged with a commit
>> hash? Version strings like  201702251416+7735~57~ubuntu16.10.1 are not
>> very useful if you want to know exactly which commit a build relates
>> to.
>>
>> The exact commit is very useful when looking at a bug report from a
>> nightly. If a regression arises between one nightly and the next, the
>> date only approximately tells you where to start bisecting.
>>
>> The Bzr-style rev number is almost entirely useless especially when
>> the build is also tagged with a date, and Bzr is dead and gone.
>> Date+git rev give you both an incrementing number for quick
>> comparisons of age of build, and the git commit is an unambiguous
>> pointer to the exact commit.
>>
>> Nearly all "nightly" and "git tracker" pacakges (PPA, AUR, etc) put
>> the git hash in the build number, pretty much for this exact reason.
>>
>> Cheers,
>>
>> John
>>
>> ___
>> 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: 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] [PATCH] Add missing header

2017-03-01 Thread John Beard
On Wed, Mar 1, 2017 at 11:40 PM, Simon Richter  wrote:
>
> color_swatch.cpp uses std::unique_ptr

Hi Simon,

This must be because the compiler (MSVC on Jenkins) has set
__cplusplus to something other than 201103L, so include/make_unique.h
isn't doing the polyfill and also isn't including . Perhaps
 should be included in include/common.h, at the same time as
make_unique.h, so that when compilers provide their own make_unique,
 is also included? Then C++14 and C++11 compiler will work
identically in this regard.

Otherwise, it's very easy to forget a  and only find breakage
much later on Jenkins under MSVC (like now!)

Cheers,

John

___
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] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-03-01 Thread John Beard
Hi Wayne,

Sorry for the delay, I've been a bit distracted by other things. The
attached patch adds io_benchmark as a new target under tools. For now,
it only includes LINE_READER input benchmarks.

It currently supports:

* Existing FILE_LINE_READERs
* Raw std::ifstream reading (with getline, so no line length limits)
* LINE_READER based on std::istream, ditto for line length limits, and
providing a simple ifstream implementation (pass a filename)
* wxInputStream LINE_READERs, for both File and FFile variants.

Running this on Linux, on a 6.4MB file made by concatenaing all .lib
files in the kicad library (for somewhat realistic data), gives:

IO Bench Mark Util
  Benchmark file: /tmp/all.lib
  Repetitions:5

std::fstream  794645 lines, acc: 62758460 in 41 ms
std::fstream, reused  794645 lines, acc: 62758460 in 41 ms
RICHIO794645 lines, acc: 62758460 in 224 ms
RICHIO, reused794645 lines, acc: 62758460 in 223 ms
std::ifstream L_R 794645 lines, acc: 62758460 in 54 ms
std::ifstream L_R, reused 794645 lines, acc: 62758460 in 52 ms
wxIStream 794645 lines, acc: 62758460 in 8817 ms
wxIStream, reused 794645 lines, acc: 62758460 in 9115 ms
wxFFIStream   794645 lines, acc: 62758460 in 1444 ms
wxFFIStream, reused   794645 lines, acc: 62758460 in 1441 ms

Note that this only considers the LINE_READER classes, which are the
right tool when reading line-based formats like .lib, but not really
for things like sexp which don't have to have line breaks at all, or
if they do, don't have to put them in "friendly" places. But that's
the parser's problem!

Cheers,

John

On Thu, Mar 2, 2017 at 7:31 AM, Wayne Stambaugh  wrote:
> On 2/19/2017 4:27 AM, John Beard wrote:
>> On Sat, Feb 18, 2017 at 1:08 AM, Nox  wrote:
>>> What about wxFFileInputStream instead of wxFileInputStream?
>>>
>>
>> wxFFileInputStream appears to be about 5-6 times faster than
>> wxFileInputStream, but that's still much much slower than RICHIO or
>> std::ifstream.
>>
>> $ qa/io_benchmark/io_benchmark /tmp/all.lib 2
>> IO Bench Mark Util
>>   Benchmark file: /tmp/all.lib
>>   Repetitions:2
>> std::fstream  317858 lines, acc: 25103384 in 16 ms
>> std::fstream, reused  317858 lines, acc: 25103384 in 16 ms
>> RICHIO317858 lines, acc: 25103384 in 91 ms
>> RICHIO, reused317858 lines, acc: 25103384 in 90 ms
>> New fstream IO317858 lines, acc: 25103384 in 19 ms
>> New fstream IO, reused317858 lines, acc: 25103384 in 19 ms
>> wxIStream 317858 lines, acc: 25103384 in 3558 ms
>> wxIStream, reused 317858 lines, acc: 25103384 in 3429 ms
>> wxFFIStream   317858 lines, acc: 25103384 in 589 ms
>> wxFFIStream, reused   317858 lines, acc: 25103384 in 602 ms
>>
>
> John,
>
> Did you create a patch for your wxInputStream benchmarking code?  I
> would like to merge this.  I think it would be a useful tool for developers.
>
> Thanks,
>
> 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
From c9b03211ea274800611cc39331b814f73ef0a1e1 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 15 Feb 2017 16:47:41 +0800
Subject: [PATCH] IO benchmark program: RICHIO vs std::fstream and wxStream

This compares the performance of RICHIO line readers against other
implementations, and can be used for profiling and optimisation.

Current benchmarks provided:

* richio FILE_LINE_READER
* Raw std::ifstream (no LINE_READER wrapper), using getline (so no line
length limiting)
* LINE_READER wrapper around std::istream, with a std::ifstream
implementation
* Existing richio wxInputStream wrappers (with File and FFile
implemntations)
---
 tools/CMakeLists.txt |   2 +
 tools/io_benchmark/CMakeLists.txt|  17 ++
 tools/io_benchmark/io_benchmark.cpp  | 321 +++
 tools/io_benchmark/stdstream_line_reader.cpp |  90 
 tools/io_benchmark/stdstream_line_reader.h   |  70 ++
 5 files changed, 500 insertions(+)
 create mode 100644 tools/io_benchmark/CMakeLists.txt
 create mode 100644 tools/io_benchmark/io_benchmark.cpp
 create mode 100644 tools/io_benchmark/stdstream_line_reader.cpp
 create mode 100644 tools/io_benchmark/stdstream_line_reader.h

diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index 21f47d0a7..7955536b5 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -35,3 +35,5 @@ add_executable( property_tree
 target_link_libraries( property_tree
 ${w

[Kicad-developers] [PATCH] Add WithAlpha method to COLOR4D

2017-03-01 Thread John Beard
Hi,

Here's a patch to add a new method to COLOR4D: WithAlpha. This means
you can do this:

const COLOR4D color = getSomeColor(); //could be an argument or static const

gal.SetStrokeColor( color.WithAlpha( 0.5 ) );

Rather than:

const COLOR4D color = getSomeColor(); //could be an argument or static const

COLOR4D temp = color; //don't modify base color
color.a = 0.5;
gal.SetStrokeColor( temp );

or the shorter but less readable:

gal.SetStrokeColor( { color.r, color.g, color.b, 0.5 } );

It also uses transparent sanity checking in debug builds.

Cheers,

John
From 25566fab5dbd0ef185b995383eae1ca5b97f6c12 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Wed, 1 Mar 2017 09:59:25 +0800
Subject: [PATCH] Add WithAlpha method to COLOR4D

This is useful for taking a "base" color and making derived colours with
the given alpha, but the same RGB value.
---
 include/gal/color4d.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/gal/color4d.h b/include/gal/color4d.h
index 4e0c0f4ae..ebcf333bc 100644
--- a/include/gal/color4d.h
+++ b/include/gal/color4d.h
@@ -224,6 +224,19 @@ public:
 }
 
 /**
+ * Function WithAlpha
+ * Returns a colour with the same colour, but the given alpha
+ * @param aAlpha specifies the alpha of the new color
+ * @return COLOR4D color with that alpha
+ */
+ COLOR4D WithAlpha( double aAlpha ) const
+ {
+ assert( aAlpha >= 0.0 && aAlpha <= 1.0 );
+
+ return COLOR4D( r, g, b, aAlpha );
+ }
+
+/**
  * Function Inverted
  * Returns an inverted color, alpha remains the same.
  * @return COLOR4D& Inverted color.
-- 
2.12.0

___
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] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-03-02 Thread John Beard
Hi Wayne,

On Thu, Mar 2, 2017 at 9:02 PM, Wayne Stambaugh  wrote:
>
> Does this patch get applied on top of the previous patch or does it
> replace the previous patch.

Sorry, that wasn't very clear! It's a new patch, all rebased into one
commit, since I messed with it quite a bit and the intermediate state
is not very exciting.

Let me know if there's a better location for it in the tree. Since
it's not actually a test or part of QA process, but rather
development, I moved it out of qa.

Cheers,

John

___
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] Build failed in Jenkins: kicad-noscript-fedora20 #1645

2017-03-02 Thread John Beard
 <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.cpp>:24:
> /usr/include/c++/4.9.2/bits/basic_ios.h:66:11: error: within this context
>  class basic_ios : public ios_base
>^
> In file included from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.h>:30:0,
>  from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.cpp>:24:
> /usr/include/c++/4.9.2/fstream:430:11: error: use of deleted function 
> ‘std::basic_ios::basic_ios(const std::basic_ios&)’
>  class basic_ifstream : public basic_istream<_CharT, _Traits>
>^
> /usr/include/c++/4.9.2/fstream:430:11: error: use of deleted function 
> ‘std::basic_filebuf::basic_filebuf(const std::basic_filebuf&)’
> /usr/include/c++/4.9.2/fstream:72:11: note: 
> ‘std::basic_filebuf::basic_filebuf(const std::basic_filebuf&)’ is 
> implicitly deleted because the default definition would be ill-formed:
>  class basic_filebuf : public basic_streambuf<_CharT, _Traits>
>^
> In file included from /usr/include/c++/4.9.2/ios:43:0,
>  from /usr/include/c++/4.9.2/ostream:38,
>  from /usr/include/c++/4.9.2/iterator:64,
>  from /usr/include/wx-3.0/wx/arrstr.h:116,
>  from /usr/include/wx-3.0/wx/wx.h:21,
>  from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/include/richio.h>:37,
>  from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.h>:27,
>  from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.cpp>:24:
> /usr/include/c++/4.9.2/streambuf:802:7: error: ‘std::basic_streambuf<_CharT, 
> _Traits>::basic_streambuf(const std::basic_streambuf<_CharT, _Traits>&) [with 
> _CharT = char; _Traits = std::char_traits]’ is private
>basic_streambuf(const basic_streambuf& __sb)
>^
> In file included from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.h>:30:0,
>  from 
> <http://ci.kicad-pcb.org/job/kicad-noscript-fedora20/ws/tools/io_benchmark/stdstream_line_reader.cpp>:24:
> /usr/include/c++/4.9.2/fstream:72:11: error: within this context
>  class basic_filebuf : public basic_streambuf<_CharT, _Traits>
>^
> tools/io_benchmark/CMakeFiles/io_benchmark.dir/build.make:77: recipe for 
> target 
> 'tools/io_benchmark/CMakeFiles/io_benchmark.dir/stdstream_line_reader.cpp.o' 
> failed
> make[2]: *** 
> [tools/io_benchmark/CMakeFiles/io_benchmark.dir/stdstream_line_reader.cpp.o] 
> Error 1
> CMakeFiles/Makefile2:2167: recipe for target 
> 'tools/io_benchmark/CMakeFiles/io_benchmark.dir/all' failed
> make[1]: *** [tools/io_benchmark/CMakeFiles/io_benchmark.dir/all] Error 2
> make[1]: *** Waiting for unfinished jobs
> [ 83%] Built target pl_editor
> [ 83%] Built target pcb_calculator
> [ 83%] Built target gerbview
> Makefile:137: recipe for target 'all' failed
> make: *** [all] Error 2
> Build step 'Execute shell' marked build as failure
>
> ___
> 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
From b3534d319c11504387db253d16ce1f03457488de Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 3 Mar 2017 00:03:53 +0800
Subject: [PATCH] io_benchmark: construct std::ifstream directly

This appears to normally use the move constructor, but on Jenkins Fedora
20, it tried to use the copy ctor, which is deleted.

Constructing directly fixes this, and is the right way, anyway.
---
 tools/io_benchmark/stdstream_line_reader.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/io_benchmark/stdstream_line_reader.cpp b/tools/io_benchmark/stdstream_line_reader.cpp
index 8a0954a8e..ce0042f77 100644
--- a/tools/io_benchmark/stdstream_line_reader.cpp
+++ b/tools/io_benchmark/stdstream_line_reader.cpp
@@ -68,7 +68,7 @@ void STDISTREAM_LINE_READER::setStream( std::istream& aStream )
 
 
 IFSTREAM_LINE_READER::IFSTREAM_LINE_READER( const wxString& aFileName ) throw( IO_ERROR ) :
-m_fStream( std::ifstream( aFileName ) )
+m_fStream( aFileName )
 {
 if( !m_fStream.is_open() )
 {
-- 
2.12.0

___
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] RICHIO performance - 3 to 30 times slower than std::ifstream

2017-03-02 Thread John Beard
On Thu, Mar 2, 2017 at 10:30 PM, Wayne Stambaugh  wrote:
>
> One thing I overlooked is wxWidgets provides wxBufferedInputStream[1]
> which takes a reference to a wxInputStream object as an argument.  The
> reason wxFileInputStream and wxFFileInputStream may be so slow is that
> they are not buffered at all internally and need to be wrapped by
> wxBufferedInputStream to perform the buffering.  It might be something
> worth taking a look at.

I tried this out - there's a factor of just over 10 improvment over a
plain wxFileInputStream and a 30% ish speedup over wxFFileStream, but
it's still 20 times slower than std::ifstream. I can't help but think
I'm just doing all the wx stuff wrong - it cannot be that bad for
reals!


I have also noticed, doing this in a quieter room, that the 4000ms
wxInputStream benchmarks (File, FFile and Buffered) cause the fans on
my computer to kick up to full, so something is being computed
furiously!

std::fstream   317858 lines, acc: 25103384 in 16 ms
std::fstream, reused   317858 lines, acc: 25103384 in 16 ms
RICHIO 317858 lines, acc: 25103384 in 95 ms
RICHIO, reused 317858 lines, acc: 25103384 in 93 ms
std::ifstream L_R  317858 lines, acc: 25103384 in 21 ms
std::ifstream L_R, reused  317858 lines, acc: 25103384 in 21 ms
wxFileIStream  317858 lines, acc: 25103384 in 4218 ms
wxFileIStream, reused  317858 lines, acc: 25103384 in 4225 ms
wxFFileIStream 317858 lines, acc: 25103384 in 568 ms
wxFFileIStream, reused 317858 lines, acc: 25103384 in 567 ms
wxFileIStream. buf'd   317858 lines, acc: 25103384 in 380 ms
wxFileIStream, buf'd, reused   317858 lines, acc: 25103384 in 383 ms
wxFFileIStream. buf'd  317858 lines, acc: 25103384 in 376 ms
wxFFileIStream, buf'd, reused  317858 lines, acc: 25103384 in 376 ms

For the wx streams, if you do more reps, there's a huge increase in
releative slowness
 (above is 2, below is 10):

std::fstream   1589290 lines, acc: 125516920 in 81 ms
wxFileIStream  1589290 lines, acc: 125516920 in 21279 ms
wxFileIStream. buf'd   1589290 lines, acc: 125516920 in 14394 ms

No idea what's behind all this, any I'll be happy to be told im
Holding It Wrong (TM)!

Patch for adding buffered wx streams to the benchmark tool attached.

Cheers,

John
From d1d066d65a4e5525f3e47f731d7d132715aa6b88 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 3 Mar 2017 00:26:39 +0800
Subject: [PATCH] io_benchmark: Add wxBufferedInputStream benchmarks

When run, these show a 10x speed up over wxFileInputStream alone and
about 30% over wxFFInputStream.
---
 tools/io_benchmark/io_benchmark.cpp | 62 +++--
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/tools/io_benchmark/io_benchmark.cpp b/tools/io_benchmark/io_benchmark.cpp
index 3eea0c095..1d7071199 100644
--- a/tools/io_benchmark/io_benchmark.cpp
+++ b/tools/io_benchmark/io_benchmark.cpp
@@ -157,7 +157,7 @@ static void bench_line_reader_reuse( const wxString& aFile, int aReps, BENCH_REP
 /**
  * Benchmark using an INPUTSTREAM_LINE_READER with a given
  * wxInputStream implementation.
- * The INPUTSTREAM_LINE_READER is reset for each cycle.
+ * The wxInputStream is recreated for each cycle.
  */
 template
 static void bench_wxis( const wxString& aFile, int aReps, BENCH_REPORT& report )
@@ -182,7 +182,7 @@ static void bench_wxis( const wxString& aFile, int aReps, BENCH_REPORT& report )
 /**
  * Benchmark using an INPUTSTREAM_LINE_READER with a given
  * wxInputStream implementation.
- * The INPUTSTREAM_LINE_READER is recreated for each cycle.
+ * The wxInputStream is reset for each cycle.
  */
 template
 static void bench_wxis_reuse( const wxString& aFile, int aReps, BENCH_REPORT& report )
@@ -202,6 +202,58 @@ static void bench_wxis_reuse( const wxString& aFile, int aReps, BENCH_REPORT& re
 }
 }
 
+
+/**
+ * Benchmark using a INPUTSTREAM_LINE_READER with a given
+ * wxInputStream implementation, buffered with wxBufferedInputStream.
+ * The wxInputStream is recreated for each cycle.
+ */
+template
+static void bench_wxbis( const wxString& aFile, int aReps, BENCH_REPORT& report )
+{
+WXIS fileStream( aFile );
+wxBufferedInputStream bufferedStream( fileStream );
+
+for( int i = 0; i < aReps; ++i)
+{
+INPUTSTREAM_LINE_READER istr( &bufferedStream, aFile );
+
+while( istr.ReadLine() )
+{
+report.linesRead++;
+report.charAcc += (unsigned char) istr.Line()[0];
+}
+
+fileStream.SeekI( 0 );
+}
+}
+
+
+/**
+ * Benchmark using a INPUTSTREAM_LINE_READER with a given
+ * wxInputStream implementation, buffered with wxBufferedInputStream.
+ * The wxInputStream is reset for each cycle.
+ 

[Kicad-developers] [PATCH/RFC] Pcbnew initial startup user experience

2017-03-04 Thread John Beard
Hi,

Here is a (proposed) patchset to modify the "user experience" (to use
a buzzword) of Pcbnew and cvPcb when the user loads it for the first
time with no fp-library-table.

The current behaviour is quite unfriendly, as it doesn't get the user
to consent to an action when initialising the library, it does one of
two actions (cpopy default table, or create empty table - which one
depends on various conditions) which means that users click the OK
button not fully knowing what just happened.

Furthermore, if they click though that dialog without reading it, they
will then be frustrated by the lack of libraries later, which
manifests as an error or an empty footprint chooser. Bear in mind, as
far as users are concerned, the libraries are installed (e.g.
/usr/share/kicad has content), but as far as KiCad is concerned, they
are not necessarily configured (empty or default fp-lib-table in
user's home).

This has caused several people to struggle with getting started with
KiCad, and people asking how to proceed is a semi-regular occurence on
IRC.

This patch changes to the following process:

* If user fp-lib-table is found and it has at least one library,
everything is normal
  and the user isn't disturbed.
* If fp-lib-table is missing:
* On Pcbnew/Cvpcb start, pop up a dialog saying this and giving
these options:
* Quit - exit Pcbnew/cvPcb witohut attempting to create
fp-lib-table. Users can then make their own arrangements.
* Copy default table - attempt to copy the default table. If
this fails, return to the choice dialog.
* Create empty table - if this fails return to the dialog.
* In no case did the user get an fp-lib-table they didn't ask for.
* If a blank or default table is created, a second dialog appears,
informing the user that the library table might need configuration.
This provides three options:
* Cancel - user doesn't want to configure the table now, table
is unchanged
* Open Manual - the manual referenced in the dialog message is
opened so the user can read about library configuration
* Open FP Library Manager - go directly to the library
manager, rather than expecting the user to cancel the dialog and find
it themselves. The wizard is accessible from the manager dialog.
* If fp-lib-table is found, but it's empty when the user goes to use
it (e.g. on a keyword search for footprints), the
Cancel/Manual/Manager dialog is shown again, but it's a warning rather
than informational, as it was before.

The aim here is to ensure that the configuration is "management by
consent" and that the user is:

* Not surprised by "secret" configuration behind the scenes
* Informed of and consenting to specific actions
* Guided though configuration with dialogs that give access to
mentioned tools, rather than a block of text and an OK/Cancel button,
after which they are on their own.

Technically speaking, this is done by moving the global fp-lib-table
loading into common, and invoking it only when the frame is shown
(rather than prior to the kiface load). The reason for this is so that
the followup dialog can access the manager. Also, it reduces special
casing before kiface load.

The on-show invocation is done by a small class subscribed to the
frame's wx "Show" event, and once the event is received, the class
unsubscribes itself (so it's a one-shot), and executes a lits of
startup actions. The class is such that you can add multiple startup
actions, any of which can "veto" startup if needed.

Sorry for the long email: the change is not really that complex, but
the configuration process has several paths. As the first thing a new
user to Pcbnew sees, I think this is a fairly important thing to get
right.

Cheers,

John
From 8e1066a946a2a2fd5138790134ff1e3e7ce195da Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Fri, 3 Mar 2017 22:25:26 +0800
Subject: [PATCH 3/3] Move definition of global library table into fp_lib_table

This puts the definition of the variable in the corresponding position
to the declaration, and also means that the individual targets don't
need to define it themselves.
---
 common/fp_lib_table.cpp | 6 ++
 cvpcb/cvpcb.cpp | 5 -
 pcbnew/pcbnew.cpp   | 6 --
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/common/fp_lib_table.cpp b/common/fp_lib_table.cpp
index c7e147221..3d785c8c1 100644
--- a/common/fp_lib_table.cpp
+++ b/common/fp_lib_table.cpp
@@ -41,6 +41,12 @@ using namespace LIB_TABLE_T;
 static const wxChar global_tbl_name[] = wxT( "fp-lib-table" );
 
 
+/// The global footprint library table.  This is not dynamically allocated because
+/// in a multiple project environment we must keep its address constant (since it is
+/// the fallback table for multiple projects).
+FP_LIB_TABLEGFootprintTable;
+
+
 bool FP_LIB_TABLE_ROW::operator==( const FP_LIB_TABLE_ROW& aRow ) c

Re: [Kicad-developers] [PATCH/RFC] Pcbnew initial startup user experience

2017-03-05 Thread John Beard
Hi Wayne,

Thanks for taking a look!

On Mon, Mar 6, 2017 at 3:24 AM, Wayne Stambaugh  wrote:
>
> It is perfectly acceptable to have an empty global library table and
> always use the project library table for defining footprint libraries so
> this check would be annoying to users who prefer this.
>

The first dialog (Quit/Copy/Create Empty) is invoked only when the
global table is missing, not empty.

The second dialog (Cancel/Manual/Manager) is a more explicit version
of the dialog that already occurs, e.g. on a keyword search, but I
agree it would be annoying to have this on startup.

Should there still be a nag prompt on things like keyword search if
all libraries come up empty in total (i.e. empty global and project
libs)?

> Rather than go through all of this, why not just use a single wizard to
> step the user all of the way through the configuration.  The first page
> of the wizard should have a list of valid footprint table configuration
> choices followed by appropriate wizard pages.  I can see five plausible
> global fp-lib-table config options.
>
> * Copy the default global footprint library table. (disabled if the
> default global footprint table cannot be found)
>
> * Copy a custom global footprint library table.
>
> * I know what I'm doing and will configure my own footprint library table.
>
> * Use the footprint library table wizard to help me create a custom
> global footprint library table.
>
> * I prefer to use project specific library tables, create an empty
> global footprint library table.

That does make more sense, I will take a look at roughing something up
like that. Logically it shouldn't be too hard, but getting UI right
might take a little while, especially if taking care of all entry
points (so far, pcbnew, cvpcb and eeschema component chooser) is not
plain sailing!

The third option still involves quitting and dealing with it
externally, right? Otherwise Pcbnew will have to be ready for having
no global table, not just an empty one.

If the user cancels any of these processes (if they can), do you
envision it dumping them back at the wizard entry point?

Perhaps also add "download latest table from Github", since that can
change a lot over the lifetime of a release? Otherwise, all the
"default" tables bundled with a release will become stale as Github
repos change, unless the release packages get updates for that.

Cheers,

John

___
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] GerbView GAL: Ready for alpha testing; need more sample Gerber files

2017-03-05 Thread John Beard
On 4 March 2017 21:37:56 GMT+00:00, Jon Evans  wrote:
>A few more notes:
>
>- Grid settings hasn't been added in the GUI yet; I'll be taking that
>from
>pcbnew so user can pick dot/line/small

Hi Jon,

I haven't had a chance to recompile yet (full disclosure!) but this
sounds great.

For the GAL settings, I was planning to break out the GAL settings
from the Pcbnew Display Settings dialog into a reusable widget than
can be embedded into tabs in each GAL-enabled program and acts on a
passed-on GAL_DISPLAY_OPTIONS instance. Currently the footprint editor
also lacks grid style control as well.

Having to separately add all GAL-specific display options to each
dialog would otherwise become a bit of a maintenance headache.

Cheers,

John

___
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


[Kicad-developers] [PATCH] Open polylines in OpenGL GAL

2017-03-07 Thread John Beard
Hi,

Attached patch fixes polylines in GAL: they were drawn always closed
in OpenGL and always open in Cairo.

I think the code could stand a refactor in this area and try to move
the logic into GAL, rather thn duplicating in each implementaiton.
However, since CAIRO_GAL uses the same code for polyline and polygon,
it's not trivial, so I'm going to submit this first, separately.

Cheers,

John
From 4452895a9f014dfe4c4ad98070b5556ccfe81d68 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Tue, 7 Mar 2017 19:17:59 +0800
Subject: [PATCH] GAL: Respect SHAPE_LINE_CHAIN closed setting

When drawing polylines using SHAPE_LINE_CHAIN, the polyline is always
was drawn closed in GAL and open in Cairo, regardless of the state of
SHAPE_LINE_CHAIN.
---
 common/gal/cairo/cairo_gal.cpp   | 7 ++-
 common/gal/opengl/opengl_gal.cpp | 7 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/common/gal/cairo/cairo_gal.cpp b/common/gal/cairo/cairo_gal.cpp
index 953c76308..724e863d8 100644
--- a/common/gal/cairo/cairo_gal.cpp
+++ b/common/gal/cairo/cairo_gal.cpp
@@ -1081,10 +1081,15 @@ void CAIRO_GAL::drawPoly( const SHAPE_LINE_CHAIN& aLineChain )
 if( aLineChain.PointCount() < 2 )
 return;
 
+auto numPoints = aLineChain.PointCount();
+
+if( aLineChain.IsClosed() )
+numPoints += 1;
+
 const VECTOR2I start = aLineChain.CPoint( 0 );
 cairo_move_to( currentContext, start.x, start.y );
 
-for( int i = 1; i < aLineChain.PointCount(); ++i )
+for( int i = 1; i < numPoints; ++i )
 {
 const VECTOR2I& p = aLineChain.CPoint( i );
 cairo_line_to( currentContext, p.x, p.y );
diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp
index a2480411b..8a6ddf054 100644
--- a/common/gal/opengl/opengl_gal.cpp
+++ b/common/gal/opengl/opengl_gal.cpp
@@ -640,7 +640,12 @@ void OPENGL_GAL::DrawPolyline( const VECTOR2D aPointList[], int aListSize )
 
 void OPENGL_GAL::DrawPolyline( const SHAPE_LINE_CHAIN& aLineChain )
 {
-drawPolyline( [&](int idx) { return aLineChain.CPoint(idx); }, aLineChain.PointCount() + 1 );
+auto numPoints = aLineChain.PointCount();
+
+if( aLineChain.IsClosed() )
+numPoints += 1;
+
+drawPolyline( [&](int idx) { return aLineChain.CPoint(idx); }, numPoints );
 }
 
 
-- 
2.12.0

___
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


[Kicad-developers] [PATCH] Measurement tool for GAL

2017-03-09 Thread John Beard
Hi,

Here's a tool I'm had on my list for a while - a ruler tool.

This patch set introduces a new directory, for "preview items" which
are EDA_ITEMS that are only used for transient previews, such as
drawing aids and selection boxes. These are located in common, so that
they can (in theory) be used in any GAL canvas, not just
Pcbnew/Modedit.

There are also some functions that hopefully will be useful for other
preview items in future in the utils file.

There's an oustanding bug with the OpenGL GAL in that it cannot draw
spaces in text, so there are a couple of places where it pre-emptively
strips spaces that should be removed when that bug is fixed.
(https://bugs.launchpad.net/kicad/+bug/1668455).

Cheers,

John
From 1e4baceaa73dd2d9dd189f71603a7916abd93850 Mon Sep 17 00:00:00 2001
From: John Beard 
Date: Thu, 9 Mar 2017 17:09:13 +0800
Subject: [PATCH 3/3] Add a ruler tool to pcbnew GAL

This allows to measure between features on a PCB. It uses a preview
EDA_ITEM in common, but due to the use of the IDs, it's currently
Pcbnew/Modedit only.

This also adds several "utils" for graphical functons useful when
drawing preview items on GAL canvases.

Fixes: lp:1467313
* https://bugs.launchpad.net/kicad/+bug/1467313
---
 bitmaps_png/CMakeLists.txt |   1 +
 bitmaps_png/cpp_26/measurement.cpp |  24 +++
 bitmaps_png/sources/measurement.svg| 140 +
 common/CMakeLists.txt  |   2 +
 common/preview_items/preview_utils.cpp | 175 ++
 common/preview_items/ruler_item.cpp| 264 +
 include/bitmaps.h  |   1 +
 include/preview_items/preview_utils.h  |  90 +++
 include/preview_items/ruler_item.h |  95 
 pcbnew/edit.cpp|   5 +
 pcbnew/modedit.cpp |   5 +
 pcbnew/modedit_onclick.cpp |   5 +
 pcbnew/moduleframe.cpp |   4 +-
 pcbnew/onleftclick.cpp |   5 +
 pcbnew/pcbframe.cpp|   4 +-
 pcbnew/pcbnew_id.h |   3 +
 pcbnew/tool_modedit.cpp|   5 +
 pcbnew/tool_pcb.cpp|   5 +
 pcbnew/tools/edit_tool.cpp |  98 
 pcbnew/tools/edit_tool.h   |   3 +
 pcbnew/tools/pcb_actions.cpp   |   4 +
 pcbnew/tools/pcb_actions.h |   1 +
 22 files changed, 935 insertions(+), 4 deletions(-)
 create mode 100644 bitmaps_png/cpp_26/measurement.cpp
 create mode 100644 bitmaps_png/sources/measurement.svg
 create mode 100644 common/preview_items/preview_utils.cpp
 create mode 100644 common/preview_items/ruler_item.cpp
 create mode 100644 include/preview_items/preview_utils.h
 create mode 100644 include/preview_items/ruler_item.h

diff --git a/bitmaps_png/CMakeLists.txt b/bitmaps_png/CMakeLists.txt
index 8dae66fcf..80bdcea45 100644
--- a/bitmaps_png/CMakeLists.txt
+++ b/bitmaps_png/CMakeLists.txt
@@ -339,6 +339,7 @@ set( BMAPS_MID
 load_module_lib
 local_ratsnest
 locked
+measurement
 mirepcb
 mirror_h
 mirror_v
diff --git a/bitmaps_png/cpp_26/measurement.cpp b/bitmaps_png/cpp_26/measurement.cpp
new file mode 100644
index 0..8031f1fbf
--- /dev/null
+++ b/bitmaps_png/cpp_26/measurement.cpp
@@ -0,0 +1,24 @@
+
+/* Do not modify this file, it was automatically generated by the
+ * PNG2cpp CMake script, using a *.png file as input.
+ */
+
+#include 
+
+static const unsigned char png[] = {
+ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
+ 0x00, 0x00, 0x00, 0x1a, 0x00, 0x00, 0x00, 0x1a, 0x08, 0x04, 0x00, 0x00, 0x00, 0x03, 0x43, 0x84,
+ 0x45, 0x00, 0x00, 0x00, 0x76, 0x49, 0x44, 0x41, 0x54, 0x38, 0xcb, 0x63, 0x60, 0x18, 0x2a, 0x20,
+ 0x35, 0x01, 0x3b, 0x1b, 0x2f, 0x48, 0xfb, 0x92, 0xaa, 0x0d, 0xd5, 0xa2, 0x9d, 0xf6, 0x85, 0x58,
+ 0x4d, 0xff, 0xd3, 0xae, 0xc6, 0x72, 0x33, 0x30, 0xc4, 0x72, 0xa7, 0x5d, 0x4d, 0xfb, 0x4f, 0xbc,
+ 0xa6, 0xff, 0xe9, 0xf3, 0x19, 0x18, 0xd2, 0xe7, 0x83, 0x58, 0x24, 0x68, 0x02, 0x29, 0x86, 0xd1,
+ 0x78, 0x81, 0xdd, 0x25, 0xe3, 0xff, 0xa4, 0x41, 0xa0, 0x26, 0xe3, 0xff, 0x17, 0x49, 0x84, 0x50,
+ 0x4d, 0xc8, 0x1a, 0x09, 0xb3, 0x51, 0x34, 0x11, 0x4b, 0x92, 0xab, 0xc9, 0xf6, 0x1a, 0x19, 0x01,
+ 0x81, 0x3d, 0x90, 0x09, 0x06, 0xfc, 0x70, 0xd4, 0x94, 0x5e, 0x8e, 0x43, 0x53, 0x3d, 0xde, 0xf4,
+ 0x07, 0xd2, 0x86, 0xa1, 0xa9, 0x9e, 0x61, 0x14, 0xe0, 0x07, 0x00, 0x55, 0x88, 0x33, 0x29, 0x9a,
+ 0xe8, 0x0b, 0x9c, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
+};
+
+const BITMAP_OPAQUE measurement_xpm[1] = {{ png, sizeof( png ), "measurement_xpm" }};
+
+//EOF
diff --git a/bitmaps_png/sources/measurement.svg b/bitmaps_png/sources/measurement.svg
new file mode 100644
index 0..f821f1037
--- /dev/null
+++ b/bitmaps_png/sources/measurement.svg
@@ -0,0 +1,140 @@
+
+http://purl.org/dc/elements/1.1/";
+   xmlns:cc="http://cre

Re: [Kicad-developers] [PATCH] Measurement tool for GAL

2017-03-09 Thread John Beard
By the way, here's a screenshot of the ruler tool:

Cheers,

John

On Fri, Mar 10, 2017 at 1:11 AM, John Beard  wrote:
> Hi,
>
> Here's a tool I'm had on my list for a while - a ruler tool.
>
> This patch set introduces a new directory, for "preview items" which
> are EDA_ITEMS that are only used for transient previews, such as
> drawing aids and selection boxes. These are located in common, so that
> they can (in theory) be used in any GAL canvas, not just
> Pcbnew/Modedit.
>
> There are also some functions that hopefully will be useful for other
> preview items in future in the utils file.
>
> There's an oustanding bug with the OpenGL GAL in that it cannot draw
> spaces in text, so there are a couple of places where it pre-emptively
> strips spaces that should be removed when that bug is fixed.
> (https://bugs.launchpad.net/kicad/+bug/1668455).
>
> Cheers,
>
> John
___
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


  1   2   3   4   5   >