PCBNew: Ratsnest always visible while updating board with new parts

This happens for any parts that are newly added to the board during the update/import step. Ratsnest lines that already exist remain hidden, it is just the new lines that are visible.

The default value for the ratsnest attribute of pads was 'true'

Patch proposal:
-The local ratsnest attribute is changed from bool to char.
-The local ratsnest has three possible values :
---BOARD_CONNECTED_ITEM_RATSNEST_FALSE 00
---BOARD_CONNECTED_ITEM_RATSNEST_TRUE 01
---BOARD_CONNECTED_ITEM_RATSNEST_UNSET 02
-When 'unset', the the value is equivalent to true (as before)
-When drawing, if a component has its local ratsnest attribute 'unset', set it to the global ratsnest setting.

If found another case. If the rats nest is disabled an I draw a new wire but terminate it in the middle of nowhere instead of connecting it to another pad/trace, a new rats nest line appears. See attached GIF!

Can reproduce this other easily case in my nightly version of kicad.
Can't reproduce it when the patch I submitted earlier is applied.

tracks and pads are both BOARD_CONNECTED_ITEMS. The changes I made apply to both.
(When a track is created, its ratsnest property was set to 'true'. In the patched version, it's set to 'unset', and get the global property)

However, what DOES happen with the patch for tracks is:
-If a pad has a track attached to it, the rastnest does not reappear when toggling the global setting.

I'll try to understand why.

Nervermind, I had some uncommitted changes that I used as a test, which turned it off.

So far, the previous patch seems to work smoothly on both issues.

Ok, thanks. I'm still using the 5.1.4 stable and just wanted to mention it in case there is a different issue.

@Fabien- I would prefer that we don't introduce a third state unless required. This is creating some worrying logic loops in the draw ratsnest routine.

Can the new part simply acquire the current global visibility?

I tried for quite a long time not to introduce the unset state, but the local visibility should be set equal to the global visibility in the "Netlist update" part of the code. I found where it should go in the code. In that context, I can find a way pad -> component -> board.
But from the board, I didn't find a way to the PCB_painter, nor the visibility options.
Maybe there is, but I didn't find one using the online doxygen.

Such solution would not fix the tracks problem Enrico identified. (it would require another patch)

What I checked :
-Using 'grep', there is no call to getLocalRatsnestVisible() that goes like:
if ( getLocalRatsnestVisible() == true) or if (getLocalRatsnestVisible() == false)
only
if (getLocalRatsnestVisible()).
So there should not be any difference between 'true', 'false' and '0x00', '0x01'.

The default value was 'true'. the 'unset' value is '0x02' which leads to 'true' when evaluated.

@Fabien,

Instead of trying to set the default value in the BOARD_CONNECTED_OBJECT class constructor, it would probably be better to just modify it after the module is added to the board. Note that this is how the ratsnest is updated when modules are exchanged (see PCB_EDIT_FRAME::Exchange_Module inside dialog_exchange_footprints.cpp).

I think something similar could be done for the newly added modules inside the BOARD_NETLIST_UPDATER::addNewComponent function, where there you could set the local ratsnest field using its setter to be the current settings value for the new modules only.

The ratsnest after routing will require a different patch, since that would be due to an issue in the router.

@Enrico, can you file a different issue for the router track ratsnest so we can track it separately from this one?

That was my first approach.

I would need to access KIGFX::PCB_RENDER_SETTINGS::GetGlobalRatsnestLinesEnabled ()
In
BOARD_NETLIST_UPDATER::addNewComponent()
where I could simply set the local ratsnest setting for each pad to the global setting.

The function I need,GetGlobalRatsnestLinesEnabled (), is defined in:
-PCB_RENDER_SETTINGS::PCB_RENDER_SETTINGS

I don't know much of Kicad source code yet, but I don't see any way to get the KIGFX::PCB_RENDER_SETTINGS instance from a BOARD

-----------------

Alternatively, I could use the class that sets the KIGFX::PCB_RENDER_SETTINGS using PCB_RENDER_SETTINGS::LoadDisplayOptions. That function is used by:

-PANEL_PCBNEW_DISPLAY_OPTIONS::TransferDataFromWindow-> A window frame
-FOOTPRINT_VIEWER_FRAME::ApplyDisplaySettingsToGAL -> A window frame

Once again, I am not sure how to link any these class to the board or the netlist updater.

-----------------

PCB_EDIT_FRAME::Exchange_Module does not access the global ratsnest settings, but only copies local settings.
------------------

One solution I can think of, is having a class variable in KIGFX::PCB_RENDER_SETTINGS, shared among all instances, so the netlist updater could instantiate one of them, and then getting the global setting value. But I have no idea what the side effects could be.

Since you are only reading it, you can get the copy from
PCB_TOOL_BASE::displayOptions()->m_ShowGlobalRatsnest

@Seth, the BOARD_NETLIST_UPDATER class exists outside the tool framework and that function is not static, so that won't work.

@Fabien, I think you should be able to access the setting by doing
m_frame->GetDisplayOptions()->m_ShowGlobalRatsnest
from inside the BOARD_NETLIST_UPDATER class (we pass in the PCB_EDIT_FRAME that created the updater, so we can access its settings through that).

Thank you both!
It took me so much time for understanding all of this, but doing this is definitely better than the first patch

@Fabien, @Ian: Does this patch include a fix for the track issue as well or should I still file a second bug report?

@Enrico, no this patch only covers the pad issue.

Ok, filed a new report at https://bugs.launchpad.net/kicad/+bug/1851611

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

@Fabien-

Thank you for your patch. I've pushed it to the master branch but reset the status to in-progress as it needs to be cherry-picked into 5.1.6 when that branch is opened.

There were some formatting issues in your patch that I addressed in a follow-on patch. The follow-on was something of a rabbit-hole as it exposed our left-over routines from years past that we've been patching over for a while.

When this is cherry-picked, we should remember to touch-up the formatting at the same time.

For future reference, C-style casts are discouraged (I know, they are everywhere still). Instead, please use static_cast<TYPE>() or dynamic_cast<TYPE>() and spaces go inside all parentheses.

Cherry picked to 5.1 in commit e299343807dd691229878f159127fe60fd7a00be (https://git.launchpad.net/kicad/commit/?id=e299343807dd691229878f159127fe60fd7a00be).

Note that a slight alteration had to be made since the ratsnest visibility is not in the display settings struct in 5.1, so it had to be loaded from the board object.