Change track width for multiple segments NOK

[Bernhard Stegmaier]

Use-Case:
Select multiple segments of a track.
Open track properties and change track width (in my case from 0.25mm to 0.5mm).

Observed:
Track width changes, but segment start/end points move somewhere else (far away).
See attached pictures.

Expected:
Tracks don't move, but just change their width.

Version:
<<<
Application: kicad
Version: (5.0.0-rc2-dev-100-g5a33f0960), release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.54.0 LibreSSL/2.0.20 zlib/1.2.11 nghttp2/1.24.0
Platform: Mac OS X (Darwin 17.4.0 x86_64), 64 bit, Little endian, wxMac
Build Info:
    wxWidgets: 3.0.4 (UTF-8,STL containers,compatible with 2.8)
    Boost: 1.66.0
    Curl: 7.58.0
    Compiler: Clang 9.0.0 with C++ ABI 1002

Build settings:
    USE_WX_GRAPHICS_CONTEXT=ON
    USE_WX_OVERLAY=ON
    KICAD_SCRIPTING=OFF
    KICAD_SCRIPTING_MODULES=OFF
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_SPICE=OFF
>>>

[Jon Evans (craftyjon)]

Cannot reproduce with:

Application: pcbnew
Version: (5.0.0-rc2-dev-250-gb40bf4c0e), debug build
Libraries:
    wxWidgets 3.0.3
    libcurl/7.55.1 OpenSSL/1.0.2g zlib/1.2.11 libidn2/2.0.2 libpsl/0.18.0 (+libidn2/2.0.2) librtmp/2.3
Platform: Linux 4.13.0-36-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.3 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.62.0
    Curl: 7.55.1
    Compiler: GCC 7.2.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_SPICE=ON

Bernhard, are you able to try with a newer nightly to see if the problem is already fixed?

[Bernhard Stegmaier (stegmaier)]

I also can reproduce with
<<<
Application: kicad
Version: (5.0.0-rc2-dev-252-gb5f1fdd98), release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.54.0 LibreSSL/2.0.20 zlib/1.2.11 nghttp2/1.24.0
Platform: Mac OS X (Darwin 17.4.0 x86_64), 64 bit, Little endian, wxMac
Build Info:
    wxWidgets: 3.0.4 (UTF-8,STL containers,compatible with 2.8)
    Boost: 1.66.0
    Curl: 7.59.0
    Compiler: Clang 9.0.0 with C++ ABI 1002

Build settings:
    USE_WX_GRAPHICS_CONTEXT=ON
    USE_WX_OVERLAY=ON
    KICAD_SCRIPTING=OFF
    KICAD_SCRIPTING_MODULES=OFF
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_SPICE=OFF
>>>

I'll send you the PCB, maybe it is some issue with it.

Or, do you have an hint where to look?

[Jon Evans (craftyjon)]

Yes it does happen in your board, interesting! I will take a look to see why yours is special.

[Bernhard Stegmaier (stegmaier)]

Thanks!
I am happy that it is not some weird macOS only thing… :)

> On 20. Mar 2018, at 14:43, Jon Evans <email address hidden> wrote:
>
> Yes it does happen in your board, interesting! I will take a look to
> see why yours is special.
>
> ** Changed in: kicad
> Status: Incomplete => Confirmed
>
> ** Changed in: kicad
> Importance: Undecided => High
>
> ** Changed in: kicad
> Milestone: None => 5.0.0-rc2
>
> ** Changed in: kicad
> Assignee: (unassigned) => Jon Evans (craftyjon)
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1756995
>
> Title:
> Change track width for multiple segments NOK
>
> Status in KiCad:
> Confirmed
>
> Bug description:
> Use-Case:
> Select multiple segments of a track.
> Open track properties and change track width (in my case from 0.25mm to 0.5mm).
>
> Observed:
> Track width changes, but segment start/end points move somewhere else (far away).
> See attached pictures.
>
> Expected:
> Tracks don't move, but just change their width.
>
>
> Version:
> <<<
> Application: kicad
> Version: (5.0.0-rc2-dev-100-g5a33f0960), release build
> Libraries:
> wxWidgets 3.0.4
> libcurl/7.54.0 LibreSSL/2.0.20 zlib/1.2.11 nghttp2/1.24.0
> Platform: Mac OS X (Darwin 17.4.0 x86_64), 64 bit, Little endian, wxMac
> Build Info:
> wxWidgets: 3.0.4 (UTF-8,STL containers,compatible with 2.8)
> Boost: 1.66.0
> Curl: 7.58.0
> Compiler: Clang 9.0.0 with C++ ABI 1002
>
> Build settings:
> USE_WX_GRAPHICS_CONTEXT=ON
> USE_WX_OVERLAY=ON
> KICAD_SCRIPTING=OFF
> KICAD_SCRIPTING_MODULES=OFF
> KICAD_SCRIPTING_WXPYTHON=OFF
> KICAD_SCRIPTING_ACTION_MENU=OFF
> BUILD_GITHUB_PLUGIN=ON
> KICAD_USE_OCE=ON
> KICAD_SPICE=OFF
>>>>
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/kicad/+bug/1756995/+subscriptions

[Jon Evans (craftyjon)]

Unfortunately, it does appear to be MacOS only as well; I tried on my Linux machine and can't reproduce.

My guess as to what is happening is that the dialog isn't working properly in MacOS and thinks that you have entered new values for the track start/end points when all you have changed is the width.

Unfortunately I don't have my Mac machine with me right now so I won't be able to hack on this today, if any of the other Mac-having devs want to try to fix it, maybe you can find it first.

[Bernhard Stegmaier (stegmaier)]

No problem, I'll check.
Thanks for the analysis.

[Bernhard Stegmaier (stegmaier)]

To be honest, I don't yet have an idea why this doesn't happen on other platforms.
It seems to come from introducing TEXT_CTRL_EVAL.

The TrackStartX has focus when dialog is open and is assigned "<...>" due to selecting more than one segment. When I change track width it loses the focus.
If the TEXT_CTRL_EVAL loses focus it does evaluate contents and sets the text box to it.
"<...>" does evaluate to 0, so "<...>" is replaced by "0".

If I change TEXT_CTRL_EVAL::evaluate() todo nothing in case of "<...>":
<<<
    if( GetValue() != "<...>")
        if( m_eval.process( GetValue().mb_str(), this ) )
            wxTextCtrl::SetValue( wxString::FromUTF8( m_eval.result() ) );
>>>
it works like it should and "<...>" is not overwritten by "0".

I am not really sure whether something like this is already the real fix...

[Jon Evans (craftyjon)]

Good find, that seems like a reasonable quick fix since the control doesn't know using some other mechanism that the value "<...>" means "ignore this control"

[Jeff Young (jeyjey)]

@Bernhard, that's definitely the bug. We check for <...> later, but if the NumericEvaluator has already turned it into 0 then we'll think the user set it to 0.

[Bernhard Stegmaier (stegmaier)]

Yes, but I don't know if it should be fixed like this (maybe using some constant for "<...>") or we might want to remove the evaluation from losing focus.
I didn't test, but I guess this might also fix it.

If you type in a complex formula and change to another control, should it be evaluated?
If you made a mistake, you can't fix it (because it is already evaluated).
On the other hand... if it doesn't get evaluated, you probably won't see that it was wrong.
And then you also have to re-type after re-opening the dialog.

[Jeff Young (jeyjey)]

@Bernhard, losing focus is correct. If you click back in the control, we'll replace the evaluated text with the source text.

There is a constant for <...>, but it's in a .cpp file. We should probably move it to common.h.

I've been in some of this code, so I'm happy to take it up if you want.

[Maciej Suminski (orsonmmz)]

Bernhard, thank you for the investigation, I am also surprised it works differently on OSX.
I think the problem here is that NumericEvaluator does not recognize <...> as invalid expression, so it is accepted and used in wxTextCtrl::SetValue() call. I will have a look at it, unless you desire to keep the bug fixing glory just for yourself.

[Bernhard Stegmaier (stegmaier)]

On a second thought I guess the "don't care" value is needed in any case.
I thought of adding it as a member, it might be useful also for other places.

I guess you all are pretty busy and I have some time right now, so I can prepare the patch and get some glory... :)

[Maciej Suminski (orsonmmz)]

Apologies, I could not resist;) Please try the attached patch.

[Bernhard Stegmaier (stegmaier)]

No worries, I am not so fast anymore and didn't start yet... :)

As usual, works perfect.
Thanks!

[KiCad Janitor (kicad-janitor)]

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

[Maciej Suminski (orsonmmz)]

Great, thank you for rapid testing!