Re: [Kicad-developers] [PATCH] GAL arc-drawing crash
It seems to me that it should be 100% reproducible, but there's only one specific way to trigger it, perhaps you're not doing it right? Here's what I did: 1. In GAL, select the arc tool. 2. Click to set the midpoint. 3. Move directly left to set the radius and startpoint. 4. Move directly left from there, so that the angle is zero, the mouse click is outside the arc radius, and the center, startpoint and click point are collinear. 5. Click to end the arc. AFAIK putting the click-point inside the radius works too, but the three must be perfectly collinear and the click-point must not be *on* the arc. On Tue, Jun 02, 2015 at 08:19:46PM +0200, Maciej Sumiński wrote: Hi Chris, I could not reproduce the assert, but your patch surely will not harm the code, so there are chances it is going to work better. Thank you, I committed your changes in revision 5695. Regards, Orson On 06/02/2015 05:44 PM, Chris Pavlina wrote: In GAL, an assertion (drawing_tool.cpp:925) will fail if you begin drawing an arc, and then accidentally terminate the arc at zero angle with the cursor /outside/ the arc radius. It seems that an attempt was made to check for this but the conditional wasn't written quite right. Here's a patch to fix the conditional and correctly cancel terminating the arc instead. -- Chris ___ 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] on incorrect polygon behavior
On 6/2/2015 2:32 PM, Tomasz Wlostowski wrote: > On 01.06.2015 19:09, jp charras wrote: >> Le 01/06/2015 18:23, Tomasz Wlostowski a écrit : >>> Hi all, >>> >>> I did a small investigation of the polygon-related >>> segfaults/miscalculations. It looks like Boost.polygon badly handles >>> cases where intersection points of the edges of the polygons lie close >>> to each other or overlap, causing the 'snap rounding' algorithm used in >>> boost to go haywire. This is the case for complex zones, with a lot of >>> thermal holes or via patterns (i.e. a thermal pad under a QFN with a via >>> field). >>> >>> See last comment from https://svn.boost.org/trac/boost/ticket/10642. >>> >>> There's a variety of effects I observed. >>> - a segfault. >>> - an incorrect or empty resulting polygon: merging the holes before >>> subtraction from the main outline (polyset_holes in >>> AddClearanceAreasPolygonsToPolysList) using boost::assign or subtracting >>> the holes one-by-one (instead of the whole set) fixes the segfault, but >>> I know at least one case where the resulting zone is incorrectly >>> calculated. I'll attach the test cases to the bug report on Launchpad >>> later in the evening. >> >> I am not sure subtracting the holes one-by-one (instead of the whole >> set) fixes the segfault. Because the segfault depends on the geometry, >> it does not happen when you changes the calculations for a given case, >> but can happen in an other case which previously did not crash. >> >>> >>> For the reasons above, I would strongly consider dropping >>> boost::polygon. There are also non-technical issues: >>> - It hasn't been maintained for ~1.5 years. The last activity of the >>> official maintainer was 3 years ago. >>> - the code is a mess (look at example below). >>> >>> >>> >>> @Jean Pierre: do you think Clipper is stable enough for our use? (at >>> least in case of a bug, it's actively developed and the author offers >>> support). I'm not asking about speed, it's not important if Kicad >>> crashes or produces incorrect polygons. >>> >>> Tom >> >> I don't think we can easily replace boost::polygon by Clipper. >> >> They have complementary features, but neither boost::polygon nor Clipper >> have all features we need ( for instance transform a self intersecting >> polygon into a set of not intersection polygons is easy with clipper, >> not with boost::polygon). >> >> A major issue (or lack of feature) is the fact Clipper does not handle >> holes inside a polygon by connecting holes by overlapping segments >> (to create only one polygon), which is needed in draw (and some other) >> functions > > Hi, > > I'm almost done with SHAPE_POLY_SET class for the geometry library, a > Clipper wrapper adding some missing Clipper features needed in Kicad > (e.g. polygon fracturing). I'll send a patch soon (affecting only > AddClearanceAreasPolygonsToPolysList), so that we can compare how well > it performs wrs to Boost.Polygon. > > Cheers, > Tom Great work Tom. Thanks. Wayne >> >> I also tried to use Clipper to replace the polygon set subtracting which >> crashes Pcbnew. >> >> But if Clipper gives good results to subtract 2 polygons, this is not >> the case to subtract 2 set of polygons (when polygons inside each set >> overlap, the result is strange, whatever the settings used in calculations). >> >> Therefore, replacing boost::polygon by clipper is not trivial. >> >> If we drop boost::polygon (due to this bug, and the lack of >> maintenance), one can consider Clipper (which looks stable enough now) >> but also CGAL. >> Clipper needs not trivial rework to calculate the zone filling. >> I do not have used CGAL, but it is a large geometry library, actively >> maintained, and it could be considered before taking a decision. >> > > > ___ > 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] on incorrect polygon behavior
On 01.06.2015 19:09, jp charras wrote: > Le 01/06/2015 18:23, Tomasz Wlostowski a écrit : >> Hi all, >> >> I did a small investigation of the polygon-related >> segfaults/miscalculations. It looks like Boost.polygon badly handles >> cases where intersection points of the edges of the polygons lie close >> to each other or overlap, causing the 'snap rounding' algorithm used in >> boost to go haywire. This is the case for complex zones, with a lot of >> thermal holes or via patterns (i.e. a thermal pad under a QFN with a via >> field). >> >> See last comment from https://svn.boost.org/trac/boost/ticket/10642. >> >> There's a variety of effects I observed. >> - a segfault. >> - an incorrect or empty resulting polygon: merging the holes before >> subtraction from the main outline (polyset_holes in >> AddClearanceAreasPolygonsToPolysList) using boost::assign or subtracting >> the holes one-by-one (instead of the whole set) fixes the segfault, but >> I know at least one case where the resulting zone is incorrectly >> calculated. I'll attach the test cases to the bug report on Launchpad >> later in the evening. > > I am not sure subtracting the holes one-by-one (instead of the whole > set) fixes the segfault. Because the segfault depends on the geometry, > it does not happen when you changes the calculations for a given case, > but can happen in an other case which previously did not crash. > >> >> For the reasons above, I would strongly consider dropping >> boost::polygon. There are also non-technical issues: >> - It hasn't been maintained for ~1.5 years. The last activity of the >> official maintainer was 3 years ago. >> - the code is a mess (look at example below). >> >> >> >> @Jean Pierre: do you think Clipper is stable enough for our use? (at >> least in case of a bug, it's actively developed and the author offers >> support). I'm not asking about speed, it's not important if Kicad >> crashes or produces incorrect polygons. >> >> Tom > > I don't think we can easily replace boost::polygon by Clipper. > > They have complementary features, but neither boost::polygon nor Clipper > have all features we need ( for instance transform a self intersecting > polygon into a set of not intersection polygons is easy with clipper, > not with boost::polygon). > > A major issue (or lack of feature) is the fact Clipper does not handle > holes inside a polygon by connecting holes by overlapping segments > (to create only one polygon), which is needed in draw (and some other) > functions Hi, I'm almost done with SHAPE_POLY_SET class for the geometry library, a Clipper wrapper adding some missing Clipper features needed in Kicad (e.g. polygon fracturing). I'll send a patch soon (affecting only AddClearanceAreasPolygonsToPolysList), so that we can compare how well it performs wrs to Boost.Polygon. Cheers, Tom > > I also tried to use Clipper to replace the polygon set subtracting which > crashes Pcbnew. > > But if Clipper gives good results to subtract 2 polygons, this is not > the case to subtract 2 set of polygons (when polygons inside each set > overlap, the result is strange, whatever the settings used in calculations). > > Therefore, replacing boost::polygon by clipper is not trivial. > > If we drop boost::polygon (due to this bug, and the lack of > maintenance), one can consider Clipper (which looks stable enough now) > but also CGAL. > Clipper needs not trivial rework to calculate the zone filling. > I do not have used CGAL, but it is a large geometry library, actively > maintained, and it could be considered before taking a decision. > ___ 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] GAL arc-drawing crash
Hi Chris, I could not reproduce the assert, but your patch surely will not harm the code, so there are chances it is going to work better. Thank you, I committed your changes in revision 5695. Regards, Orson On 06/02/2015 05:44 PM, Chris Pavlina wrote: > In GAL, an assertion (drawing_tool.cpp:925) will fail if you begin > drawing an arc, and then accidentally terminate the arc at zero angle > with the cursor /outside/ the arc radius. It seems that an attempt was > made to check for this but the conditional wasn't written quite right. > Here's a patch to fix the conditional and correctly cancel terminating > the arc instead. > > -- > Chris > > > > ___ > 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 > signature.asc Description: OpenPGP digital signature ___ 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 arc-drawing crash
In GAL, an assertion (drawing_tool.cpp:925) will fail if you begin drawing an arc, and then accidentally terminate the arc at zero angle with the cursor /outside/ the arc radius. It seems that an attempt was made to check for this but the conditional wasn't written quite right. Here's a patch to fix the conditional and correctly cancel terminating the arc instead. -- Chris diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index adb005e..d4940d0 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -920,7 +920,7 @@ bool DRAWING_TOOL::drawArc( DRAWSEGMENT*& aGraphic ) case SET_ANGLE: { -if( wxPoint( cursorPos.x, cursorPos.y ) != aGraphic->GetArcStart() ) +if( wxPoint( cursorPos.x, cursorPos.y ) != aGraphic->GetArcStart() && aGraphic->GetAngle() != 0 ) { assert( aGraphic->GetAngle() != 0 ); assert( aGraphic->GetArcStart() != aGraphic->GetArcEnd() ); ___ 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] 3D Scale settings
Hello all > No need to change file format. > .. I vote for this idea from JP. Also, we cannot assume that the scale form the models are just in mm or inch.. it could be anything. (mm, m, km, inchs, mils,... Xunities) so in the end, if the 3d model dont fit the component footprint, you have to scale it. I had a look in other EDA softwares, and they are doing it more interactive, i.e.: show the footprint+3dmodel + slides to adjust in real time the scale ... until it looks good to your eyes. Hope that will be possible in future to do with kicad also ... >It could be worth also to display offset values according to the units >selected in pcbnew (inch/mm). I vote on this too. > Historically the scaling allowed users to reuse similar models if they > didn't already have a suitable model, for example grow or shrink a THT 0.25W > resistor to pretend it's a 0.5/1.0/2.0W type or a small glass-encapsulated > diode. ... > I propose to handle these by having an orientation file which sits alongside > the model file - let's call it a ".cod" (component orientation data) file. > The cod file > gives the required scaling, 3D rotation, and offset required to put the > component > into a reference orientation suitable for KiCad; This can be already archived using VRML2 files that Kicad, 3d-viewer can read. It should support by now "inline url"syntax so it can include external files. So it should be possible to do something like: Original file: THT_0_25W.WRL Scaled version: THT_0_50W.WRL Transform { translation 0 0 -1 scale 2 2 2 children [ Inline { url "THT_0_25W.WRL" } ] } The current VRML2 parser is only at moment open other VRML2 URL files, but it can be changed in future to handle the generic file format. We may have a look in future on this file formats (VRML2/X3D) they could be useful to create new features based on their capabilities without need to change or add anything new to the current kicad file format. Regards, Mario Luzeiro ___ 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] on incorrect polygon behavior
Le 01/06/2015 21:32, Wayne Stambaugh a écrit : > On 6/1/2015 1:09 PM, jp charras wrote: >> Le 01/06/2015 18:23, Tomasz Wlostowski a écrit : >>> Hi all, >>> >>> I did a small investigation of the polygon-related >>> segfaults/miscalculations. It looks like Boost.polygon badly handles >>> cases where intersection points of the edges of the polygons lie close >>> to each other or overlap, causing the 'snap rounding' algorithm used in >>> boost to go haywire. This is the case for complex zones, with a lot of >>> thermal holes or via patterns (i.e. a thermal pad under a QFN with a via >>> field). >>> >>> See last comment from https://svn.boost.org/trac/boost/ticket/10642. >>> >>> There's a variety of effects I observed. >>> - a segfault. >>> - an incorrect or empty resulting polygon: merging the holes before >>> subtraction from the main outline (polyset_holes in >>> AddClearanceAreasPolygonsToPolysList) using boost::assign or subtracting >>> the holes one-by-one (instead of the whole set) fixes the segfault, but >>> I know at least one case where the resulting zone is incorrectly >>> calculated. I'll attach the test cases to the bug report on Launchpad >>> later in the evening. >> >> I am not sure subtracting the holes one-by-one (instead of the whole >> set) fixes the segfault. Because the segfault depends on the geometry, >> it does not happen when you changes the calculations for a given case, >> but can happen in an other case which previously did not crash. >> >>> >>> For the reasons above, I would strongly consider dropping >>> boost::polygon. There are also non-technical issues: >>> - It hasn't been maintained for ~1.5 years. The last activity of the >>> official maintainer was 3 years ago. >>> - the code is a mess (look at example below). >>> >>> >>> >>> @Jean Pierre: do you think Clipper is stable enough for our use? (at >>> least in case of a bug, it's actively developed and the author offers >>> support). I'm not asking about speed, it's not important if Kicad >>> crashes or produces incorrect polygons. >>> >>> Tom >> >> I don't think we can easily replace boost::polygon by Clipper. >> >> They have complementary features, but neither boost::polygon nor Clipper >> have all features we need ( for instance transform a self intersecting >> polygon into a set of not intersection polygons is easy with clipper, >> not with boost::polygon). >> >> A major issue (or lack of feature) is the fact Clipper does not handle >> holes inside a polygon by connecting holes by overlapping segments >> (to create only one polygon), which is needed in draw (and some other) >> functions >> >> I also tried to use Clipper to replace the polygon set subtracting which >> crashes Pcbnew. >> >> But if Clipper gives good results to subtract 2 polygons, this is not >> the case to subtract 2 set of polygons (when polygons inside each set >> overlap, the result is strange, whatever the settings used in calculations). >> >> Therefore, replacing boost::polygon by clipper is not trivial. >> >> If we drop boost::polygon (due to this bug, and the lack of >> maintenance), one can consider Clipper (which looks stable enough now) >> but also CGAL. >> Clipper needs not trivial rework to calculate the zone filling. >> I do not have used CGAL, but it is a large geometry library, actively >> maintained, and it could be considered before taking a decision. >> > > JP, > > How difficult would it be to modify the sample file you created for the > boost bug report you filed to use Clipper and cgal? At least you would > be able to verify that the result is a valid polygon before you attempt > to replace boost::polygon with either library. I realize that there > could be other cases where cgal or clipper fail and boost::polygon does not. Clipper being already in use in Kicad. We knows what works fine and what are the things it does not manage easily. CGAL needs to be learned (at least by me). This is the major work. > > I also just looked and there are mingw32 and mingw64 pacman packages of > cgal in the msys2 project so that would make life easier if we need to > switch to cgal. There is no clipper package. > > Wayne > "No clipper package" is really not an issue: Clipper is only a small .hpp file and a "small" .cpp file (140Kbytes) to link with application and does not create any compil issue. It is far from boost build issues and its build process nightmare. -- 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