kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 09/09/2011 12:10 AM, Dick Hollenbeck wrote:
> On 09/08/2011 11:43 AM, fabrizio wrote:
>> Hello,
>>
>> The following patch implement the following:
>> - fix icon margins in KiCad program icons
>> - remove metal background in all icons
>> - fix some icons here and there (save button is now the standard one)
>> - fix the icons "pcb calculator" and "bitmap2component" in KiCad
>> - fixed button image.svg
>> - fix a building problem in the icon CMakeLists file
>> - some other minor stuff
>>
>> after the patch is applied all icons need to be rebuild with:
>> cmake ../ -DKICAD_TESTING_VERSION=ON -DCMAKE_BUILD_TYPE=Debug -DMAINTAIN_PNGS=ON
> Well this should not technically be true for the following reason:
>
> The bitmaps_png/CMakeLists.txt file is designed to allow folks to build Kicad
> and NOT be PNG maintainers.  This means the PNG images are already built in our
> source tree as *.cpp files.
>
> This means your patch is broken if you think people should be building the
> cpp_26/*.cpp files.
>
> These files are part of the repo, and do not have to be rebuilt, ever, unless
> you want to be a maintainer.
> So what is wrong with the patch? 
>
> 1) it did not include the pre-built *.cpp files, according to your own words.
>
> Dick

However, Fabio's commit did rebuild the *.cpp files beforehand.  Nice job Fabio,
and the executable find stuff too.

Line 556 is still using pngcrush instead of ${pngcrush_EXECUTABLE}


Some minor observations on strategy:

A) Since these bitmaps are compiled by the CPP compiler back to binary, whatever
texts were in the original PNG file remain in the Kicad binary program. 
Therefore I felt it appropriate to remove unnecessary texts.  You could a linux
"strings" program to see all the strings in these PNGs, and I did not want to
leave a bread crumb trail in there.

B) I am pretty sure that since these PNGs are const data, that it is put into a
read only segment on most if not all platforms, in the same way that CODE is
read only.  And because it is read only, it can be shared across process
boundaries if it were to be put into a *.dll or a *.so  (DSO), in the same why
that code (but not data) is shared from a single DSO memory instance between
processes.  This means that using a DSO is an option to consider moving forward,
and may be a way to support more than one bitmap size by having alternative
swappable DSOs.  I think the remaining programs will link slightly faster if the
DSO were in play.  But the downside is that you have to put the DSO into the
right place so that the main program can find it.  Food for thought.  I am not
thinking of doing this myself however.