change active layer icon in pcbnew changes all the time

Bug #1838158 reported by Oleg Endo on 2019-07-27
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Low
Ian McInerney

Bug Description

The icon for "change active layer" in pcbnew differs when it is launched for the first time.

When kicad is freshly started, pcbnew opened from the main window, I get icon 1. After closing pcbnew and starting it again, I get icon 2.

Application: KiCad
Version: 6.0.0-unknown-2af41e8~86~ubuntu18.04.1, release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.58.0 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.4 libpsl/0.19.1 (+libidn2/2.0.4) nghttp2/1.30.0 librtmp/2.3
Platform: Linux 4.15.0-54-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 3.22
    Boost: 1.65.1
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.58.0
    Compiler: GCC 7.4.0 with C++ ABI 1011

Build settings:
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

Revision history for this message
Oleg Endo (oleg.endo) wrote :
description: updated
tags: added: pcbnew ui
Changed in kicad:
importance: Undecided → Low
milestone: none → 6.0.0-rc1
status: New → Triaged
Revision history for this message
Ian McInerney (imcinerney) wrote :

There is an inconsistency with the icon in 5.1 as well:

Icon 1 is shown in the toolbar, but the menu item in the tools menu to open the same dialog is using icon 2.

Changed in kicad:
milestone: 6.0.0-rc1 → 5.1.4
Revision history for this message
Jeff Young (jeyjey) wrote :

The menu icon is supposed to be different. We use a special icon that's rendered with the actual layer colors in the toolbar, but not in the menu.

Changed in kicad:
assignee: nobody → Jeff Young (jeyjey)
status: Triaged → In Progress
Revision history for this message
Jeff Young (jeyjey) wrote :

The wrong icon in the toolbar on second launch doesn't reproduce on OSX, but I suspect it's because the LLDB compiler initialized stack memory to 0 and GCC doesn't. So I've merged a fix that might address it.

I'll wait for confirmation before cherry-picking it to 5.1.

Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

Fixed in revision ee1be14b62334b4bb0782a04dc9d7b1d4b8bfce6
https://git.launchpad.net/kicad/patch/?id=ee1be14b62334b4bb0782a04dc9d7b1d4b8bfce6

Changed in kicad:
status: In Progress → Fix Committed
Revision history for this message
Michael Kavanagh (michaelkavanagh) wrote :

Hmm, no luck unfortunately, I can still reproduce this. Interestingly, on the second launch if you change the layer pair in the dialog, the correct icon reappears in the toolbar.

Application: Pcbnew
Version: (5.1.0-1383-gee1be14b6), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.54.0 LibreSSL/2.6.5 zlib/1.2.11 nghttp2/1.24.1
Platform: Mac OS X (Darwin 18.6.0 x86_64), 64 bit, Little endian, wxMac
Build Info:
    wxWidgets: 3.0.4 (wchar_t,STL containers,compatible with 2.8)
    Boost: 1.70.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.54.0
    Compiler: Clang 10.0.1 with C++ ABI 1002

Build settings:
    KICAD_SCRIPTING=OFF
    KICAD_SCRIPTING_MODULES=OFF
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

Revision history for this message
Jeff Young (jeyjey) wrote :

@Michael, I've merged another attempt. Let me know if it works when you get a chance.

Revision history for this message
Ian McInerney (imcinerney) wrote :

@Jeff, this actually isn't due to the lack of static variable initialization. What seems to happen is that the static variables maintain their value across the pcbnew instances on Linux (so the .so is probably not unloaded). This means the checks saying if the bitmap should be rebuilt say not to when pcbnew is run the second time.

To fix this, we just need to force the bitmap to be rebuilt on menu creation. See the attached patch.

Changed in kicad:
status: Fix Committed → In Progress
Revision history for this message
Ian McInerney (imcinerney) wrote :

@Jeff, I just tried the fix you committed the second time around and it still doesn't give the right bitmap on the second opening. The problem is still probably the use of the static variable to say if it was initialized.

Note that my patch was actually made from your first attempt (before your second), and it fixed the issue on my Linux machine.

Revision history for this message
Jeff Young (jeyjey) wrote :

Wow, that's kind of scary. I wonder what else that breaks...

Thanks for the patch!

Revision history for this message
Ian McInerney (imcinerney) wrote :

Yea, static variables and C++ are kind of scary sometimes.

The current system is probably not going to be multi-instance safe either. The static variables will be shared between every instance of PCB_EDIT_FRAME, so if someone has 1 instance of pcbnew with top/bottom selected and another with inner1/inner2 selected, then the toolbar updates will constantly regenerate the icons because each update will keep switching the value in the static variable to their values. This kind of information should probably be moved into a member struct containing the previous colors so that it stays with the object (also, that would eliminate the need to have this boolean argument since we can initialize them to default when constructed).

The use of statics currently to hold dialog settings is probably fine since those shouldn't break anything being shared across instances, but we need to be careful about other uses.

Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

Fixed in revision b70df18939ba382eb6940f7ecc78e9cb54c13852
https://git.launchpad.net/kicad/patch/?id=b70df18939ba382eb6940f7ecc78e9cb54c13852

Changed in kicad:
status: In Progress → Fix Committed
assignee: Jeff Young (jeyjey) → Ian McInerney (imcinerney)
Revision history for this message
Jeff Young (jeyjey) wrote :

Cherry-picking didn't work out because of other changes, so I implemented another fix.

Revision history for this message
Oleg Endo (oleg.endo) wrote :

Which icon (see my screenshot above) is supposed to be displayed now actually?

Revision history for this message
Jeff Young (jeyjey) wrote :

Icon #1 (although the colours will be modified to match the two layers currently chosen).

Revision history for this message
Oleg Endo (oleg.endo) wrote :

OK, thanks. Seems to be working now.

Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers