← Back to team overview

kicad-developers team mailing list archive

Re: Elements visibility

 

Brian Sidebotham wrote:
2009/10/1 Isaac <isaacbavaresco@...>:

I think that the method of signaling the visibility of layers and classes of elements by the use of a bit in their colors a bit clumsy and makes harder to implement some ideas.

At first I thought that it would be too hard to change, but last night I dived into the KiCAD source code to understand it better, and found that it may not be that hard. Only 26 or so source files are involved in the procss.

1) The macro ITEM_NOT_SHOW is present in only 26 different files (less in newer revisions);
2) The member g_DesignSettings.m_LayerColor is present in only 25 source files;

Then I tentatively started to change things.
I created a member "int m_VisibleLayers" (intended to be a bit-mask) to the class EDA_BoardDesignSettings and some accessor methods ( bool IsLayerVisible(int layer), void SetLayerVisibility(int layer, bool state) ) and replaced all the occurrences of expressions like "color & ITEM_NOT_SHOW" in the sources.

It worked well, there are just five files left to change to finish the job.

Some questions:

a) Would the accessor methods overhead make any difference in performance? If so, I could declare them inline or replace them by macros, it would look almost the same;

Your strategy seems fine. I would make your IsLayerVisible() test function be an inline, because each object is tested against this. Small, single or few statement Inline functions give the compiler a chance to maximally optimize.
b) Would this break other's ongoing work? (If you think so, please let me know).


If its not in the repo, I would assume not. Basically it has been a "first to commit" arbitration scheme, with the idea that we can always move backwards if need be because after all, it is a version control system.

c) Do these changes violate any rules?

No.



Hi Isaac,

I found the whole colour system clumsy (although of course, it has
good reasons for it's roots). I changed colors from being an array of
structs to an array of a new class. This held a wxColour internally
and I also created setters and getters for visibility.

I felt it was the right way to go, and will improve the code
readability a lot. There were plenty of bits of code that would
greatly benefit from the colours being in a class.

I think as much bit-masking as possible should go.

There are a couple of messages between me and Jean-Pierre on the
subject about a month or so ago. Search for Layer Properties Dialog.

Good luck.

Best Regards,

Brian.









References