Re: [Kicad-developers] Library Convention
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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)
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)
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
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
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
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
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
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
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
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
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
<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
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
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
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
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
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
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
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