Ref/value labels upside down for rotated parts

[Chris Pavlina]

As of [1], reference and value labels are now shown upside down for components rotated at 180 degrees (possibly other orientations too, not tested). I've attached a simple board where I placed a footprint with KiCad built from the commit just before [1]; [2] shows how the footprint looks in that build. I then opened the same file with KiCad built from [1]; [3] shows how it looks now.

[1]: https://github.com/KiCad/kicad-source-mirror/commit/f71d3fe7b65426cbde50c641d8309d32615da048
[2]: https://misc.c4757p.com/bug-demo-correct.png
[3]: https://misc.c4757p.com/bug-demo-incorrect.png

Version info from broken build:

Application: pcbnew
Version: (2017-11-11 revision f71d3fe7b)-HEAD, debug build
Libraries:
    wxWidgets 3.0.3
    libcurl/7.56.1 OpenSSL/1.1.0g zlib/1.2.11 libpsl/0.18.0 (+libidn2/2.0.4) libssh2/1.8.0 nghttp2/1.27.0
Platform: Linux 4.13.12-1-ARCH 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.65.1
    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

[Eldar Khayrullin (eldar)]

I think it is not a bug.
Ref rotating set relative footprint rotating 0.
Previously (before this commit) kicad did not show the right rotating of a reference.
Now kicad shows right rotating, pcb should be corrected (rotate references).

[jean-pierre charras (jp-charras)]

@Chris,
Have a look at *your* response in this bug report:
https://bugs.launchpad.net/kicad/+bug/613616

[Chris Pavlina (pavlina-chris)]

JP, not related. I don't have a problem with getting the _ability_ to rotate labels a certain way, the problem here is that they've suddenly flipped on an existing board. The example is a minimum case to demonstrate; I just opened a board from last year with a few hundred footprints and half of them suddenly had upside down labels.

[Wayne Stambaugh (stambaughw)]

The original position should be preserved but this may be an issue
because the change was not tied to a file revision because technically
the file didn't change.

On 11/19/2017 03:05 AM, Chris Pavlina wrote:
> JP, not related. I don't have a problem with getting the _ability_ to
> rotate labels a certain way, the problem here is that they've suddenly
> flipped on an existing board. The example is a minimum case to
> demonstrate; I just opened a board from last year with a few hundred
> footprints and half of them suddenly had upside down labels.
>

[Chris Pavlina (pavlina-chris)]

I'd argue that if the same thing on the PCB has a different representation now (in this case, a different number as the angle), that the format _did_ change, but I think that's a slightly pedantic argument from both sides so I won't get into it. I think the fact simply stands: if you opened a document you spent months working on in, say, Microsoft Word, and half the words were inexplicably upside down and you had to go through and fix them, you'd be absolutely furious. We shouldn't do that either, arguments about what constitutes a format change aside.

[Wayne Stambaugh (stambaughw)]

I agree with you but I believe that is not the case here. Internally,
the text object rotation was being clamped so technically you could have
the unclamped angles in the file and they would not get changed when the
file was parsed unless you changed them after the file was loaded. This
is why I think this is going to be a tricky issue to solve.

On 11/19/2017 01:38 PM, Chris Pavlina wrote:
> I'd argue that if the same thing on the PCB has a different
> representation now (in this case, a different number as the angle), that
> the format _did_ change, but I think that's a slightly pedantic argument
> from both sides so I won't get into it. I think the fact simply stands:
> if you opened a document you spent months working on in, say, Microsoft
> Word, and half the words were inexplicably upside down and you had to go
> through and fix them, you'd be absolutely furious. We shouldn't do that
> either, arguments about what constitutes a format change aside.
>

[Chris Pavlina (pavlina-chris)]

What about adding an "unlock rotation" flag to the file that indicates the angle is no longer clamped? New parts have that and are interpreted in the new way, old parts in the old way. Could probably convert silently without much issue. I think that since we're introducing format changes in v5 anyway it really shouldn't be a problem to add another?

[Chris Pavlina (pavlina-chris)]

This could be exposed in the UI or not, depending on whether we want to make that feature explicitly available to the user (I think that could be a nice thing to do, but it doesn't really matter either way)

[Wayne Stambaugh (stambaughw)]

I don't have a preference one way or the other. A file format bump may
be the easiest solution so it's a one shot conversion. Although if
memory serves, the original clamping was introduced at the request of a
user so maybe an option to continue clamping text rotation would keep
(almost?) everyone happy.

On 11/19/2017 04:32 PM, Chris Pavlina wrote:
> This could be exposed in the UI or not, depending on whether we want to
> make that feature explicitly available to the user (I think that could
> be a nice thing to do, but it doesn't really matter either way)
>

[Chris Pavlina (pavlina-chris)]

Yup, I really like the idea of being able to keep the current (well, previously current...) behavior and disable it as necessary. It _is_ useful.

[Wayne Stambaugh (stambaughw)]

I agree. I really have to be careful now when rotating text to make
sure all of my vertical and horizontal reference silkscreens are in the
same orientation. I would have a lot of upset production personnel if
they are all over the map. Any volunteers to code this? I'm at my
bandwidth limit at the moment.

On 11/19/2017 04:52 PM, Chris Pavlina wrote:
> Yup, I really like the idea of being able to keep the current (well,
> previously current...) behavior and disable it as necessary. It _is_
> useful.
>

[Chris Pavlina (pavlina-chris)]

I'll do it if I get the time - no promises, so if someone else wants to do it definitely go ahead. If nobody else jumps in I'll try pick this up sometime this week.

[Hildo Guillardi Júnior (hildogjr)]

Yes.
One idea was make some menu/wizard/configuration to set the all text in silks to one direction / angle.
https://bugs.launchpad.net/kicad/+bug/1731952

[Chris Pavlina (pavlina-chris)]

That sounds useful but I'm not sure how it addresses this bug.

[Chris Pavlina (pavlina-chris)]

Here's an attempt at fixing this. There are definitely different ways to go about this, so feel free to discuss, I'm willing to make changes.

I added an "unlocked" property to footprint texts. When set, you get the new behavior; when unset you get the old. I'm not forcing the orientation to be normalized to -90..90 in the dialog anymore - you can still set the orientation outside this range, it'll just render clamped to the range if locked. I added an "unlocked" token to the file format to indicate this property, so boards that keep everything locked have no change in format, the new token is only used if you use the new feature. This way, forward compatibility is maintained (as much as possible) as well as backward compatibility.

[Chris Pavlina (pavlina-chris)]

Sorry, here's an updated version fixing a code style violation.

[Wayne Stambaugh (stambaughw)]

This patch looks fine to me although I did see at least one coding
policy violation (missing argument spaces in class_text_mod.h). I think
it's a good idea to preserve the old behavior until the user explicitly
changes it. If no one else has any major objections, please push this
patch to the master branch.

On 11/25/2017 3:43 PM, Chris Pavlina wrote:
> Sorry, here's an updated version fixing a code style violation.
>
> ** Patch added: "0001-Add-unlock-property-to-footprint-texts.patch"
> https://bugs.launchpad.net/kicad/+bug/1733141/+attachment/5014595/+files/0001-Add-unlock-property-to-footprint-texts.patch
>

[Chris Pavlina (pavlina-chris)]

I actually noticed more violations after posting the patch but didn't post another so as to not make unnecessary noise, since I figured I'd be the one pushing it anyway.

Pushed, with corrections.