On 02/06/2014 03:18 PM, Maciej Sumiński wrote:
> Hi Dick,
> 
> Sorry for the fuss, I have reverted the commit you find wrong. To explain myself: it was done, as the C++ standard says that int is at least 16 bits long, but if you say that we are safe, then I fully trust you.
> The most important parts are commits 4658 and then 4656. The bug was caused by the fact, that I have added too many PCB_VISIBLE elements. As they are saved [kicad_plugin.cpp:650] as a 32-bit number, some of visibility settings were truncated.
> After loading a PCB and its settings, it turned out that if you have turned off 'Pads Back' in the Render tab - it influenced the GP_OVERLAY visibility and in the end many items drawn by the GAL were not visible. Even worse is the fact that you cannot enable its visibility by checking 'Pads Back' again - as in the application, internally everything is fine and visiblity settings are not in conflict. The bad thing happens in the PCB_EDIT_FRAME::syncLayerVisibilities() function - it iterates over all the PCB_VISIBLE elements calling
> 
>     bool BOARD_DESIGN_SETTINGS::IsElementVisible( int aElementCategory ) const
>     {
>         return ( m_VisibleElements & ( 1 << aElementCategory ) );
>     }
> 
> that goes out of int boundaries for aElementCategory > 31.
> In order to retain the currently used .kicad_pcb format, I decided to move the netname layers to separate group. They do not need to be saved, as they are directly related to copper layers.
> 
> Regards,
> Orson
> 


long is bad.  That's all I really said.  I don't ever use, ever.  Except when I have to to
make some 1980's C function call.



If you want 64 bits on any platform, and I think you did, then the thing to use is int64_t
but hide it the best you can.

This means don't expose any more in the *public* API than you have to to client code.

The implementation can use it fine.

Here's what I would have done, simply make the bit map hold 64 bits, the easiest way to do
that is go to int64_t.


aElementCategory can remain int, it is simply a bit number, but caller does not know that.

bool BOARD_DESIGN_SETTINGS::IsElementVisible( int aElementCategory ) const
{
     return ( m_VisibleElements & ( int64_t(1) << aElementCategory ) );
}


and make m_VisibleElements int64_t also in the class.


Then you have to doctor the Load() and Save() functions in a "machine width independent"
way.  That means a special way of handling the int64_t sprintf() call, and a way to get
back the int64_t from ascii.

snprintf() I think is *implemented* by mingw maintainers on windows, whereas sprintf() is
implemented by mvscrt.dll.  Fortunately OUTPUTSTREAM::Print() uses snprintf().  So you can
simply find the correct format string for int64_t and be done quickly.

If possible, we want a machine width independent (i.e. mingw and gcc) format string for
int64_t.  It might even be a macro.

Google __PRI64_PREFIX

and look at /usr/include/inttypes.h

The source fle which uses __PRI64_PREFIX may have to define

__STDC_FORMAT_MACROS

and include inttypes.h

I find this file on my shiny new mingw 4.8.2 cross compiler also.


Maybe you already have a solution.   I'm just saying what I would have done.



Dick