Merge lp:~cern-kicad/kicad/bugfix_1275319 into lp:kicad/product

Proposed by Maciej Suminski
Status: Merged
Merged at revision: 4701
Proposed branch: lp:~cern-kicad/kicad/bugfix_1275319
Merge into: lp:kicad/product
Diff against target: 461 lines (+106/-77)
11 files modified
include/class_board_design_settings.h (+7/-5)
include/layers_id_colors_and_visibility.h (+29/-21)
pcbnew/basepcbframe.cpp (+22/-22)
pcbnew/class_board_design_settings.cpp (+13/-0)
pcbnew/class_pad.cpp (+3/-3)
pcbnew/class_track.cpp (+2/-2)
pcbnew/pcb_painter.cpp (+14/-13)
pcbnew/pcb_painter.h (+1/-0)
pcbnew/pcbframe.cpp (+13/-10)
pcbnew/router/router_preview_item.cpp (+1/-0)
pcbnew/router/router_preview_item.h (+1/-1)
To merge this branch: bzr merge lp:~cern-kicad/kicad/bugfix_1275319
Reviewer Review Type Date Requested Status
KiCad Lead Developers Pending
Review via email: mp+205166@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :

On 02/06/2014 08:17 AM, Maciej Sumiński wrote:
> Maciej Sumiński has proposed merging lp:~cern-kicad/kicad/bugfix_1275319 into lp:kicad.
>
> Requested reviews:
> kicad-product-committers (kicad-product-committers)
> Related bugs:
> Bug #1275319 in KiCad: "pcbnew OpenGL PNS router does not show track while routing"
> https://bugs.launchpad.net/kicad/+bug/1275319
>
> For more details, see:
> https://code.launchpad.net/~cern-kicad/kicad/bugfix_1275319/+merge/205166
>

on a 32 bit platform, a long and an int are both 32 bits.

I stopped reading the patch after I saw that. I don't like long where an "int" will
suffice, or where an int and a long are the same thing. long really should never be used.
 Because if you need a certain bitness, then there are datatypes which ensure that.

int64_t comes to mind.

The rest seems like mostly noise and renaming. Where is the actual fix? What lines,
maybe you can name two or 3 that fix the code.

Dick

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :

On 02/06/2014 11:53 AM, Dick Hollenbeck wrote:
> On 02/06/2014 08:17 AM, Maciej Sumiński wrote:
>> Maciej Sumiński has proposed merging lp:~cern-kicad/kicad/bugfix_1275319 into lp:kicad.
>>
>> Requested reviews:
>> kicad-product-committers (kicad-product-committers)
>> Related bugs:
>> Bug #1275319 in KiCad: "pcbnew OpenGL PNS router does not show track while routing"
>> https://bugs.launchpad.net/kicad/+bug/1275319
>>
>> For more details, see:
>> https://code.launchpad.net/~cern-kicad/kicad/bugfix_1275319/+merge/205166
>>
>
>
> on a 32 bit platform, a long and an int are both 32 bits.
>
> I stopped reading the patch after I saw that. I don't like long where an "int" will
> suffice, or where an int and a long are the same thing. long really should never be used.
> Because if you need a certain bitness, then there are datatypes which ensure that.
>
> int64_t comes to mind.
>
>
> The rest seems like mostly noise and renaming. Where is the actual fix? What lines,
> maybe you can name two or 3 that fix the code.
>
> Dick
>

I do not mind the renaming and the clean up, just the long.

(But amidst the renaming, was unable to see the fix.)

Go for clean up where ever you can! Just recognize what that means :)

lp:~cern-kicad/kicad/bugfix_1275319 updated
4659. By Maciej Suminski

Reverted changes introduced by the revision 4655.

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

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :
Download full text (3.1 KiB)

On 02/06/2014 03:18 PM, Maciej Sumiński 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
>

long is bad. That's all I really said. I don't ever use, ever. Except when I have to to
make some 1980's C function call.

If you want 64 bits on any platform, and I think you did, then the thing to use is int64_t
but hide it the best you can.

This means don't expose any more in the *public* API than you have to to client code.

The implementation can use it fine.

Here's what I would have done, simply make the bit map hold 64 bits, the easiest way to do
that is go to int64_t.

aElementCategory can remain int, it is simply a bit number, but caller does not know that.

bool BOARD_DESIGN_SETTINGS::IsElementVisible( int aElementCategory ) const
{
     return ( m_VisibleElements & ( int64_t(1) << aElementCategory ) );
}

and make m_VisibleElements int64_t also in the class.

Then you have to doctor the Load() and Save() functions in a "machine width independent"
way. That means a special way of handling the int64_t sprintf() call, and a way to get
back the int64_t from ascii.

snprintf() I think is *implemented* by mingw maintainers on windows, whereas sprintf() is
implemented by mvscrt.dll. Fortunately OUTPUTSTREAM::Print() uses snprintf(). So you can
simply find the correct format string for int64_t and be done quickly.

If possible, we want a machine width independent (i.e. mingw and gcc) format string for
int64_t. It might even be a macro.

Google __PRI64_PREFIX

and look at /usr/include/inttypes.h

The source fle which uses __PRI64_PREFIX may have to define

__STDC_FORMAT_MACROS

and include inttypes.h

I find this file on my shiny new mingw 4.8.2 cross compiler also.

Maybe you already have a solution. I'm just saying what I would have done.

...

Read more...

Revision history for this message
Maciej Suminski (orsonmmz) wrote :

It is not my intention to increase the number of bits in m_VisibleElements - I prefer to leave it as it is now, particularly that it is saved as a 32-bit number in .kicad_pcb files and I would like to preserve compatibility (so if older versions expect 32-bit number there - let them have it and do not crash/do weird things on load of PCBs holding 64 bits for elements visiblity). Saving netnames layers visibility is not really crucial, it can be restored basing on visibility settings of appropriate copper layers.
The change from int to long resulted only from the fact that in the back of my head I remember that int is guaranteed to be 16 bits, so maybe on some platforms the code would not run as expected. But as it works on the major platforms then I simply do not insist on changing that.
Anyway - the patch fixes the problem even when m_VisibleElements stays as integer, so what do you think about the rest of the changes?

Regards,
Orson

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :

On 02/06/2014 04:26 PM, Maciej Sumiński wrote:

> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
> to leave it as it is now, particularly that it is saved as a 32-bit number in
> .kicad_pcb files and I would like to preserve compatibility (so if older versions
> expect 32-bit number there

Note, that if we were always bound to allow older versions of KiCad to load newer files,
then it would be impossible to add new features which needed saving on disk.

I value new features far more. We can all stop writing code and wasting time on a mailing
list if we are not to write features requiring file format changes. It would essentially
mean that we could only make improvements which do not require change.

The pay is too low already. If you can find a way to make improvements without also
making changes, then I am in the wrong universe.

> - let them have it and do not crash/do weird things on load
> of PCBs holding 64 bits for elements visiblity).

Not really an issue, if you parse an ASCII 64 bit number into a 32 bit binary integer, in
old code, the lower 32 bits are likely preserved. So in this case you might have been OK.
 If not, folks can stick with their old software and old BOARDs.

> Saving netnames layers visibility is
> not really crucial, it can be restored basing on visibility settings of appropriate
> copper layers.

I like the feature! And I like the 33rd feature which I want to save to disk, whatever it is.

I would like to save it to disk, and have it be BOARD centric, not project centric.

Eventually we'll hit some kind of wall, if not here, now, then sometime.
Saving a bit map as an ASCII integer is only one possibility anyways. There are numerous
alternatives for expansion:

a) save a 64 bit integer in ASCII.

b) use s-expressions to name each bit.

c) An arbitrartily long ASCII binary number
0000_1011_1110_0011....

This can extend far off the page if needed.

Parsing that would be pretty easy, the underscore saves a human some counting. At some
point you quietly switch to a bitmap of arbitrary length. But since you've hidden the
implementation from the client code, that is nobody's damn business.

d) blah blah blah

> The change from int to long resulted only from the fact that in the back of my head I
> remember that int is guaranteed to be 16 bits,

Fully incomprehensible. I don't know where 16 bits even enters into it. Find a computer
that uses 16 bits and you've found one that should not be running KiCad.

The lesson here is that "long" is

a) not necessarily longer than int, and

b) not known until you pick a machine and a compiler. So you don't get 64 bitness if
thats what you need.

> so maybe on some platforms the code
> would not run as expected. But as it works on the major platforms then I simply do not
> insist on changing that.

int64_t is 64 bits, guaranteed.

Anyway - the patch fixes the problem even when
> m_VisibleElements stays as integer, so what do you think about the rest of the
> changes?

I have not looked at it, I can if nobody else want's to. The original patch came to me
via email.

> Regards, Orson

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :
Revision history for this message
Lorenzo Marcantonio (l-marcantonio) wrote :

On Thu, Feb 06, 2014 at 05:54:21PM -0000, Dick Hollenbeck wrote:
> I stopped reading the patch after I saw that. I don't like long where an "int" will
> suffice, or where an int and a long are the same thing. long really should never be used.
> Because if you need a certain bitness, then there are datatypes which ensure that.

Markup on calendar :D I fully agree with Dick :P

> int64_t comes to mind.

I confirm that int64_t and uint64_t fully fit the bill. I'm using them
for the extended layer supports. Doing bi-architecture printf and scanf
is another thing, sadly...

--
Lorenzo Marcantonio
Logos Srl

Revision history for this message
Maciej Suminski (orsonmmz) wrote :
Download full text (4.8 KiB)

On 02/06/2014 11:48 PM, Dick Hollenbeck wrote:
> On 02/06/2014 04:26 PM, Maciej Sumiński wrote:
>
>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
>> to leave it as it is now, particularly that it is saved as a 32-bit number in
>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
>> expect 32-bit number there
>
> Note, that if we were always bound to allow older versions of KiCad to load newer files,
> then it would be impossible to add new features which needed saving on disk.
>
> I value new features far more. We can all stop writing code and wasting time on a mailing
> list if we are not to write features requiring file format changes. It would essentially
> mean that we could only make improvements which do not require change.
>
> The pay is too low already. If you can find a way to make improvements without also
> making changes, then I am in the wrong universe.

Sure, I am aware that to improve you need to change, but not always the
file format. In this case, one cannot even benefit from saving the
visibility settings for netname layers, as they are turned on/off
together with copper layers. To take advantage of that, we need to add
some checkboxes first.
We could put in a document all the changes that we want to introduce to
the file format and then change it every e.g. 6 months according to the
paper. In this way, users will not be bothered by so frequent file
format changes. Especially that the majority seems to be using the
stable version, so they would not be able to open files done with the
newest product (and getting the freshest product is still beyond
capabilities of many users).

>> - let them have it and do not crash/do weird things on load
>> of PCBs holding 64 bits for elements visiblity).
>
>
> Not really an issue, if you parse an ASCII 64 bit number into a 32 bit binary integer, in
> old code, the lower 32 bits are likely preserved. So in this case you might have been OK.
> If not, folks can stick with their old software and old BOARDs.

Ok, I checked the code and it probably will not crash. parseHex() calls
strtol(), so depending on the platform (and size of long int) you will
get either the least significant 32 bits (good; that works for x86_64)
or LONG_MAX or LONG_MIN (i686) - to me it looks like a bug.

>> Saving netnames layers visibility is
>> not really crucial, it can be restored basing on visibility settings of appropriate
>> copper layers.
>
> I like the feature! And I like the 33rd feature which I want to save to disk, whatever it is.
>
> I would like to save it to disk, and have it be BOARD centric, not project centric.
>
> Eventually we'll hit some kind of wall, if not here, now, then sometime.
> Saving a bit map as an ASCII integer is only one possibility anyways. There are numerous
> alternatives for expansion:
>
> a) save a 64 bit integer in ASCII.
>
> b) use s-expressions to name each bit.
>
> c) An arbitrartily long ASCII binary number
> 0000_1011_1110_0011....
>
> This can extend far off the page if needed.
>
> Parsing that would be pretty easy, the underscore saves a human some counting. At some
> point you qu...

Read more...

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :

>
> Sincerely, my main concern is to fix the bug. I have already learned a
> lesson and reverted the commit that you have found improper. Is there
> anything else that I could do to have the branch merged?

Yep, move this out of the header file, into a *.cpp file please.
I think it is being instantiated in ALL compilation units in which it is visible, when
doing a DEBUG build. You only need it in ONE compilation unit for it to do its duty.

+#ifndef NDEBUG
+struct static_check {
+ static_check()
+ {
+ // Long (the type used for saving visibility settings) is only 32 bits guaranteed,
+ // be sure that we do not cross the limit
+ assert( END_PCB_VISIBLE_LIST <= 32 );
+ };
+};
+static static_check check;
+#endif

After than change, I approve the patch and somebody else can feel free to commit it as far
as I am concerned.

Sorry, I am too busy.

Dick

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :
Download full text (5.0 KiB)

On 02/07/2014 02:45 AM, Maciej Sumiński wrote:
> On 02/06/2014 11:48 PM, Dick Hollenbeck wrote:
>> On 02/06/2014 04:26 PM, Maciej Sumiński wrote:
>>
>>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
>>> to leave it as it is now, particularly that it is saved as a 32-bit number in
>>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
>>> expect 32-bit number there
>>
>> Note, that if we were always bound to allow older versions of KiCad to load newer files,
>> then it would be impossible to add new features which needed saving on disk.
>>
>> I value new features far more. We can all stop writing code and wasting time on a mailing
>> list if we are not to write features requiring file format changes. It would essentially
>> mean that we could only make improvements which do not require change.
>>
>> The pay is too low already. If you can find a way to make improvements without also
>> making changes, then I am in the wrong universe.
>
> Sure, I am aware that to improve you need to change, but not always the
> file format. In this case, one cannot even benefit from saving the
> visibility settings for netname layers, as they are turned on/off
> together with copper layers. To take advantage of that, we need to add
> some checkboxes first.
> We could put in a document all the changes that we want to introduce to
> the file format and then change it every e.g. 6 months according to the
> paper. In this way, users will not be bothered by so frequent file
> format changes. Especially that the majority seems to be using the
> stable version, so they would not be able to open files done with the
> newest product (and getting the freshest product is still beyond
> capabilities of many users).

Orson,

I am very impressed with your skill set, and I think that your time on the project is
extremely valuable to its future longevity. You have my profound respect and hopes for
the future. You are one of the bright spots in the project moving forward.

Your employer is paying for your time. I am paying for my time.

Therefore, my time is more important than anything, and I am insanely intolerant of it
being wasted or disrespected. But it also gives you a view of the lens from which I see
things.

You think our users are the most important thing. I think our developers are the most
important thing.

Note that I run a company. In that world, the users are the most important thing.

Here, in open source, you've got squat without developers.

>>
>> The lesson here is that "long" is
>>
>> a) not necessarily longer than int, and
>>
>> b) not known until you pick a machine and a compiler. So you don't get 64 bitness if
>> thats what you need.
>
> But it is not what I needed - as I said, it was only to assure 32 bits
> size.

int is 32 bits (at least) or the whole wx graphics realm would not work, nor would you
be able to support our internal units of nanometers. You are new to the project, but
anyone with 3 years here would know we needed 32 bit ints for wxInt to work for us.
wxPoint, etc. We did all the math, looked at every one of the 4 billion possibilities, etc....

Read more...

lp:~cern-kicad/kicad/bugfix_1275319 updated
4660. By Maciej Suminski

Moved PCB_VISIBLE size check to .cpp.

Revision history for this message
Maciej Suminski (orsonmmz) wrote :
Download full text (6.2 KiB)

On 02/07/2014 09:00 PM, Dick Hollenbeck wrote:
> On 02/07/2014 02:45 AM, Maciej Sumiński wrote:
>> On 02/06/2014 11:48 PM, Dick Hollenbeck wrote:
>>> On 02/06/2014 04:26 PM, Maciej Sumiński wrote:
>>>
>>>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
>>>> to leave it as it is now, particularly that it is saved as a 32-bit number in
>>>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
>>>> expect 32-bit number there
>>>
>>> Note, that if we were always bound to allow older versions of KiCad to load newer files,
>>> then it would be impossible to add new features which needed saving on disk.
>>>
>>> I value new features far more. We can all stop writing code and wasting time on a mailing
>>> list if we are not to write features requiring file format changes. It would essentially
>>> mean that we could only make improvements which do not require change.
>>>
>>> The pay is too low already. If you can find a way to make improvements without also
>>> making changes, then I am in the wrong universe.
>>
>> Sure, I am aware that to improve you need to change, but not always the
>> file format. In this case, one cannot even benefit from saving the
>> visibility settings for netname layers, as they are turned on/off
>> together with copper layers. To take advantage of that, we need to add
>> some checkboxes first.
>> We could put in a document all the changes that we want to introduce to
>> the file format and then change it every e.g. 6 months according to the
>> paper. In this way, users will not be bothered by so frequent file
>> format changes. Especially that the majority seems to be using the
>> stable version, so they would not be able to open files done with the
>> newest product (and getting the freshest product is still beyond
>> capabilities of many users).
>
>
> Orson,
>
> I am very impressed with your skill set, and I think that your time on the project is
> extremely valuable to its future longevity. You have my profound respect and hopes for
> the future. You are one of the bright spots in the project moving forward.
>
> Your employer is paying for your time. I am paying for my time.
>
> Therefore, my time is more important than anything, and I am insanely intolerant of it
> being wasted or disrespected. But it also gives you a view of the lens from which I see
> things.
>
> You think our users are the most important thing. I think our developers are the most
> important thing.
>
> Note that I run a company. In that world, the users are the most important thing.
>
> Here, in open source, you've got squat without developers.

Thank you for the words of appreciation. I can only imagine that running
a company is far from recreation and sometimes I really wonder how do
you manage to still find time for KiCad development.
I believe that in the open source world users and developers are equally
valuable, they just have different roles. And I know that development
costs much more time than filling out bug reports (if user decides to
give any feedback at all), but without having users you lose the
sensation of contribution and (sometimes) gratitude from users...

Read more...

Revision history for this message
Lorenzo Marcantonio (l-marcantonio) wrote :

On Fri, Feb 07, 2014 at 08:00:46PM -0000, Dick Hollenbeck wrote:
> >>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
> >>> to leave it as it is now, particularly that it is saved as a 32-bit number in
> >>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
> >>> expect 32-bit number there

My 2 eurocents:

1) I agree that it's granted that int is 32 or 64 bits on a PC. IIRC
the standard says that 'int is the most efficient integer for the
machine' (in fact for many RISC machines short or chars are hell on
earth... some DSPs even say that a char is 16 bits: have fun with
sizeof). So that predates even 1995, up to the first 32 bit C compilers with
dos extenders on the 386, circa 1990...

Another (more important IMHO) thing against long is that on the x86_64
ABI it's actually 128 bit which is overkill for most applications. And
needs 8 bytes and multiple precision arithmetic, AFAIK. So it's
not a noise issue for me since I am a typedef lover anyway :D

2) the 'correct' solution would be saving the visibility list as a list,
like the layers. Slightly more than a nuisance (I hope), we just need
someone implementing that.

> >> The lesson here is that "long" is
> >>
> >> a) not necessarily longer than int, and

ISO says so anyway, long is "at least" big as an int (and int is at
least 16 bits, but we can rely on it being at least 32). Unless you want
to port kicad to your favourite 16 bit micro, obviously.

> >> b) not known until you pick a machine and a compiler. So you don't get 64 bitness if
> >> thats what you need.

cmake/autoconf does that. stdint.h does it even better.

--
Lorenzo Marcantonio
Logos Srl

Revision history for this message
Dick Hollenbeck (dickelbeck) wrote :
Download full text (4.0 KiB)

On 02/07/2014 02:27 PM, Maciej Sumiński wrote:
> On 02/07/2014 09:00 PM, Dick Hollenbeck wrote:
>> On 02/07/2014 02:45 AM, Maciej Sumiński wrote:
>>> On 02/06/2014 11:48 PM, Dick Hollenbeck wrote:
>>>> On 02/06/2014 04:26 PM, Maciej Sumiński wrote:
>>>>
>>>>> It is not my intention to increase the number of bits in m_VisibleElements - I prefer
>>>>> to leave it as it is now, particularly that it is saved as a 32-bit number in
>>>>> .kicad_pcb files and I would like to preserve compatibility (so if older versions
>>>>> expect 32-bit number there
>>>>
>>>> Note, that if we were always bound to allow older versions of KiCad to load newer files,
>>>> then it would be impossible to add new features which needed saving on disk.
>>>>
>>>> I value new features far more. We can all stop writing code and wasting time on a mailing
>>>> list if we are not to write features requiring file format changes. It would essentially
>>>> mean that we could only make improvements which do not require change.
>>>>
>>>> The pay is too low already. If you can find a way to make improvements without also
>>>> making changes, then I am in the wrong universe.
>>>
>>> Sure, I am aware that to improve you need to change, but not always the
>>> file format. In this case, one cannot even benefit from saving the
>>> visibility settings for netname layers, as they are turned on/off
>>> together with copper layers. To take advantage of that, we need to add
>>> some checkboxes first.
>>> We could put in a document all the changes that we want to introduce to
>>> the file format and then change it every e.g. 6 months according to the
>>> paper. In this way, users will not be bothered by so frequent file
>>> format changes. Especially that the majority seems to be using the
>>> stable version, so they would not be able to open files done with the
>>> newest product (and getting the freshest product is still beyond
>>> capabilities of many users).
>>
>>
>> Orson,
>>
>> I am very impressed with your skill set, and I think that your time on the project is
>> extremely valuable to its future longevity. You have my profound respect and hopes for
>> the future. You are one of the bright spots in the project moving forward.
>>
>> Your employer is paying for your time. I am paying for my time.
>>
>> Therefore, my time is more important than anything, and I am insanely intolerant of it
>> being wasted or disrespected. But it also gives you a view of the lens from which I see
>> things.
>>
>> You think our users are the most important thing. I think our developers are the most
>> important thing.
>>
>> Note that I run a company. In that world, the users are the most important thing.
>>
>> Here, in open source, you've got squat without developers.
>
> Thank you for the words of appreciation. I can only imagine that running
> a company is far from recreation and sometimes I really wonder how do
> you manage to still find time for KiCad development.
> I believe that in the open source world users and developers are equally
> valuable, they just have different roles. And I know that development
> costs much more time than filling out bug reports (if user decides to
> give any fee...

Read more...

Revision history for this message
Maciej Suminski (orsonmmz) wrote :

That is quite interesting point of view, that is hard to argue with.
Getting back to the problem - is there anything that I should correct or could I have the patch merged? Now it is just the fix and some code cleaning. I bet that someone will again report the bug later.

Regards,
Orson

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/class_board_design_settings.h'
2--- include/class_board_design_settings.h 2013-08-28 16:14:39 +0000
3+++ include/class_board_design_settings.h 2014-02-07 20:17:03 +0000
4@@ -119,23 +119,25 @@
5 * Function IsElementVisible
6 * tests whether a given element category is visible. Keep this as an
7 * inline function.
8- * @param aPCB_VISIBLE is from the enum by the same name
9+ * @param aElementCategory is from the enum by the same name
10 * @return bool - true if the element is visible.
11 * @see enum PCB_VISIBLE
12 */
13- bool IsElementVisible( int aPCB_VISIBLE ) const
14+ bool IsElementVisible( int aElementCategory ) const
15 {
16- return bool( m_VisibleElements & (1 << aPCB_VISIBLE) );
17+ assert( aElementCategory >= 0 && aElementCategory < END_PCB_VISIBLE_LIST );
18+
19+ return ( m_VisibleElements & ( 1 << aElementCategory ) );
20 }
21
22 /**
23 * Function SetElementVisibility
24 * changes the visibility of an element category
25- * @param aPCB_VISIBLE is from the enum by the same name
26+ * @param aElementCategory is from the enum by the same name
27 * @param aNewState = The new visibility state of the element category
28 * @see enum PCB_VISIBLE
29 */
30- void SetElementVisibility( int aPCB_VISIBLE, bool aNewState );
31+ void SetElementVisibility( int aElementCategory, bool aNewState );
32
33 /**
34 * Function GetEnabledLayers
35
36=== modified file 'include/layers_id_colors_and_visibility.h'
37--- include/layers_id_colors_and_visibility.h 2013-11-06 17:17:42 +0000
38+++ include/layers_id_colors_and_visibility.h 2014-02-07 20:17:03 +0000
39@@ -239,8 +239,21 @@
40 PADS_HOLES_VISIBLE,
41 VIAS_HOLES_VISIBLE,
42
43- // Netname layers
44- LAYER_1_NETNAMES_VISIBLE, // Bottom layer
45+ WORKSHEET, ///< worksheet frame
46+ GP_OVERLAY, ///< general purpose overlay
47+
48+ END_PCB_VISIBLE_LIST // sentinel
49+};
50+
51+/**
52+ * Enum NETNAMES_VISIBLE
53+ * is a set of layers specific for displaying net names.
54+ * Their visiblity is not supposed to be saved in a board file,
55+ * they are only to be used by the GAL.
56+ */
57+enum NETNAMES_VISIBLE
58+{
59+ LAYER_1_NETNAMES_VISIBLE, // bottom layer
60 LAYER_2_NETNAMES_VISIBLE,
61 LAYER_3_NETNAMES_VISIBLE,
62 LAYER_4_NETNAMES_VISIBLE,
63@@ -255,25 +268,22 @@
64 LAYER_13_NETNAMES_VISIBLE,
65 LAYER_14_NETNAMES_VISIBLE,
66 LAYER_15_NETNAMES_VISIBLE,
67- LAYER_16_NETNAMES_VISIBLE, // Top layer
68+ LAYER_16_NETNAMES_VISIBLE, // top layer
69+
70 PAD_FR_NETNAMES_VISIBLE,
71 PAD_BK_NETNAMES_VISIBLE,
72 PADS_NETNAMES_VISIBLE,
73
74- WORKSHEET,
75- GP_OVERLAY, // General purpose overlay
76-
77- END_PCB_VISIBLE_LIST // sentinel
78+ END_NETNAMES_VISIBLE_LIST // sentinel
79 };
80
81-#define FIRST_NETNAME_LAYER ITEM_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE )
82-#define LAST_NETNAME_LAYER ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE )
83-
84 /// macro for obtaining layer number for specific item (eg. pad or text)
85-#define ITEM_GAL_LAYER(layer) (NB_LAYERS + layer)
86+#define ITEM_GAL_LAYER(layer) (NB_LAYERS + layer)
87+
88+#define NETNAMES_GAL_LAYER(layer) (NB_LAYERS + END_PCB_VISIBLE_LIST + layer )
89
90 /// number of *all* layers including PCB and item layers
91-#define TOTAL_LAYER_COUNT 128 //(NB_LAYERS + END_PCB_VISIBLE_LIST)
92+#define TOTAL_LAYER_COUNT (NB_LAYERS + END_PCB_VISIBLE_LIST + END_NETNAMES_VISIBLE_LIST)
93
94 /**
95 * Function IsValidLayer
96@@ -390,30 +400,28 @@
97 inline LAYER_NUM GetNetnameLayer( LAYER_NUM aLayer )
98 {
99 if( IsCopperLayer( aLayer ) )
100- {
101- // Compute the offset in description layers
102- return FIRST_NETNAME_LAYER + ( aLayer - FIRST_COPPER_LAYER );
103- }
104+ return NETNAMES_GAL_LAYER( aLayer );
105 else if( aLayer == ITEM_GAL_LAYER( PADS_VISIBLE ) )
106- return ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE );
107+ return NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE );
108 else if( aLayer == ITEM_GAL_LAYER( PAD_FR_VISIBLE ) )
109- return ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
110+ return NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
111 else if( aLayer == ITEM_GAL_LAYER( PAD_BK_VISIBLE ) )
112- return ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
113+ return NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
114
115 // Fallback
116 return COMMENT_N;
117 }
118
119 /**
120- * Function IsCopperLayer
121+ * Function IsNetnameLayer
122 * tests whether a layer is a netname layer
123 * @param aLayer = Layer to test
124 * @return true if aLayer is a valid netname layer
125 */
126 inline bool IsNetnameLayer( LAYER_NUM aLayer )
127 {
128- return aLayer >= FIRST_NETNAME_LAYER && aLayer <= LAST_NETNAME_LAYER;
129+ return aLayer >= NETNAMES_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ) &&
130+ aLayer < NETNAMES_GAL_LAYER( END_NETNAMES_VISIBLE_LIST );
131 }
132
133 #endif // _LAYERS_ID_AND_VISIBILITY_H_
134
135=== modified file 'pcbnew/basepcbframe.cpp'
136--- pcbnew/basepcbframe.cpp 2013-12-26 22:36:43 +0000
137+++ pcbnew/basepcbframe.cpp 2014-02-07 20:17:03 +0000
138@@ -75,7 +75,7 @@
139 const LAYER_NUM PCB_BASE_FRAME::GAL_LAYER_ORDER[] =
140 {
141 ITEM_GAL_LAYER( GP_OVERLAY ),
142- ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
143+ NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
144 DRAW_N, COMMENT_N, ECO1_N, ECO2_N, EDGE_N,
145 UNUSED_LAYER_29, UNUSED_LAYER_30, UNUSED_LAYER_31,
146 ITEM_GAL_LAYER( MOD_TEXT_FR_VISIBLE ),
147@@ -85,25 +85,25 @@
148 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ),
149 ITEM_GAL_LAYER( VIAS_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
150
151- ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ), SOLDERMASK_N_FRONT,
152- ITEM_GAL_LAYER( LAYER_16_NETNAMES_VISIBLE ), LAYER_N_FRONT,
153+ NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ), SOLDERMASK_N_FRONT,
154+ NETNAMES_GAL_LAYER( LAYER_16_NETNAMES_VISIBLE ), LAYER_N_FRONT,
155 SILKSCREEN_N_FRONT, SOLDERPASTE_N_FRONT, ADHESIVE_N_FRONT,
156- ITEM_GAL_LAYER( LAYER_15_NETNAMES_VISIBLE ), LAYER_N_15,
157- ITEM_GAL_LAYER( LAYER_14_NETNAMES_VISIBLE ), LAYER_N_14,
158- ITEM_GAL_LAYER( LAYER_13_NETNAMES_VISIBLE ), LAYER_N_13,
159- ITEM_GAL_LAYER( LAYER_12_NETNAMES_VISIBLE ), LAYER_N_12,
160- ITEM_GAL_LAYER( LAYER_11_NETNAMES_VISIBLE ), LAYER_N_11,
161- ITEM_GAL_LAYER( LAYER_10_NETNAMES_VISIBLE ), LAYER_N_10,
162- ITEM_GAL_LAYER( LAYER_9_NETNAMES_VISIBLE ), LAYER_N_9,
163- ITEM_GAL_LAYER( LAYER_8_NETNAMES_VISIBLE ), LAYER_N_8,
164- ITEM_GAL_LAYER( LAYER_7_NETNAMES_VISIBLE ), LAYER_N_7,
165- ITEM_GAL_LAYER( LAYER_6_NETNAMES_VISIBLE ), LAYER_N_6,
166- ITEM_GAL_LAYER( LAYER_5_NETNAMES_VISIBLE ), LAYER_N_5,
167- ITEM_GAL_LAYER( LAYER_4_NETNAMES_VISIBLE ), LAYER_N_4,
168- ITEM_GAL_LAYER( LAYER_3_NETNAMES_VISIBLE ), LAYER_N_3,
169- ITEM_GAL_LAYER( LAYER_2_NETNAMES_VISIBLE ), LAYER_N_2,
170- ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ), SOLDERMASK_N_BACK,
171- ITEM_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ), LAYER_N_BACK,
172+ NETNAMES_GAL_LAYER( LAYER_15_NETNAMES_VISIBLE ), LAYER_N_15,
173+ NETNAMES_GAL_LAYER( LAYER_14_NETNAMES_VISIBLE ), LAYER_N_14,
174+ NETNAMES_GAL_LAYER( LAYER_13_NETNAMES_VISIBLE ), LAYER_N_13,
175+ NETNAMES_GAL_LAYER( LAYER_12_NETNAMES_VISIBLE ), LAYER_N_12,
176+ NETNAMES_GAL_LAYER( LAYER_11_NETNAMES_VISIBLE ), LAYER_N_11,
177+ NETNAMES_GAL_LAYER( LAYER_10_NETNAMES_VISIBLE ), LAYER_N_10,
178+ NETNAMES_GAL_LAYER( LAYER_9_NETNAMES_VISIBLE ), LAYER_N_9,
179+ NETNAMES_GAL_LAYER( LAYER_8_NETNAMES_VISIBLE ), LAYER_N_8,
180+ NETNAMES_GAL_LAYER( LAYER_7_NETNAMES_VISIBLE ), LAYER_N_7,
181+ NETNAMES_GAL_LAYER( LAYER_6_NETNAMES_VISIBLE ), LAYER_N_6,
182+ NETNAMES_GAL_LAYER( LAYER_5_NETNAMES_VISIBLE ), LAYER_N_5,
183+ NETNAMES_GAL_LAYER( LAYER_4_NETNAMES_VISIBLE ), LAYER_N_4,
184+ NETNAMES_GAL_LAYER( LAYER_3_NETNAMES_VISIBLE ), LAYER_N_3,
185+ NETNAMES_GAL_LAYER( LAYER_2_NETNAMES_VISIBLE ), LAYER_N_2,
186+ NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ), SOLDERMASK_N_BACK,
187+ NETNAMES_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ), LAYER_N_BACK,
188
189 ADHESIVE_N_BACK, SOLDERPASTE_N_BACK, SILKSCREEN_N_BACK,
190 ITEM_GAL_LAYER( MOD_TEXT_BK_VISIBLE ),
191@@ -793,14 +793,14 @@
192 // Some more required layers settings
193 view->SetRequired( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( VIAS_VISIBLE ) );
194 view->SetRequired( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );
195- view->SetRequired( ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );
196+ view->SetRequired( NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );
197
198- view->SetRequired( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
199+ view->SetRequired( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
200 view->SetRequired( ADHESIVE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
201 view->SetRequired( SOLDERPASTE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
202 view->SetRequired( SOLDERMASK_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
203
204- view->SetRequired( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
205+ view->SetRequired( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
206 view->SetRequired( ADHESIVE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
207 view->SetRequired( SOLDERPASTE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
208 view->SetRequired( SOLDERMASK_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
209
210=== modified file 'pcbnew/class_board_design_settings.cpp'
211--- pcbnew/class_board_design_settings.cpp 2013-08-28 16:14:39 +0000
212+++ pcbnew/class_board_design_settings.cpp 2014-02-07 20:17:03 +0000
213@@ -243,3 +243,16 @@
214 // update m_CopperLayerCount to ensure its consistency with m_EnabledLayers
215 m_CopperLayerCount = LayerMaskCountSet( aMask & ALL_CU_LAYERS);
216 }
217+
218+
219+#ifndef NDEBUG
220+struct static_check {
221+ static_check()
222+ {
223+ // Int (the type used for saving visibility settings) is only 32 bits guaranteed,
224+ // be sure that we do not cross the limit
225+ assert( END_PCB_VISIBLE_LIST <= 32 );
226+ };
227+};
228+static static_check check;
229+#endif
230
231=== modified file 'pcbnew/class_pad.cpp'
232--- pcbnew/class_pad.cpp 2014-01-26 14:20:58 +0000
233+++ pcbnew/class_pad.cpp 2014-02-07 20:17:03 +0000
234@@ -841,7 +841,7 @@
235 {
236 // Multi layer pad
237 aLayers[aCount++] = ITEM_GAL_LAYER( PADS_VISIBLE );
238- aLayers[aCount++] = ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE );
239+ aLayers[aCount++] = NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE );
240 aLayers[aCount++] = SOLDERMASK_N_FRONT;
241 aLayers[aCount++] = SOLDERMASK_N_BACK;
242 aLayers[aCount++] = SOLDERPASTE_N_FRONT;
243@@ -850,14 +850,14 @@
244 else if( IsOnLayer( LAYER_N_FRONT ) )
245 {
246 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_FR_VISIBLE );
247- aLayers[aCount++] = ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
248+ aLayers[aCount++] = NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
249 aLayers[aCount++] = SOLDERMASK_N_FRONT;
250 aLayers[aCount++] = SOLDERPASTE_N_FRONT;
251 }
252 else if( IsOnLayer( LAYER_N_BACK ) )
253 {
254 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_BK_VISIBLE );
255- aLayers[aCount++] = ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
256+ aLayers[aCount++] = NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
257 aLayers[aCount++] = SOLDERMASK_N_BACK;
258 aLayers[aCount++] = SOLDERPASTE_N_BACK;
259 }
260
261=== modified file 'pcbnew/class_track.cpp'
262--- pcbnew/class_track.cpp 2013-12-29 11:01:54 +0000
263+++ pcbnew/class_track.cpp 2014-02-07 20:17:03 +0000
264@@ -763,9 +763,9 @@
265 unsigned int TRACK::ViewGetLOD( int aLayer ) const
266 {
267 // Netnames will be shown only if zoom is appropriate
268- if( aLayer == GetNetnameLayer( GetLayer() ) )
269+ if( IsNetnameLayer( aLayer ) )
270 {
271- return ( 20000000 / m_Width );
272+ return ( 20000000 / ( m_Width + 1 ) );
273 }
274
275 // Other layers are shown without any conditions
276
277=== modified file 'pcbnew/pcb_painter.cpp'
278--- pcbnew/pcb_painter.cpp 2014-01-26 14:20:58 +0000
279+++ pcbnew/pcb_painter.cpp 2014-02-07 20:17:03 +0000
280@@ -68,14 +68,14 @@
281 }
282
283 // Default colors for specific layers
284- m_layerColors[ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE )] = COLOR4D( 0.5, 0.4, 0.0, 1.0 );
285- m_layerColors[ITEM_GAL_LAYER( PADS_HOLES_VISIBLE )] = COLOR4D( 0.0, 0.5, 0.5, 1.0 );
286- m_layerColors[ITEM_GAL_LAYER( VIAS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
287- m_layerColors[ITEM_GAL_LAYER( PADS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
288- m_layerColors[ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
289- m_layerColors[ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
290- m_layerColors[ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
291- m_layerColors[ITEM_GAL_LAYER( WORKSHEET )] = COLOR4D( 0.5, 0.0, 0.0, 1.0 );
292+ m_layerColors[ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE )] = COLOR4D( 0.5, 0.4, 0.0, 1.0 );
293+ m_layerColors[ITEM_GAL_LAYER( PADS_HOLES_VISIBLE )] = COLOR4D( 0.0, 0.5, 0.5, 1.0 );
294+ m_layerColors[ITEM_GAL_LAYER( VIAS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
295+ m_layerColors[ITEM_GAL_LAYER( PADS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
296+ m_layerColors[NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
297+ m_layerColors[NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
298+ m_layerColors[NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
299+ m_layerColors[ITEM_GAL_LAYER( WORKSHEET )] = COLOR4D( 0.5, 0.0, 0.0, 1.0 );
300
301 // Netnames for copper layers
302 for( LAYER_NUM layer = FIRST_COPPER_LAYER; layer <= LAST_COPPER_LAYER; ++layer )
303@@ -261,13 +261,14 @@
304 VECTOR2D start( aTrack->GetStart() );
305 VECTOR2D end( aTrack->GetEnd() );
306 int width = aTrack->GetWidth();
307- int netNumber = aTrack->GetNet();
308 COLOR4D color;
309
310 if( m_pcbSettings->m_netNamesOnTracks && IsNetnameLayer( aLayer ) )
311 {
312+ int netCode = aTrack->GetNet();
313+
314 // If there is a net name - display it on the track
315- if( netNumber > 0 )
316+ if( netCode > 0 )
317 {
318 VECTOR2D line = ( end - start );
319 double length = line.EuclideanNorm();
320@@ -276,11 +277,11 @@
321 if( length < 10 * width )
322 return;
323
324- NETINFO_ITEM* net = ( (BOARD*) aTrack->GetParent() )->FindNet( netNumber );
325+ NETINFO_ITEM* net = ( (BOARD*) aTrack->GetParent() )->FindNet( netCode );
326 if( !net )
327 return;
328
329- std::wstring netName = std::wstring( net->GetShortNetname().wc_str() );
330+ wxString netName = net->GetShortNetname();
331 VECTOR2D textPosition = start + line / 2.0; // center of the track
332 double textOrientation = -atan( line.y / line.x );
333 double textSize = std::min( static_cast<double>( width ), length / netName.length() );
334@@ -304,7 +305,7 @@
335 m_gal->StrokeText( netName, textPosition, textOrientation );
336 }
337 }
338- else if( IsCopperLayer( aLayer ))
339+ else if( IsCopperLayer( aLayer ) )
340 {
341 // Draw a regular track
342 color = m_pcbSettings->GetColor( aTrack, aLayer );
343
344=== modified file 'pcbnew/pcb_painter.h'
345--- pcbnew/pcb_painter.h 2013-11-01 09:06:50 +0000
346+++ pcbnew/pcb_painter.h 2014-02-07 20:17:03 +0000
347@@ -114,6 +114,7 @@
348 ///> Colors for all layers (darkened)
349 COLOR4D m_layerColorsDark[TOTAL_LAYER_COUNT];
350
351+ ///> Flag determining if items on a given layer should be drawn as an outline or a full item
352 bool m_sketchModeSelect[TOTAL_LAYER_COUNT];
353
354 ///> Flag determining if pad numbers should be visible
355
356=== modified file 'pcbnew/pcbframe.cpp'
357--- pcbnew/pcbframe.cpp 2014-01-28 01:29:26 +0000
358+++ pcbnew/pcbframe.cpp 2014-02-07 20:17:03 +0000
359@@ -916,7 +916,7 @@
360 LAYER_NUM layers[] = {
361 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),
362 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
363- ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
364+ ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
365 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE )
366 };
367
368@@ -927,12 +927,12 @@
369 if( aLayer == FIRST_COPPER_LAYER )
370 {
371 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
372- rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
373+ rSettings->SetActiveLayer( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
374 }
375 else if( aLayer == LAST_COPPER_LAYER )
376 {
377 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
378- rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
379+ rSettings->SetActiveLayer( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
380 }
381 }
382
383@@ -956,7 +956,7 @@
384 LAYER_NUM layers[] = {
385 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),
386 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
387- ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
388+ ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
389 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE ), DRAW_N
390 };
391
392@@ -969,12 +969,12 @@
393 if( aLayer == FIRST_COPPER_LAYER )
394 {
395 view->SetTopLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
396- view->SetTopLayer( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
397+ view->SetTopLayer( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
398 }
399 else if( aLayer == LAST_COPPER_LAYER )
400 {
401 view->SetTopLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
402- view->SetTopLayer( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
403+ view->SetTopLayer( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
404 }
405 }
406
407@@ -1014,10 +1014,15 @@
408 m_Layers->SyncLayerVisibilities();
409
410 KIGFX::VIEW* view = GetGalCanvas()->GetView();
411+
412 // Load layer & elements visibility settings
413 for( LAYER_NUM i = 0; i < NB_LAYERS; ++i )
414 {
415 view->SetLayerVisible( i, m_Pcb->IsLayerVisible( i ) );
416+
417+ // Synchronize netname layers as well
418+ if( IsCopperLayer( i ) )
419+ view->SetLayerVisible( GetNetnameLayer( i ), m_Pcb->IsLayerVisible( i ) );
420 }
421
422 for( LAYER_NUM i = 0; i < END_PCB_VISIBLE_LIST; ++i )
423@@ -1026,12 +1031,10 @@
424 }
425
426 // Enable some layers that are GAL specific
427- for( LAYER_NUM i = FIRST_NETNAME_LAYER; i < LAST_NETNAME_LAYER; ++i )
428- {
429- view->SetLayerVisible( i, true );
430- }
431 view->SetLayerVisible( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), true );
432 view->SetLayerVisible( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), true );
433+ view->SetLayerVisible( ITEM_GAL_LAYER( WORKSHEET ), true );
434+ view->SetLayerVisible( ITEM_GAL_LAYER( GP_OVERLAY ), true );
435 }
436
437
438
439=== modified file 'pcbnew/router/router_preview_item.cpp'
440--- pcbnew/router/router_preview_item.cpp 2013-10-14 18:40:36 +0000
441+++ pcbnew/router/router_preview_item.cpp 2014-02-07 20:17:03 +0000
442@@ -36,6 +36,7 @@
443 {
444 m_Flags = 0;
445 m_parent = aParent;
446+ m_layer = DRAW_N;
447
448 if( aItem )
449 Update( aItem );
450
451=== modified file 'pcbnew/router/router_preview_item.h'
452--- pcbnew/router/router_preview_item.h 2013-10-14 14:13:35 +0000
453+++ pcbnew/router/router_preview_item.h 2014-02-07 20:17:03 +0000
454@@ -73,7 +73,7 @@
455
456 virtual void ViewGetLayers( int aLayers[], int& aCount ) const
457 {
458- aLayers[0] = GP_OVERLAY;
459+ aLayers[0] = m_layer;
460 aCount = 1;
461 }
462