Code review comment for lp:~cern-kicad/kicad/bugfix_1275319

Revision history for this message
Maciej Suminski (orsonmmz) 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

« Back to merge proposal