← Back to team overview

kicad-developers team mailing list archive

Re: About ayer widget

 

jean-pierre.charras@... wrote:
Dick,
In my last commit, i wrote PCB_LAYER_WIDGET::RenderSynchronize( ) to update the check boxes state in the layer manager when an item visibility is changed outside the layer manager (from the left toolbar for instance, or some hot keys) I do not like the code I wrote, because i can be easily broken if the layer manager is modified.
But Checkboxes can be updated without calling Refill() that creates flicker.
The main problem I found is the fact it is not easy to reach the check boxes in the layer manager because the pointer to these check boxes exist only in sizers.

Because it could be interesting to update more easily these check boxes with very few new code, i believe an easy way to do that it to set (for check boxes we want to update) a wxValidator, and just call TransferDataToWindow to update all checkboxes

from PCB_LAYER_WIDGET class, the requirement is just use bools (and not a mask) to handle items visibility, but it should not be a problem.

Inside layer_widget the changes are small :
void AppendRenderRow and AppendLayerRow could return a pointer to the corresponding checkbox,
and AppendLayerRows and AppendRenderRows an array of pointers.
PCB_LAYER_WIDGET functions could set a wxValidator for check boxes (or store the list of checkboxes, if we do not use validators)

Inside the EDA_BoardDesignSettings class, use an array of bools to handle enabled and visible layers and items
but they are not directly acceded, so the cost is small.

I never used validators and TransferDataToWindow , so I like to know your thoughts about this.


Jean-Pierre,


wxValidator and TransferDataToWindow do not meet our needs precisely because as you say there can be multiple controls associated with a single piece of data. If a SINGLE validator could manage more than one control the fit might be better. If we need two validators, then there is an easier approach described below.

I also don't like exposing pointers to wxWindows within the LAYER_WIDGET. A black box approach is better for this widget. Admittedly, the black box needs to have sufficient transaction capability. LAYER_WIDGET needs a couple more functions, but none that expose the internals more than what is already known. What is already known are the integer "IDs" of the two sets of rows, since the client code already provided those via the LAYER_WIDGET::ROW.

I will add to LAYER_WIDGET: SetRenderState(), GetRenderState(), SetLayerColor(), GetLayerColor().


Now on to the essence of your question:


*************************************************************
How do we best tie a data variable to two or more wxWindows?
*************************************************************

My answer is to provide accessors to each data variable, these are the "set" and "get" type functions. They need to be added to the proper place in the class and program hierarchy, not too high, not too low. My suggestion is WinEDA_PcbFrame, since this is the controller of the PCBNEW, in a MVC sense.

By placing the accessor at the correct place in the class and program hierarchy, accessor can have knowledge of the known listening (interested) controls of the data, these are controls which need to be notified of changes in the data. WinEDA_PcbFrame knows about basically everything, so it is a good place to add accessors. A function like IsElementVisible( aId ) can act as an accessor for more than one piece of data, so we don't need a separate function for each piece of data.

When the "set" accessor changes the data value within the data structure, wherever that data actually is (and the caller really does not care where that is exactly), the set accessor then also notifies the know listening controls, right then and there. See setActiveLayer().

The only trick is that sometimes this can lead to recursion, but wxWidgets has been designed of late so that when you set the state of a control from outside it programmatically, it will not fire an event. (The event is only fired when the user changes the state, i.e. a non-programmatic change.) So a checkbox listener simply calls the set accessor, the set accessor changes the data and notifies all the controls who are interested in the state of this data. Yes, it may notified original checkbox too, but this should not fire another checkbox change event, according to what I said above.


See my comments at P2 in the TODO.txt file.

This is something that I was planning working on, as you can see in TODO.txt.

Accessor are a concept that can solve a lot of problems, and they are helpful for debugging also, because you can set breakpoints in the set accessor function, and trap all changes to a data value. This is a very commonly used technique. These are sorely missing within Kicad as a whole, probably because it did not fully evolve from C to C++.


Hope this helps,

Dick







Follow ups

References