Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Am 2019-01-31 12:24, schrieb jp charras: Le 31/01/2019 à 18:21, John Beard a écrit : Hi JP, On Thu, Jan 31, 2019 at 4:55 PM jp charras wrote: Perhaps this: target_include_directories( pcbnew_kiface_objects PRIVATE $ ) Unfortunately, does not work. Is it the same error? Cheers, John Yes it is, but your second set of patches fixed the issue. Thanks. Hi John- I've pushed this patch as well as the segment tests and lib table updates. I modified the last one to change the sign of one comparison as well as update the copyright dates on the changed files. Thanks again! -Seth ___ 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] Build fixes: bitmaps and polygon
On 1/31/2019 1:15 PM, Simon Richter wrote: > Hi, > > On 31.01.19 18:11, Simon Richter wrote: > >>> I would prefer that we create a base units object and then derived >>> application level units objects as needed. These objects would be >>> small, compile quickly, and could be included in the common library so >>> they are available everywhere. Using 1nm units for schematics doesn't >>> make a lot of sense to me even if/when we do convert to 64 bit integers. > >> Hm, we could get most of that by making the unit conversion functions in >> BASE_SCREEN abstract, which would already dynamically dispatch most of >> the unit conversions. > > Okay, I've tried that, and the situation is not as good as I've expected > — we have way too much code that doesn't have a SCREEN context and thus > needs to pull the conversion factor out of the macro definition. I'm not surprised. If the fix were easy, we would have fixed it a long time ago. > > I think it would be more work to pass the a screen reference (or a unit > conversion object, so we can work without a screen) to all of these > places than it would be to define a common conversion factor for everyone. I don't think passing a screen reference around to non-ui code is a good idea. The way I see it is the application specific units would be static (singleton?) and used to initialize a BASE_SCREEN object BASE_UNITS member variable at run time. In other words when a SCH_UNITS object would be instantiated and used to set the BASE_UNITS member for SCH_SCREEN objects. This way the static SCH_UNITS object can be used for non-ui code directly and the BASE_SCREEN object can use it to perform coordinate conversions. This is just a thought but there may be a more elegant way to do it. Honestly, it's been a long time since I looked at the units code so I maybe off base here. > >Simon > > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi, On 31.01.19 18:11, Simon Richter wrote: >> I would prefer that we create a base units object and then derived >> application level units objects as needed. These objects would be >> small, compile quickly, and could be included in the common library so >> they are available everywhere. Using 1nm units for schematics doesn't >> make a lot of sense to me even if/when we do convert to 64 bit integers. > Hm, we could get most of that by making the unit conversion functions in > BASE_SCREEN abstract, which would already dynamically dispatch most of > the unit conversions. Okay, I've tried that, and the situation is not as good as I've expected — we have way too much code that doesn't have a SCREEN context and thus needs to pull the conversion factor out of the macro definition. I think it would be more work to pass the a screen reference (or a unit conversion object, so we can work without a screen) to all of these places than it would be to define a common conversion factor for everyone. Simon 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Le 31/01/2019 à 18:21, John Beard a écrit : > Hi JP, > > On Thu, Jan 31, 2019 at 4:55 PM jp charras wrote: >> >>> Perhaps this: >>> >>> target_include_directories( pcbnew_kiface_objects PRIVATE >>>$ >>> ) >> >> Unfortunately, does not work. > > Is it the same error? > > Cheers, > > John > Yes it is, but your second set of patches fixed the issue. Thanks. -- 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Le 31/01/2019 à 17:48, John Beard a écrit : > The second set of patches I just sent seems to work on the Ubuntu > docker image with CMake 3.5.1. > > Cheers, > > John Hi John, After cleaning the build directory and reappling your second set, the build works. I tested it on msys2 with cmake 3.7, and on Kubuntu 14.04 LTS (still maintained) with cmake 3.0 Thanks. > > On Thu, Jan 31, 2019 at 4:42 PM Wayne Stambaugh wrote: >> >> On 1/31/2019 11:27 AM, jp charras wrote: >>> Le 31/01/2019 à 16:52, jp charras a écrit : Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit : > JP, > > On 1/31/2019 10:11 AM, jp charras wrote: >> Le 31/01/2019 à 15:40, John Beard a écrit : >>> Hi, >>> >>> Two patches for building these libs: >>> >>> 1) Make bitmaps a "Proper" library. By splitting it off like this, the >>> includes for each of the hundreds of XPM cpp files do not need to suck >>> in WX headers. This speeds compilation by something like 10x or more >>> (it now builds in <5 seconds with -j6), with a similar reduction in >>> the size of libbitmaps.a. Also, making it a proper library allows to >>> use the CMake dependency mechanism better. >>> 2) Do the same for the CMake of the polygon libraries, again >>> simplifying all the "downstream" targets - they no longer need to >>> manually specify the linkage to polygon or the include dirs. >>> >>> Apparently, both these libraries were setting off static analysers, as >>> they were a circular dependency. For example, bitmaps required common >>> for the defs, but common required bitmaps for the images. This ended >>> up pushing all the linkage down to the final executables, which is now >>> much simpler (just link common, and you should get what you need, with >>> includes set correctly). >>> >>> Jenkins passes (MSVC and Msys2), but might be worth getting an OSX >>> build as there is a different CMake command in there! >>> >>> Cheers, >>> >>> John >> >> Hi John, >> >> I have this cmake error when trying to build Kicad with these patches: >> >> "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): >> Object library target "pcbnew_kiface_objects" may not link to >> anything." >> >> (cmake version 3.7.2, W7 32bits, msys2) >> >> > > I didn't have any problem creating a 32 bit build using msys2. It maybe > your CMake version. I'm using CMake 3.12.4. I also did a clean build > so that could the issue. This change works pretty well so I hope it's > not a CMake version issue. I don't want to bump the CMake version if we > don't have to. > > Cheers, > > Wayne I just tested with a more recent msys2 version (using cmake 3.11) on a clean build, but no success. >>> >>> After tests: >>> >>> Using cmake 3.12 works. >>> But older versions do not work. >>> >> >> Forcing CMake to 3.12 will severely limit the number of platforms that >> would be able to build kicad so this patch set doesn't seem likely to be >> accepted unless we can figure out a way to accomplish the same task with >> older versions of CMake. Ubuntu Trusty uses CMake 2.8.12 but >> trusty-updates is 3.5.1 so I would reluctant to limit cmake to a newer >> version that that. >> >> ___ >> 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 > -- 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi Wayne, On 31.01.19 17:49, Wayne Stambaugh wrote: > I would prefer that we create a base units object and then derived > application level units objects as needed. These objects would be > small, compile quickly, and could be included in the common library so > they are available everywhere. Using 1nm units for schematics doesn't > make a lot of sense to me even if/when we do convert to 64 bit integers. Hm, we could get most of that by making the unit conversion functions in BASE_SCREEN abstract, which would already dynamically dispatch most of the unit conversions. Simon 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi JP, On Thu, Jan 31, 2019 at 4:55 PM jp charras wrote: > > > Perhaps this: > > > > target_include_directories( pcbnew_kiface_objects PRIVATE > >$ > > ) > > Unfortunately, does not work. Is it the same error? 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] Build fixes: bitmaps and polygon
On 1/31/2019 11:17 AM, Simon Richter wrote: > Hi, > > On 31.01.19 16:11, jp charras wrote: > >> "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): >> Object library target "pcbnew_kiface_objects" may not link to anything." > > By the way, why are we even linking pcbnew twice? The python module and > the kiface are bitwise identical on my system, and we could probably use > CMake's builtin copy function to allow installing it twice with > different names. I thought Dick fixed this a long time ago unless someone unfixed it. > > If there is no good reason for that, that would solve a lot of issues > already including the failing QA builds that ajo's Jenkins instance reports. > >Simon > > > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Simon, On 1/31/2019 10:49 AM, Simon Richter wrote: > Hi, > > On 31.01.19 16:01, John Beard wrote: > >> I'm not really sure. I suppose it depends on the benefits it brings (and >> in future with shared common libs, perhaps it won't matter as much?) > > I doubt it makes much of a difference — the biggest hot spots in compile > time are: > > - waiting for pcbnew_wrap.cxx to be generated before we even start > compiling pcbnew objects > - cmake's dependency handling, which uses a separate dependency scan > rather than generating them on the fly like automake does > - compiling pcbnew_wrap.cxx (this is started fairly late, and takes the > longest) > - linking pcbnew three times in parallel > - linking common/pcbcommon statically because of incompatible base units > > We could probably shave off a few seconds here or there by minimizing > includes or using abstract interfaces, but that is probably not worth > pursuing on its own, but might be interesting as part of unit tests. > > TBH, I'm not even sure the kiface mechanism really buys us anything but > the option to start individual modules from the command line. I'd expect > a separate 3d-viewer to contain another copy of common/pcbcommon, so > it'd slow down linking and use more memory. > > The biggest savings we could probably get from unified units. I'd think > 64 bit nanometers would be the way to go, even if that will write a lot > of leading zeroes to memory, saving the extra code copies will > definitely offset that. I would prefer that we create a base units object and then derived application level units objects as needed. These objects would be small, compile quickly, and could be included in the common library so they are available everywhere. Using 1nm units for schematics doesn't make a lot of sense to me even if/when we do convert to 64 bit integers. > >Simon > > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Le 31/01/2019 à 17:27, John Beard a écrit :
> Hmm,
>
> Perhaps this:
>
> target_include_directories( pcbnew_kiface_objects PRIVATE
>$
> )
>
> This gives the desired effect of providing the transitive include
> dirs, but doesn't require the recent CMake ability to link OBJECT
> libraries.
Unfortunately, does not work.
>
> If every target specifies the correct libraries, we can also
> eventually avoid the sledgehammer approach of "${INC_BEFORE}" and
> "${INC_AFTER}".
>
> Cheers,
>
> John
>
> On Thu, Jan 31, 2019 at 3:52 PM jp charras wrote:
>>
>> Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit :
>>> JP,
>>>
>>> On 1/31/2019 10:11 AM, jp charras wrote:
Le 31/01/2019 à 15:40, John Beard a écrit :
> Hi,
>
> Two patches for building these libs:
>
> 1) Make bitmaps a "Proper" library. By splitting it off like this, the
> includes for each of the hundreds of XPM cpp files do not need to suck
> in WX headers. This speeds compilation by something like 10x or more
> (it now builds in <5 seconds with -j6), with a similar reduction in
> the size of libbitmaps.a. Also, making it a proper library allows to
> use the CMake dependency mechanism better.
> 2) Do the same for the CMake of the polygon libraries, again
> simplifying all the "downstream" targets - they no longer need to
> manually specify the linkage to polygon or the include dirs.
>
> Apparently, both these libraries were setting off static analysers, as
> they were a circular dependency. For example, bitmaps required common
> for the defs, but common required bitmaps for the images. This ended
> up pushing all the linkage down to the final executables, which is now
> much simpler (just link common, and you should get what you need, with
> includes set correctly).
>
> Jenkins passes (MSVC and Msys2), but might be worth getting an OSX
> build as there is a different CMake command in there!
>
> Cheers,
>
> John
Hi John,
I have this cmake error when trying to build Kicad with these patches:
"CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries):
Object library target "pcbnew_kiface_objects" may not link to anything."
(cmake version 3.7.2, W7 32bits, msys2)
>>>
>>> I didn't have any problem creating a 32 bit build using msys2. It maybe
>>> your CMake version. I'm using CMake 3.12.4. I also did a clean build
>>> so that could the issue. This change works pretty well so I hope it's
>>> not a CMake version issue. I don't want to bump the CMake version if we
>>> don't have to.
>>>
>>> Cheers,
>>>
>>> Wayne
>>
>> I just tested with a more recent msys2 version (using cmake 3.11) on a
>> clean build, but no success.
>>
>>
>> --
>> Jean-Pierre CHARRAS
--
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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
On 1/31/2019 11:27 AM, jp charras wrote: > Le 31/01/2019 à 16:52, jp charras a écrit : >> Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit : >>> JP, >>> >>> On 1/31/2019 10:11 AM, jp charras wrote: Le 31/01/2019 à 15:40, John Beard a écrit : > Hi, > > Two patches for building these libs: > > 1) Make bitmaps a "Proper" library. By splitting it off like this, the > includes for each of the hundreds of XPM cpp files do not need to suck > in WX headers. This speeds compilation by something like 10x or more > (it now builds in <5 seconds with -j6), with a similar reduction in > the size of libbitmaps.a. Also, making it a proper library allows to > use the CMake dependency mechanism better. > 2) Do the same for the CMake of the polygon libraries, again > simplifying all the "downstream" targets - they no longer need to > manually specify the linkage to polygon or the include dirs. > > Apparently, both these libraries were setting off static analysers, as > they were a circular dependency. For example, bitmaps required common > for the defs, but common required bitmaps for the images. This ended > up pushing all the linkage down to the final executables, which is now > much simpler (just link common, and you should get what you need, with > includes set correctly). > > Jenkins passes (MSVC and Msys2), but might be worth getting an OSX > build as there is a different CMake command in there! > > Cheers, > > John Hi John, I have this cmake error when trying to build Kicad with these patches: "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): Object library target "pcbnew_kiface_objects" may not link to anything." (cmake version 3.7.2, W7 32bits, msys2) >>> >>> I didn't have any problem creating a 32 bit build using msys2. It maybe >>> your CMake version. I'm using CMake 3.12.4. I also did a clean build >>> so that could the issue. This change works pretty well so I hope it's >>> not a CMake version issue. I don't want to bump the CMake version if we >>> don't have to. >>> >>> Cheers, >>> >>> Wayne >> >> I just tested with a more recent msys2 version (using cmake 3.11) on a >> clean build, but no success. >> > > After tests: > > Using cmake 3.12 works. > But older versions do not work. > Forcing CMake to 3.12 will severely limit the number of platforms that would be able to build kicad so this patch set doesn't seem likely to be accepted unless we can figure out a way to accomplish the same task with older versions of CMake. Ubuntu Trusty uses CMake 2.8.12 but trusty-updates is 3.5.1 so I would reluctant to limit cmake to a newer version that that. ___ 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] Build fixes: bitmaps and polygon
The second set of patches I just sent seems to work on the Ubuntu docker image with CMake 3.5.1. Cheers, John On Thu, Jan 31, 2019 at 4:42 PM Wayne Stambaugh wrote: > > On 1/31/2019 11:27 AM, jp charras wrote: > > Le 31/01/2019 à 16:52, jp charras a écrit : > >> Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit : > >>> JP, > >>> > >>> On 1/31/2019 10:11 AM, jp charras wrote: > Le 31/01/2019 à 15:40, John Beard a écrit : > > Hi, > > > > Two patches for building these libs: > > > > 1) Make bitmaps a "Proper" library. By splitting it off like this, the > > includes for each of the hundreds of XPM cpp files do not need to suck > > in WX headers. This speeds compilation by something like 10x or more > > (it now builds in <5 seconds with -j6), with a similar reduction in > > the size of libbitmaps.a. Also, making it a proper library allows to > > use the CMake dependency mechanism better. > > 2) Do the same for the CMake of the polygon libraries, again > > simplifying all the "downstream" targets - they no longer need to > > manually specify the linkage to polygon or the include dirs. > > > > Apparently, both these libraries were setting off static analysers, as > > they were a circular dependency. For example, bitmaps required common > > for the defs, but common required bitmaps for the images. This ended > > up pushing all the linkage down to the final executables, which is now > > much simpler (just link common, and you should get what you need, with > > includes set correctly). > > > > Jenkins passes (MSVC and Msys2), but might be worth getting an OSX > > build as there is a different CMake command in there! > > > > Cheers, > > > > John > > Hi John, > > I have this cmake error when trying to build Kicad with these patches: > > "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): > Object library target "pcbnew_kiface_objects" may not link to > anything." > > (cmake version 3.7.2, W7 32bits, msys2) > > > >>> > >>> I didn't have any problem creating a 32 bit build using msys2. It maybe > >>> your CMake version. I'm using CMake 3.12.4. I also did a clean build > >>> so that could the issue. This change works pretty well so I hope it's > >>> not a CMake version issue. I don't want to bump the CMake version if we > >>> don't have to. > >>> > >>> Cheers, > >>> > >>> Wayne > >> > >> I just tested with a more recent msys2 version (using cmake 3.11) on a > >> clean build, but no success. > >> > > > > After tests: > > > > Using cmake 3.12 works. > > But older versions do not work. > > > > Forcing CMake to 3.12 will severely limit the number of platforms that > would be able to build kicad so this patch set doesn't seem likely to be > accepted unless we can figure out a way to accomplish the same task with > older versions of CMake. Ubuntu Trusty uses CMake 2.8.12 but > trusty-updates is 3.5.1 so I would reluctant to limit cmake to a newer > version that that. > > ___ > 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] Build fixes: bitmaps and polygon
Le 31/01/2019 à 16:52, jp charras a écrit : > Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit : >> JP, >> >> On 1/31/2019 10:11 AM, jp charras wrote: >>> Le 31/01/2019 à 15:40, John Beard a écrit : Hi, Two patches for building these libs: 1) Make bitmaps a "Proper" library. By splitting it off like this, the includes for each of the hundreds of XPM cpp files do not need to suck in WX headers. This speeds compilation by something like 10x or more (it now builds in <5 seconds with -j6), with a similar reduction in the size of libbitmaps.a. Also, making it a proper library allows to use the CMake dependency mechanism better. 2) Do the same for the CMake of the polygon libraries, again simplifying all the "downstream" targets - they no longer need to manually specify the linkage to polygon or the include dirs. Apparently, both these libraries were setting off static analysers, as they were a circular dependency. For example, bitmaps required common for the defs, but common required bitmaps for the images. This ended up pushing all the linkage down to the final executables, which is now much simpler (just link common, and you should get what you need, with includes set correctly). Jenkins passes (MSVC and Msys2), but might be worth getting an OSX build as there is a different CMake command in there! Cheers, John >>> >>> Hi John, >>> >>> I have this cmake error when trying to build Kicad with these patches: >>> >>> "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): >>> Object library target "pcbnew_kiface_objects" may not link to anything." >>> >>> (cmake version 3.7.2, W7 32bits, msys2) >>> >>> >> >> I didn't have any problem creating a 32 bit build using msys2. It maybe >> your CMake version. I'm using CMake 3.12.4. I also did a clean build >> so that could the issue. This change works pretty well so I hope it's >> not a CMake version issue. I don't want to bump the CMake version if we >> don't have to. >> >> Cheers, >> >> Wayne > > I just tested with a more recent msys2 version (using cmake 3.11) on a > clean build, but no success. > After tests: Using cmake 3.12 works. But older versions do not work. -- 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi, On 31.01.19 16:11, jp charras wrote: > "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): > Object library target "pcbnew_kiface_objects" may not link to anything." By the way, why are we even linking pcbnew twice? The python module and the kiface are bitwise identical on my system, and we could probably use CMake's builtin copy function to allow installing it twice with different names. If there is no good reason for that, that would solve a lot of issues already including the failing QA builds that ajo's Jenkins instance reports. Simon 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Le 31/01/2019 à 16:29, Wayne Stambaugh a écrit : > JP, > > On 1/31/2019 10:11 AM, jp charras wrote: >> Le 31/01/2019 à 15:40, John Beard a écrit : >>> Hi, >>> >>> Two patches for building these libs: >>> >>> 1) Make bitmaps a "Proper" library. By splitting it off like this, the >>> includes for each of the hundreds of XPM cpp files do not need to suck >>> in WX headers. This speeds compilation by something like 10x or more >>> (it now builds in <5 seconds with -j6), with a similar reduction in >>> the size of libbitmaps.a. Also, making it a proper library allows to >>> use the CMake dependency mechanism better. >>> 2) Do the same for the CMake of the polygon libraries, again >>> simplifying all the "downstream" targets - they no longer need to >>> manually specify the linkage to polygon or the include dirs. >>> >>> Apparently, both these libraries were setting off static analysers, as >>> they were a circular dependency. For example, bitmaps required common >>> for the defs, but common required bitmaps for the images. This ended >>> up pushing all the linkage down to the final executables, which is now >>> much simpler (just link common, and you should get what you need, with >>> includes set correctly). >>> >>> Jenkins passes (MSVC and Msys2), but might be worth getting an OSX >>> build as there is a different CMake command in there! >>> >>> Cheers, >>> >>> John >> >> Hi John, >> >> I have this cmake error when trying to build Kicad with these patches: >> >> "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): >> Object library target "pcbnew_kiface_objects" may not link to anything." >> >> (cmake version 3.7.2, W7 32bits, msys2) >> >> > > I didn't have any problem creating a 32 bit build using msys2. It maybe > your CMake version. I'm using CMake 3.12.4. I also did a clean build > so that could the issue. This change works pretty well so I hope it's > not a CMake version issue. I don't want to bump the CMake version if we > don't have to. > > Cheers, > > Wayne I just tested with a more recent msys2 version (using cmake 3.11) on a clean build, but no success. -- 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi, On 31.01.19 16:29, Wayne Stambaugh wrote: > I didn't have any problem creating a 32 bit build using msys2. It maybe > your CMake version. I'm using CMake 3.12.4. I also did a clean build > so that could the issue. This change works pretty well so I hope it's > not a CMake version issue. I don't want to bump the CMake version if we > don't have to. Yes, flags on object libraries require recent CMake. Simon 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi, On 31.01.19 16:01, John Beard wrote: > I'm not really sure. I suppose it depends on the benefits it brings (and > in future with shared common libs, perhaps it won't matter as much?) I doubt it makes much of a difference — the biggest hot spots in compile time are: - waiting for pcbnew_wrap.cxx to be generated before we even start compiling pcbnew objects - cmake's dependency handling, which uses a separate dependency scan rather than generating them on the fly like automake does - compiling pcbnew_wrap.cxx (this is started fairly late, and takes the longest) - linking pcbnew three times in parallel - linking common/pcbcommon statically because of incompatible base units We could probably shave off a few seconds here or there by minimizing includes or using abstract interfaces, but that is probably not worth pursuing on its own, but might be interesting as part of unit tests. TBH, I'm not even sure the kiface mechanism really buys us anything but the option to start individual modules from the command line. I'd expect a separate 3d-viewer to contain another copy of common/pcbcommon, so it'd slow down linking and use more memory. The biggest savings we could probably get from unified units. I'd think 64 bit nanometers would be the way to go, even if that will write a lot of leading zeroes to memory, saving the extra code copies will definitely offset that. Simon 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
JP, On 1/31/2019 10:11 AM, jp charras wrote: > Le 31/01/2019 à 15:40, John Beard a écrit : >> Hi, >> >> Two patches for building these libs: >> >> 1) Make bitmaps a "Proper" library. By splitting it off like this, the >> includes for each of the hundreds of XPM cpp files do not need to suck >> in WX headers. This speeds compilation by something like 10x or more >> (it now builds in <5 seconds with -j6), with a similar reduction in >> the size of libbitmaps.a. Also, making it a proper library allows to >> use the CMake dependency mechanism better. >> 2) Do the same for the CMake of the polygon libraries, again >> simplifying all the "downstream" targets - they no longer need to >> manually specify the linkage to polygon or the include dirs. >> >> Apparently, both these libraries were setting off static analysers, as >> they were a circular dependency. For example, bitmaps required common >> for the defs, but common required bitmaps for the images. This ended >> up pushing all the linkage down to the final executables, which is now >> much simpler (just link common, and you should get what you need, with >> includes set correctly). >> >> Jenkins passes (MSVC and Msys2), but might be worth getting an OSX >> build as there is a different CMake command in there! >> >> Cheers, >> >> John > > Hi John, > > I have this cmake error when trying to build Kicad with these patches: > > "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): > Object library target "pcbnew_kiface_objects" may not link to anything." > > (cmake version 3.7.2, W7 32bits, msys2) > > I didn't have any problem creating a 32 bit build using msys2. It maybe your CMake version. I'm using CMake 3.12.4. I also did a clean build so that could the issue. This change works pretty well so I hope it's not a CMake version issue. I don't want to bump the CMake version if we don't have to. Cheers, Wayne ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Le 31/01/2019 à 15:40, John Beard a écrit : > Hi, > > Two patches for building these libs: > > 1) Make bitmaps a "Proper" library. By splitting it off like this, the > includes for each of the hundreds of XPM cpp files do not need to suck > in WX headers. This speeds compilation by something like 10x or more > (it now builds in <5 seconds with -j6), with a similar reduction in > the size of libbitmaps.a. Also, making it a proper library allows to > use the CMake dependency mechanism better. > 2) Do the same for the CMake of the polygon libraries, again > simplifying all the "downstream" targets - they no longer need to > manually specify the linkage to polygon or the include dirs. > > Apparently, both these libraries were setting off static analysers, as > they were a circular dependency. For example, bitmaps required common > for the defs, but common required bitmaps for the images. This ended > up pushing all the linkage down to the final executables, which is now > much simpler (just link common, and you should get what you need, with > includes set correctly). > > Jenkins passes (MSVC and Msys2), but might be worth getting an OSX > build as there is a different CMake command in there! > > Cheers, > > John Hi John, I have this cmake error when trying to build Kicad with these patches: "CMake Error at pcbnew/CMakeLists.txt:645 (target_link_libraries): Object library target "pcbnew_kiface_objects" may not link to anything." (cmake version 3.7.2, W7 32bits, msys2) -- 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
Re: [Kicad-developers] [PATCH] Build fixes: bitmaps and polygon
Hi Tom, I'm not really sure. I suppose it depends on the benefits it brings (and in future with shared common libs, perhaps it won't matter as much?) Doesn't separate KiFace imply a separate unit test and utility tools too, which would be a bit of extra overhead? Unless you can still combine? Cheers, John On 31 January 2019 14:38:33 GMT, Tomasz Wlostowski wrote: >On 31/01/2019 15:40, John Beard wrote: >> Hi, >> >> Two patches for building these libs: >> >> 1) Make bitmaps a "Proper" library. By splitting it off like this, >the >> includes for each of the hundreds of XPM cpp files do not need to >suck >> in WX headers. This speeds compilation by something like 10x or more >> (it now builds in <5 seconds with -j6), with a similar reduction in >> the size of libbitmaps.a. Also, making it a proper library allows to >> use the CMake dependency mechanism better. >> 2) Do the same for the CMake of the polygon libraries, again >> simplifying all the "downstream" targets - they no longer need to >> manually specify the linkage to polygon or the include dirs. > >Hi John, > >Looks good. > >Do you think it would make sense (for the sake of faster builds) to >factor out the 3D viewer into a separate KiFace? > >Tom ___ 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] Build fixes: bitmaps and polygon
On 31/01/2019 15:40, John Beard wrote: > Hi, > > Two patches for building these libs: > > 1) Make bitmaps a "Proper" library. By splitting it off like this, the > includes for each of the hundreds of XPM cpp files do not need to suck > in WX headers. This speeds compilation by something like 10x or more > (it now builds in <5 seconds with -j6), with a similar reduction in > the size of libbitmaps.a. Also, making it a proper library allows to > use the CMake dependency mechanism better. > 2) Do the same for the CMake of the polygon libraries, again > simplifying all the "downstream" targets - they no longer need to > manually specify the linkage to polygon or the include dirs. Hi John, Looks good. Do you think it would make sense (for the sake of faster builds) to factor out the 3D viewer into a separate KiFace? Tom ___ 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
