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
=== modified file 'include/class_board_design_settings.h'
--- include/class_board_design_settings.h 2013-08-28 16:14:39 +0000
+++ include/class_board_design_settings.h 2014-02-07 20:17:03 +0000
@@ -119,23 +119,25 @@
119 * Function IsElementVisible119 * Function IsElementVisible
120 * tests whether a given element category is visible. Keep this as an120 * tests whether a given element category is visible. Keep this as an
121 * inline function.121 * inline function.
122 * @param aPCB_VISIBLE is from the enum by the same name122 * @param aElementCategory is from the enum by the same name
123 * @return bool - true if the element is visible.123 * @return bool - true if the element is visible.
124 * @see enum PCB_VISIBLE124 * @see enum PCB_VISIBLE
125 */125 */
126 bool IsElementVisible( int aPCB_VISIBLE ) const126 bool IsElementVisible( int aElementCategory ) const
127 {127 {
128 return bool( m_VisibleElements & (1 << aPCB_VISIBLE) );128 assert( aElementCategory >= 0 && aElementCategory < END_PCB_VISIBLE_LIST );
129
130 return ( m_VisibleElements & ( 1 << aElementCategory ) );
129 }131 }
130132
131 /**133 /**
132 * Function SetElementVisibility134 * Function SetElementVisibility
133 * changes the visibility of an element category135 * changes the visibility of an element category
134 * @param aPCB_VISIBLE is from the enum by the same name136 * @param aElementCategory is from the enum by the same name
135 * @param aNewState = The new visibility state of the element category137 * @param aNewState = The new visibility state of the element category
136 * @see enum PCB_VISIBLE138 * @see enum PCB_VISIBLE
137 */139 */
138 void SetElementVisibility( int aPCB_VISIBLE, bool aNewState );140 void SetElementVisibility( int aElementCategory, bool aNewState );
139141
140 /**142 /**
141 * Function GetEnabledLayers143 * Function GetEnabledLayers
142144
=== modified file 'include/layers_id_colors_and_visibility.h'
--- include/layers_id_colors_and_visibility.h 2013-11-06 17:17:42 +0000
+++ include/layers_id_colors_and_visibility.h 2014-02-07 20:17:03 +0000
@@ -239,8 +239,21 @@
239 PADS_HOLES_VISIBLE,239 PADS_HOLES_VISIBLE,
240 VIAS_HOLES_VISIBLE,240 VIAS_HOLES_VISIBLE,
241241
242 // Netname layers242 WORKSHEET, ///< worksheet frame
243 LAYER_1_NETNAMES_VISIBLE, // Bottom layer243 GP_OVERLAY, ///< general purpose overlay
244
245 END_PCB_VISIBLE_LIST // sentinel
246};
247
248/**
249 * Enum NETNAMES_VISIBLE
250 * is a set of layers specific for displaying net names.
251 * Their visiblity is not supposed to be saved in a board file,
252 * they are only to be used by the GAL.
253 */
254enum NETNAMES_VISIBLE
255{
256 LAYER_1_NETNAMES_VISIBLE, // bottom layer
244 LAYER_2_NETNAMES_VISIBLE,257 LAYER_2_NETNAMES_VISIBLE,
245 LAYER_3_NETNAMES_VISIBLE,258 LAYER_3_NETNAMES_VISIBLE,
246 LAYER_4_NETNAMES_VISIBLE,259 LAYER_4_NETNAMES_VISIBLE,
@@ -255,25 +268,22 @@
255 LAYER_13_NETNAMES_VISIBLE,268 LAYER_13_NETNAMES_VISIBLE,
256 LAYER_14_NETNAMES_VISIBLE,269 LAYER_14_NETNAMES_VISIBLE,
257 LAYER_15_NETNAMES_VISIBLE,270 LAYER_15_NETNAMES_VISIBLE,
258 LAYER_16_NETNAMES_VISIBLE, // Top layer271 LAYER_16_NETNAMES_VISIBLE, // top layer
272
259 PAD_FR_NETNAMES_VISIBLE,273 PAD_FR_NETNAMES_VISIBLE,
260 PAD_BK_NETNAMES_VISIBLE,274 PAD_BK_NETNAMES_VISIBLE,
261 PADS_NETNAMES_VISIBLE,275 PADS_NETNAMES_VISIBLE,
262276
263 WORKSHEET,277 END_NETNAMES_VISIBLE_LIST // sentinel
264 GP_OVERLAY, // General purpose overlay
265
266 END_PCB_VISIBLE_LIST // sentinel
267};278};
268279
269#define FIRST_NETNAME_LAYER ITEM_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE )
270#define LAST_NETNAME_LAYER ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE )
271
272/// macro for obtaining layer number for specific item (eg. pad or text)280/// macro for obtaining layer number for specific item (eg. pad or text)
273#define ITEM_GAL_LAYER(layer) (NB_LAYERS + layer)281#define ITEM_GAL_LAYER(layer) (NB_LAYERS + layer)
282
283#define NETNAMES_GAL_LAYER(layer) (NB_LAYERS + END_PCB_VISIBLE_LIST + layer )
274284
275/// number of *all* layers including PCB and item layers285/// number of *all* layers including PCB and item layers
276#define TOTAL_LAYER_COUNT 128 //(NB_LAYERS + END_PCB_VISIBLE_LIST)286#define TOTAL_LAYER_COUNT (NB_LAYERS + END_PCB_VISIBLE_LIST + END_NETNAMES_VISIBLE_LIST)
277287
278/**288/**
279 * Function IsValidLayer289 * Function IsValidLayer
@@ -390,30 +400,28 @@
390inline LAYER_NUM GetNetnameLayer( LAYER_NUM aLayer )400inline LAYER_NUM GetNetnameLayer( LAYER_NUM aLayer )
391{401{
392 if( IsCopperLayer( aLayer ) )402 if( IsCopperLayer( aLayer ) )
393 {403 return NETNAMES_GAL_LAYER( aLayer );
394 // Compute the offset in description layers
395 return FIRST_NETNAME_LAYER + ( aLayer - FIRST_COPPER_LAYER );
396 }
397 else if( aLayer == ITEM_GAL_LAYER( PADS_VISIBLE ) )404 else if( aLayer == ITEM_GAL_LAYER( PADS_VISIBLE ) )
398 return ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE );405 return NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE );
399 else if( aLayer == ITEM_GAL_LAYER( PAD_FR_VISIBLE ) )406 else if( aLayer == ITEM_GAL_LAYER( PAD_FR_VISIBLE ) )
400 return ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );407 return NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
401 else if( aLayer == ITEM_GAL_LAYER( PAD_BK_VISIBLE ) )408 else if( aLayer == ITEM_GAL_LAYER( PAD_BK_VISIBLE ) )
402 return ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );409 return NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
403410
404 // Fallback411 // Fallback
405 return COMMENT_N;412 return COMMENT_N;
406}413}
407414
408/**415/**
409 * Function IsCopperLayer416 * Function IsNetnameLayer
410 * tests whether a layer is a netname layer417 * tests whether a layer is a netname layer
411 * @param aLayer = Layer to test418 * @param aLayer = Layer to test
412 * @return true if aLayer is a valid netname layer419 * @return true if aLayer is a valid netname layer
413 */420 */
414inline bool IsNetnameLayer( LAYER_NUM aLayer )421inline bool IsNetnameLayer( LAYER_NUM aLayer )
415{422{
416 return aLayer >= FIRST_NETNAME_LAYER && aLayer <= LAST_NETNAME_LAYER;423 return aLayer >= NETNAMES_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ) &&
424 aLayer < NETNAMES_GAL_LAYER( END_NETNAMES_VISIBLE_LIST );
417}425}
418426
419#endif // _LAYERS_ID_AND_VISIBILITY_H_427#endif // _LAYERS_ID_AND_VISIBILITY_H_
420428
=== modified file 'pcbnew/basepcbframe.cpp'
--- pcbnew/basepcbframe.cpp 2013-12-26 22:36:43 +0000
+++ pcbnew/basepcbframe.cpp 2014-02-07 20:17:03 +0000
@@ -75,7 +75,7 @@
75const LAYER_NUM PCB_BASE_FRAME::GAL_LAYER_ORDER[] =75const LAYER_NUM PCB_BASE_FRAME::GAL_LAYER_ORDER[] =
76{76{
77 ITEM_GAL_LAYER( GP_OVERLAY ),77 ITEM_GAL_LAYER( GP_OVERLAY ),
78 ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),78 NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
79 DRAW_N, COMMENT_N, ECO1_N, ECO2_N, EDGE_N,79 DRAW_N, COMMENT_N, ECO1_N, ECO2_N, EDGE_N,
80 UNUSED_LAYER_29, UNUSED_LAYER_30, UNUSED_LAYER_31,80 UNUSED_LAYER_29, UNUSED_LAYER_30, UNUSED_LAYER_31,
81 ITEM_GAL_LAYER( MOD_TEXT_FR_VISIBLE ),81 ITEM_GAL_LAYER( MOD_TEXT_FR_VISIBLE ),
@@ -85,25 +85,25 @@
85 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ),85 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ),
86 ITEM_GAL_LAYER( VIAS_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),86 ITEM_GAL_LAYER( VIAS_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
8787
88 ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ), SOLDERMASK_N_FRONT,88 NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ), SOLDERMASK_N_FRONT,
89 ITEM_GAL_LAYER( LAYER_16_NETNAMES_VISIBLE ), LAYER_N_FRONT,89 NETNAMES_GAL_LAYER( LAYER_16_NETNAMES_VISIBLE ), LAYER_N_FRONT,
90 SILKSCREEN_N_FRONT, SOLDERPASTE_N_FRONT, ADHESIVE_N_FRONT,90 SILKSCREEN_N_FRONT, SOLDERPASTE_N_FRONT, ADHESIVE_N_FRONT,
91 ITEM_GAL_LAYER( LAYER_15_NETNAMES_VISIBLE ), LAYER_N_15,91 NETNAMES_GAL_LAYER( LAYER_15_NETNAMES_VISIBLE ), LAYER_N_15,
92 ITEM_GAL_LAYER( LAYER_14_NETNAMES_VISIBLE ), LAYER_N_14,92 NETNAMES_GAL_LAYER( LAYER_14_NETNAMES_VISIBLE ), LAYER_N_14,
93 ITEM_GAL_LAYER( LAYER_13_NETNAMES_VISIBLE ), LAYER_N_13,93 NETNAMES_GAL_LAYER( LAYER_13_NETNAMES_VISIBLE ), LAYER_N_13,
94 ITEM_GAL_LAYER( LAYER_12_NETNAMES_VISIBLE ), LAYER_N_12,94 NETNAMES_GAL_LAYER( LAYER_12_NETNAMES_VISIBLE ), LAYER_N_12,
95 ITEM_GAL_LAYER( LAYER_11_NETNAMES_VISIBLE ), LAYER_N_11,95 NETNAMES_GAL_LAYER( LAYER_11_NETNAMES_VISIBLE ), LAYER_N_11,
96 ITEM_GAL_LAYER( LAYER_10_NETNAMES_VISIBLE ), LAYER_N_10,96 NETNAMES_GAL_LAYER( LAYER_10_NETNAMES_VISIBLE ), LAYER_N_10,
97 ITEM_GAL_LAYER( LAYER_9_NETNAMES_VISIBLE ), LAYER_N_9,97 NETNAMES_GAL_LAYER( LAYER_9_NETNAMES_VISIBLE ), LAYER_N_9,
98 ITEM_GAL_LAYER( LAYER_8_NETNAMES_VISIBLE ), LAYER_N_8,98 NETNAMES_GAL_LAYER( LAYER_8_NETNAMES_VISIBLE ), LAYER_N_8,
99 ITEM_GAL_LAYER( LAYER_7_NETNAMES_VISIBLE ), LAYER_N_7,99 NETNAMES_GAL_LAYER( LAYER_7_NETNAMES_VISIBLE ), LAYER_N_7,
100 ITEM_GAL_LAYER( LAYER_6_NETNAMES_VISIBLE ), LAYER_N_6,100 NETNAMES_GAL_LAYER( LAYER_6_NETNAMES_VISIBLE ), LAYER_N_6,
101 ITEM_GAL_LAYER( LAYER_5_NETNAMES_VISIBLE ), LAYER_N_5,101 NETNAMES_GAL_LAYER( LAYER_5_NETNAMES_VISIBLE ), LAYER_N_5,
102 ITEM_GAL_LAYER( LAYER_4_NETNAMES_VISIBLE ), LAYER_N_4,102 NETNAMES_GAL_LAYER( LAYER_4_NETNAMES_VISIBLE ), LAYER_N_4,
103 ITEM_GAL_LAYER( LAYER_3_NETNAMES_VISIBLE ), LAYER_N_3,103 NETNAMES_GAL_LAYER( LAYER_3_NETNAMES_VISIBLE ), LAYER_N_3,
104 ITEM_GAL_LAYER( LAYER_2_NETNAMES_VISIBLE ), LAYER_N_2,104 NETNAMES_GAL_LAYER( LAYER_2_NETNAMES_VISIBLE ), LAYER_N_2,
105 ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ), SOLDERMASK_N_BACK,105 NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ), SOLDERMASK_N_BACK,
106 ITEM_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ), LAYER_N_BACK,106 NETNAMES_GAL_LAYER( LAYER_1_NETNAMES_VISIBLE ), LAYER_N_BACK,
107107
108 ADHESIVE_N_BACK, SOLDERPASTE_N_BACK, SILKSCREEN_N_BACK,108 ADHESIVE_N_BACK, SOLDERPASTE_N_BACK, SILKSCREEN_N_BACK,
109 ITEM_GAL_LAYER( MOD_TEXT_BK_VISIBLE ),109 ITEM_GAL_LAYER( MOD_TEXT_BK_VISIBLE ),
@@ -793,14 +793,14 @@
793 // Some more required layers settings793 // Some more required layers settings
794 view->SetRequired( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( VIAS_VISIBLE ) );794 view->SetRequired( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( VIAS_VISIBLE ) );
795 view->SetRequired( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );795 view->SetRequired( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );
796 view->SetRequired( ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );796 view->SetRequired( NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ) );
797797
798 view->SetRequired( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );798 view->SetRequired( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
799 view->SetRequired( ADHESIVE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );799 view->SetRequired( ADHESIVE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
800 view->SetRequired( SOLDERPASTE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );800 view->SetRequired( SOLDERPASTE_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
801 view->SetRequired( SOLDERMASK_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );801 view->SetRequired( SOLDERMASK_N_FRONT, ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
802802
803 view->SetRequired( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );803 view->SetRequired( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ), ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
804 view->SetRequired( ADHESIVE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );804 view->SetRequired( ADHESIVE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
805 view->SetRequired( SOLDERPASTE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );805 view->SetRequired( SOLDERPASTE_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
806 view->SetRequired( SOLDERMASK_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );806 view->SetRequired( SOLDERMASK_N_BACK, ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
807807
=== modified file 'pcbnew/class_board_design_settings.cpp'
--- pcbnew/class_board_design_settings.cpp 2013-08-28 16:14:39 +0000
+++ pcbnew/class_board_design_settings.cpp 2014-02-07 20:17:03 +0000
@@ -243,3 +243,16 @@
243 // update m_CopperLayerCount to ensure its consistency with m_EnabledLayers243 // update m_CopperLayerCount to ensure its consistency with m_EnabledLayers
244 m_CopperLayerCount = LayerMaskCountSet( aMask & ALL_CU_LAYERS);244 m_CopperLayerCount = LayerMaskCountSet( aMask & ALL_CU_LAYERS);
245}245}
246
247
248#ifndef NDEBUG
249struct static_check {
250 static_check()
251 {
252 // Int (the type used for saving visibility settings) is only 32 bits guaranteed,
253 // be sure that we do not cross the limit
254 assert( END_PCB_VISIBLE_LIST <= 32 );
255 };
256};
257static static_check check;
258#endif
246259
=== modified file 'pcbnew/class_pad.cpp'
--- pcbnew/class_pad.cpp 2014-01-26 14:20:58 +0000
+++ pcbnew/class_pad.cpp 2014-02-07 20:17:03 +0000
@@ -841,7 +841,7 @@
841 {841 {
842 // Multi layer pad842 // Multi layer pad
843 aLayers[aCount++] = ITEM_GAL_LAYER( PADS_VISIBLE );843 aLayers[aCount++] = ITEM_GAL_LAYER( PADS_VISIBLE );
844 aLayers[aCount++] = ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE );844 aLayers[aCount++] = NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE );
845 aLayers[aCount++] = SOLDERMASK_N_FRONT;845 aLayers[aCount++] = SOLDERMASK_N_FRONT;
846 aLayers[aCount++] = SOLDERMASK_N_BACK;846 aLayers[aCount++] = SOLDERMASK_N_BACK;
847 aLayers[aCount++] = SOLDERPASTE_N_FRONT;847 aLayers[aCount++] = SOLDERPASTE_N_FRONT;
@@ -850,14 +850,14 @@
850 else if( IsOnLayer( LAYER_N_FRONT ) )850 else if( IsOnLayer( LAYER_N_FRONT ) )
851 {851 {
852 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_FR_VISIBLE );852 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_FR_VISIBLE );
853 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );853 aLayers[aCount++] = NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE );
854 aLayers[aCount++] = SOLDERMASK_N_FRONT;854 aLayers[aCount++] = SOLDERMASK_N_FRONT;
855 aLayers[aCount++] = SOLDERPASTE_N_FRONT;855 aLayers[aCount++] = SOLDERPASTE_N_FRONT;
856 }856 }
857 else if( IsOnLayer( LAYER_N_BACK ) )857 else if( IsOnLayer( LAYER_N_BACK ) )
858 {858 {
859 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_BK_VISIBLE );859 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_BK_VISIBLE );
860 aLayers[aCount++] = ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );860 aLayers[aCount++] = NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE );
861 aLayers[aCount++] = SOLDERMASK_N_BACK;861 aLayers[aCount++] = SOLDERMASK_N_BACK;
862 aLayers[aCount++] = SOLDERPASTE_N_BACK;862 aLayers[aCount++] = SOLDERPASTE_N_BACK;
863 }863 }
864864
=== modified file 'pcbnew/class_track.cpp'
--- pcbnew/class_track.cpp 2013-12-29 11:01:54 +0000
+++ pcbnew/class_track.cpp 2014-02-07 20:17:03 +0000
@@ -763,9 +763,9 @@
763unsigned int TRACK::ViewGetLOD( int aLayer ) const763unsigned int TRACK::ViewGetLOD( int aLayer ) const
764{764{
765 // Netnames will be shown only if zoom is appropriate765 // Netnames will be shown only if zoom is appropriate
766 if( aLayer == GetNetnameLayer( GetLayer() ) )766 if( IsNetnameLayer( aLayer ) )
767 {767 {
768 return ( 20000000 / m_Width );768 return ( 20000000 / ( m_Width + 1 ) );
769 }769 }
770770
771 // Other layers are shown without any conditions771 // Other layers are shown without any conditions
772772
=== modified file 'pcbnew/pcb_painter.cpp'
--- pcbnew/pcb_painter.cpp 2014-01-26 14:20:58 +0000
+++ pcbnew/pcb_painter.cpp 2014-02-07 20:17:03 +0000
@@ -68,14 +68,14 @@
68 }68 }
6969
70 // Default colors for specific layers70 // Default colors for specific layers
71 m_layerColors[ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE )] = COLOR4D( 0.5, 0.4, 0.0, 1.0 );71 m_layerColors[ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE )] = COLOR4D( 0.5, 0.4, 0.0, 1.0 );
72 m_layerColors[ITEM_GAL_LAYER( PADS_HOLES_VISIBLE )] = COLOR4D( 0.0, 0.5, 0.5, 1.0 );72 m_layerColors[ITEM_GAL_LAYER( PADS_HOLES_VISIBLE )] = COLOR4D( 0.0, 0.5, 0.5, 1.0 );
73 m_layerColors[ITEM_GAL_LAYER( VIAS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );73 m_layerColors[ITEM_GAL_LAYER( VIAS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
74 m_layerColors[ITEM_GAL_LAYER( PADS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );74 m_layerColors[ITEM_GAL_LAYER( PADS_VISIBLE )] = COLOR4D( 0.7, 0.7, 0.7, 1.0 );
75 m_layerColors[ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );75 m_layerColors[NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
76 m_layerColors[ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );76 m_layerColors[NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
77 m_layerColors[ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );77 m_layerColors[NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE )] = COLOR4D( 0.8, 0.8, 0.8, 0.7 );
78 m_layerColors[ITEM_GAL_LAYER( WORKSHEET )] = COLOR4D( 0.5, 0.0, 0.0, 1.0 );78 m_layerColors[ITEM_GAL_LAYER( WORKSHEET )] = COLOR4D( 0.5, 0.0, 0.0, 1.0 );
7979
80 // Netnames for copper layers80 // Netnames for copper layers
81 for( LAYER_NUM layer = FIRST_COPPER_LAYER; layer <= LAST_COPPER_LAYER; ++layer )81 for( LAYER_NUM layer = FIRST_COPPER_LAYER; layer <= LAST_COPPER_LAYER; ++layer )
@@ -261,13 +261,14 @@
261 VECTOR2D start( aTrack->GetStart() );261 VECTOR2D start( aTrack->GetStart() );
262 VECTOR2D end( aTrack->GetEnd() );262 VECTOR2D end( aTrack->GetEnd() );
263 int width = aTrack->GetWidth();263 int width = aTrack->GetWidth();
264 int netNumber = aTrack->GetNet();
265 COLOR4D color;264 COLOR4D color;
266265
267 if( m_pcbSettings->m_netNamesOnTracks && IsNetnameLayer( aLayer ) )266 if( m_pcbSettings->m_netNamesOnTracks && IsNetnameLayer( aLayer ) )
268 {267 {
268 int netCode = aTrack->GetNet();
269
269 // If there is a net name - display it on the track270 // If there is a net name - display it on the track
270 if( netNumber > 0 )271 if( netCode > 0 )
271 {272 {
272 VECTOR2D line = ( end - start );273 VECTOR2D line = ( end - start );
273 double length = line.EuclideanNorm();274 double length = line.EuclideanNorm();
@@ -276,11 +277,11 @@
276 if( length < 10 * width )277 if( length < 10 * width )
277 return;278 return;
278279
279 NETINFO_ITEM* net = ( (BOARD*) aTrack->GetParent() )->FindNet( netNumber );280 NETINFO_ITEM* net = ( (BOARD*) aTrack->GetParent() )->FindNet( netCode );
280 if( !net )281 if( !net )
281 return;282 return;
282283
283 std::wstring netName = std::wstring( net->GetShortNetname().wc_str() );284 wxString netName = net->GetShortNetname();
284 VECTOR2D textPosition = start + line / 2.0; // center of the track285 VECTOR2D textPosition = start + line / 2.0; // center of the track
285 double textOrientation = -atan( line.y / line.x );286 double textOrientation = -atan( line.y / line.x );
286 double textSize = std::min( static_cast<double>( width ), length / netName.length() );287 double textSize = std::min( static_cast<double>( width ), length / netName.length() );
@@ -304,7 +305,7 @@
304 m_gal->StrokeText( netName, textPosition, textOrientation );305 m_gal->StrokeText( netName, textPosition, textOrientation );
305 }306 }
306 }307 }
307 else if( IsCopperLayer( aLayer ))308 else if( IsCopperLayer( aLayer ) )
308 {309 {
309 // Draw a regular track310 // Draw a regular track
310 color = m_pcbSettings->GetColor( aTrack, aLayer );311 color = m_pcbSettings->GetColor( aTrack, aLayer );
311312
=== modified file 'pcbnew/pcb_painter.h'
--- pcbnew/pcb_painter.h 2013-11-01 09:06:50 +0000
+++ pcbnew/pcb_painter.h 2014-02-07 20:17:03 +0000
@@ -114,6 +114,7 @@
114 ///> Colors for all layers (darkened)114 ///> Colors for all layers (darkened)
115 COLOR4D m_layerColorsDark[TOTAL_LAYER_COUNT];115 COLOR4D m_layerColorsDark[TOTAL_LAYER_COUNT];
116116
117 ///> Flag determining if items on a given layer should be drawn as an outline or a full item
117 bool m_sketchModeSelect[TOTAL_LAYER_COUNT];118 bool m_sketchModeSelect[TOTAL_LAYER_COUNT];
118119
119 ///> Flag determining if pad numbers should be visible120 ///> Flag determining if pad numbers should be visible
120121
=== modified file 'pcbnew/pcbframe.cpp'
--- pcbnew/pcbframe.cpp 2014-01-28 01:29:26 +0000
+++ pcbnew/pcbframe.cpp 2014-02-07 20:17:03 +0000
@@ -916,7 +916,7 @@
916 LAYER_NUM layers[] = {916 LAYER_NUM layers[] = {
917 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),917 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),
918 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),918 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
919 ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),919 ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
920 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE )920 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE )
921 };921 };
922922
@@ -927,12 +927,12 @@
927 if( aLayer == FIRST_COPPER_LAYER )927 if( aLayer == FIRST_COPPER_LAYER )
928 {928 {
929 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );929 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
930 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );930 rSettings->SetActiveLayer( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
931 }931 }
932 else if( aLayer == LAST_COPPER_LAYER )932 else if( aLayer == LAST_COPPER_LAYER )
933 {933 {
934 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );934 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
935 rSettings->SetActiveLayer( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );935 rSettings->SetActiveLayer( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
936 }936 }
937 }937 }
938938
@@ -956,7 +956,7 @@
956 LAYER_NUM layers[] = {956 LAYER_NUM layers[] = {
957 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),957 GetNetnameLayer( aLayer ), ITEM_GAL_LAYER( VIAS_VISIBLE ),
958 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),958 ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_VISIBLE ),
959 ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), ITEM_GAL_LAYER( PADS_NETNAMES_VISIBLE ),959 ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), NETNAMES_GAL_LAYER( PADS_NETNAMES_VISIBLE ),
960 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE ), DRAW_N960 ITEM_GAL_LAYER( GP_OVERLAY ), ITEM_GAL_LAYER( RATSNEST_VISIBLE ), DRAW_N
961 };961 };
962962
@@ -969,12 +969,12 @@
969 if( aLayer == FIRST_COPPER_LAYER )969 if( aLayer == FIRST_COPPER_LAYER )
970 {970 {
971 view->SetTopLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );971 view->SetTopLayer( ITEM_GAL_LAYER( PAD_BK_VISIBLE ) );
972 view->SetTopLayer( ITEM_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );972 view->SetTopLayer( NETNAMES_GAL_LAYER( PAD_BK_NETNAMES_VISIBLE ) );
973 }973 }
974 else if( aLayer == LAST_COPPER_LAYER )974 else if( aLayer == LAST_COPPER_LAYER )
975 {975 {
976 view->SetTopLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );976 view->SetTopLayer( ITEM_GAL_LAYER( PAD_FR_VISIBLE ) );
977 view->SetTopLayer( ITEM_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );977 view->SetTopLayer( NETNAMES_GAL_LAYER( PAD_FR_NETNAMES_VISIBLE ) );
978 }978 }
979 }979 }
980980
@@ -1014,10 +1014,15 @@
1014 m_Layers->SyncLayerVisibilities();1014 m_Layers->SyncLayerVisibilities();
10151015
1016 KIGFX::VIEW* view = GetGalCanvas()->GetView();1016 KIGFX::VIEW* view = GetGalCanvas()->GetView();
1017
1017 // Load layer & elements visibility settings1018 // Load layer & elements visibility settings
1018 for( LAYER_NUM i = 0; i < NB_LAYERS; ++i )1019 for( LAYER_NUM i = 0; i < NB_LAYERS; ++i )
1019 {1020 {
1020 view->SetLayerVisible( i, m_Pcb->IsLayerVisible( i ) );1021 view->SetLayerVisible( i, m_Pcb->IsLayerVisible( i ) );
1022
1023 // Synchronize netname layers as well
1024 if( IsCopperLayer( i ) )
1025 view->SetLayerVisible( GetNetnameLayer( i ), m_Pcb->IsLayerVisible( i ) );
1021 }1026 }
10221027
1023 for( LAYER_NUM i = 0; i < END_PCB_VISIBLE_LIST; ++i )1028 for( LAYER_NUM i = 0; i < END_PCB_VISIBLE_LIST; ++i )
@@ -1026,12 +1031,10 @@
1026 }1031 }
10271032
1028 // Enable some layers that are GAL specific1033 // Enable some layers that are GAL specific
1029 for( LAYER_NUM i = FIRST_NETNAME_LAYER; i < LAST_NETNAME_LAYER; ++i )
1030 {
1031 view->SetLayerVisible( i, true );
1032 }
1033 view->SetLayerVisible( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), true );1034 view->SetLayerVisible( ITEM_GAL_LAYER( PADS_HOLES_VISIBLE ), true );
1034 view->SetLayerVisible( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), true );1035 view->SetLayerVisible( ITEM_GAL_LAYER( VIAS_HOLES_VISIBLE ), true );
1036 view->SetLayerVisible( ITEM_GAL_LAYER( WORKSHEET ), true );
1037 view->SetLayerVisible( ITEM_GAL_LAYER( GP_OVERLAY ), true );
1035}1038}
10361039
10371040
10381041
=== modified file 'pcbnew/router/router_preview_item.cpp'
--- pcbnew/router/router_preview_item.cpp 2013-10-14 18:40:36 +0000
+++ pcbnew/router/router_preview_item.cpp 2014-02-07 20:17:03 +0000
@@ -36,6 +36,7 @@
36{36{
37 m_Flags = 0;37 m_Flags = 0;
38 m_parent = aParent;38 m_parent = aParent;
39 m_layer = DRAW_N;
3940
40 if( aItem )41 if( aItem )
41 Update( aItem );42 Update( aItem );
4243
=== modified file 'pcbnew/router/router_preview_item.h'
--- pcbnew/router/router_preview_item.h 2013-10-14 14:13:35 +0000
+++ pcbnew/router/router_preview_item.h 2014-02-07 20:17:03 +0000
@@ -73,7 +73,7 @@
7373
74 virtual void ViewGetLayers( int aLayers[], int& aCount ) const74 virtual void ViewGetLayers( int aLayers[], int& aCount ) const
75 {75 {
76 aLayers[0] = GP_OVERLAY;76 aLayers[0] = m_layer;
77 aCount = 1;77 aCount = 1;
78 }78 }
7979