Eeschema, Pin helpers: fix wrong label orientation if the symbol is rotated

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18012

eeschema/CMakeLists.txt

@@ -412,6 +412,7 @@ set( EESCHEMA_SRCS
     symbol_tree_model_adapter.cpp
     symbol_tree_synchronizing_adapter.cpp
     symbol_viewer_frame.cpp
+    symb_transforms_utils.cpp
     toolbars_sch_editor.cpp
     toolbars_symbol_viewer.cpp
 

eeschema/sch_painter.cpp

@@ -61,6 +61,7 @@
 #include "sch_painter.h"
 #include "common.h"
 
+#include "symb_transforms_utils.h"
 
 namespace KIGFX
 {
@@ -2160,56 +2161,6 @@ void SCH_PAINTER::draw( const SCH_TABLE* aTable, int aLayer, bool aDimmed )
 }
 
 
-static void orientSymbol( LIB_SYMBOL* symbol, int orientation )
-{
-    struct ORIENT
-    {
-        int flag;
-        int n_rots;
-        int mirror_x;
-        int mirror_y;
-    }
-    orientations[] =
-    {
-        { SYM_ORIENT_0,                  0, 0, 0 },
-        { SYM_ORIENT_90,                 1, 0, 0 },
-        { SYM_ORIENT_180,                2, 0, 0 },
-        { SYM_ORIENT_270,                3, 0, 0 },
-        { SYM_MIRROR_X + SYM_ORIENT_0,   0, 1, 0 },
-        { SYM_MIRROR_X + SYM_ORIENT_90,  1, 1, 0 },
-        { SYM_MIRROR_Y,                  0, 0, 1 },
-        { SYM_MIRROR_X + SYM_ORIENT_270, 3, 1, 0 },
-        { SYM_MIRROR_Y + SYM_ORIENT_0,   0, 0, 1 },
-        { SYM_MIRROR_Y + SYM_ORIENT_90,  1, 0, 1 },
-        { SYM_MIRROR_Y + SYM_ORIENT_180, 2, 0, 1 },
-        { SYM_MIRROR_Y + SYM_ORIENT_270, 3, 0, 1 }
-    };
-
-    ORIENT o = orientations[ 0 ];
-
-    for( ORIENT& i : orientations )
-    {
-        if( i.flag == orientation )
-        {
-            o = i;
-            break;
-        }
-    }
-
-    for( SCH_ITEM& item : symbol->GetDrawItems() )
-    {
-        for( int i = 0; i < o.n_rots; i++ )
-            item.Rotate( VECTOR2I( 0, 0 ), true );
-
-        if( o.mirror_x )
-            item.MirrorVertically( 0 );
-
-        if( o.mirror_y )
-            item.MirrorHorizontally( 0 );
-    }
-}
-
-
 wxString SCH_PAINTER::expandLibItemTextVars( const wxString& aSourceText,
                                              const SCH_SYMBOL* aSymbolContext )
 {
@@ -2259,7 +2210,7 @@ void SCH_PAINTER::draw( const SCH_SYMBOL* aSymbol, int aLayer )
 
     tempSymbol.SetFlags( aSymbol->GetFlags() );
 
-    orientSymbol( &tempSymbol, aSymbol->GetOrientation() );
+    OrientAndMirrorSymbolItems( &tempSymbol, aSymbol->GetOrientation() );
 
     for( SCH_ITEM& tempItem : tempSymbol.GetDrawItems() )
     {

eeschema/sch_pin.cpp

@@ -1451,72 +1451,84 @@ void SCH_PIN::Move( const VECTOR2I& aOffset )
 }
 
 
+void SCH_PIN::MirrorHorizontallyPin( int aCenter )
+{
+    m_position.x -= aCenter;
+    m_position.x *= -1;
+    m_position.x += aCenter;
+
+    if( m_orientation == PIN_ORIENTATION::PIN_RIGHT )
+        m_orientation = PIN_ORIENTATION::PIN_LEFT;
+    else if( m_orientation == PIN_ORIENTATION::PIN_LEFT )
+        m_orientation = PIN_ORIENTATION::PIN_RIGHT;
+}
+
+
 void SCH_PIN::MirrorHorizontally( int aCenter )
 {
     if( dynamic_cast<LIB_SYMBOL*>( GetParentSymbol() ) )
-    {
-        m_position.x -= aCenter;
-        m_position.x *= -1;
-        m_position.x += aCenter;
+        MirrorHorizontallyPin( aCenter );
+}
 
-        if( m_orientation == PIN_ORIENTATION::PIN_RIGHT )
-            m_orientation = PIN_ORIENTATION::PIN_LEFT;
-        else if( m_orientation == PIN_ORIENTATION::PIN_LEFT )
-            m_orientation = PIN_ORIENTATION::PIN_RIGHT;
-    }
+
+void SCH_PIN::MirrorVerticallyPin( int aCenter )
+{
+    m_position.y -= aCenter;
+    m_position.y *= -1;
+    m_position.y += aCenter;
+
+    if( m_orientation == PIN_ORIENTATION::PIN_UP )
+        m_orientation = PIN_ORIENTATION::PIN_DOWN;
+    else if( m_orientation == PIN_ORIENTATION::PIN_DOWN )
+        m_orientation = PIN_ORIENTATION::PIN_UP;
 }
 
 
 void SCH_PIN::MirrorVertically( int aCenter )
 {
     if( dynamic_cast<LIB_SYMBOL*>( GetParentSymbol() ) )
-    {
-        m_position.y -= aCenter;
-        m_position.y *= -1;
-        m_position.y += aCenter;
-
-        if( m_orientation == PIN_ORIENTATION::PIN_UP )
-            m_orientation = PIN_ORIENTATION::PIN_DOWN;
-        else if( m_orientation == PIN_ORIENTATION::PIN_DOWN )
-            m_orientation = PIN_ORIENTATION::PIN_UP;
-    }
+        MirrorVerticallyPin( aCenter );
 }
 
 
-void SCH_PIN::Rotate( const VECTOR2I& aCenter, bool aRotateCCW )
+void SCH_PIN::RotatePin( const VECTOR2I& aCenter, bool aRotateCCW )
 {
-    if( dynamic_cast<LIB_SYMBOL*>( GetParentSymbol() ) )
+    if( aRotateCCW )
     {
-        if( aRotateCCW )
-        {
-            RotatePoint( m_position, aCenter, ANGLE_90 );
+        RotatePoint( m_position, aCenter, ANGLE_90 );
 
-            switch( GetOrientation() )
-            {
-            default:
-            case PIN_ORIENTATION::PIN_RIGHT: m_orientation = PIN_ORIENTATION::PIN_UP;    break;
-            case PIN_ORIENTATION::PIN_UP:    m_orientation = PIN_ORIENTATION::PIN_LEFT;  break;
-            case PIN_ORIENTATION::PIN_LEFT:  m_orientation = PIN_ORIENTATION::PIN_DOWN;  break;
-            case PIN_ORIENTATION::PIN_DOWN:  m_orientation = PIN_ORIENTATION::PIN_RIGHT; break;
-            }
-        }
-        else
+        switch( GetOrientation() )
         {
-            RotatePoint( m_position, aCenter, -ANGLE_90 );
+        default:
+        case PIN_ORIENTATION::PIN_RIGHT: m_orientation = PIN_ORIENTATION::PIN_UP;    break;
+        case PIN_ORIENTATION::PIN_UP:    m_orientation = PIN_ORIENTATION::PIN_LEFT;  break;
+        case PIN_ORIENTATION::PIN_LEFT:  m_orientation = PIN_ORIENTATION::PIN_DOWN;  break;
+        case PIN_ORIENTATION::PIN_DOWN:  m_orientation = PIN_ORIENTATION::PIN_RIGHT; break;
+        }
+    }
+    else
+    {
+        RotatePoint( m_position, aCenter, -ANGLE_90 );
 
-            switch( GetOrientation() )
-            {
-            default:
-            case PIN_ORIENTATION::PIN_RIGHT: m_orientation = PIN_ORIENTATION::PIN_DOWN;  break;
-            case PIN_ORIENTATION::PIN_UP:    m_orientation = PIN_ORIENTATION::PIN_RIGHT; break;
-            case PIN_ORIENTATION::PIN_LEFT:  m_orientation = PIN_ORIENTATION::PIN_UP;    break;
-            case PIN_ORIENTATION::PIN_DOWN:  m_orientation = PIN_ORIENTATION::PIN_LEFT;  break;
-            }
+        switch( GetOrientation() )
+        {
+        default:
+        case PIN_ORIENTATION::PIN_RIGHT: m_orientation = PIN_ORIENTATION::PIN_DOWN;  break;
+        case PIN_ORIENTATION::PIN_UP:    m_orientation = PIN_ORIENTATION::PIN_RIGHT; break;
+        case PIN_ORIENTATION::PIN_LEFT:  m_orientation = PIN_ORIENTATION::PIN_UP;    break;
+        case PIN_ORIENTATION::PIN_DOWN:  m_orientation = PIN_ORIENTATION::PIN_LEFT;  break;
         }
     }
 }
 
 
+void SCH_PIN::Rotate( const VECTOR2I& aCenter, bool aRotateCCW )
+{
+    if( dynamic_cast<LIB_SYMBOL*>( GetParentSymbol() ) )
+        RotatePin( aCenter, aRotateCCW );
+}
+
+
 void SCH_PIN::Plot( PLOTTER* aPlotter, bool aBackground, const SCH_PLOT_OPTS& aPlotOpts,
                     int aUnit, int aBodyStyle, const VECTOR2I& aOffset, bool aDimmed )
 {

eeschema/sch_pin.h

@@ -198,10 +198,20 @@ public:
 
     VECTOR2I GetPinRoot() const;
 
+    /**
+     * these transforms have effect only if the pin has a LIB_SYMBOL as parent
+     */
     void MirrorHorizontally( int aCenter ) override;
     void MirrorVertically( int aCenter ) override;
     void Rotate( const VECTOR2I& aCenter, bool aRotateCCW = true ) override;
 
+    /**
+     * these transforms have always effects
+     */
+    void MirrorHorizontallyPin( int aCenter );
+    void MirrorVerticallyPin( int aCenter );
+    void RotatePin( const VECTOR2I& aCenter, bool aRotateCCW = true );
+
     /**
      * Plot the pin name and number.
      * @param aTextInside - draw the names & numbers inside the symbol body (ie: in the opposite

eeschema/symb_transforms_utils.cpp

@@ -0,0 +1,107 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Author Jean-Pierre Charras, jp.charras at wanadoo.fr
+ * Copyright (C) 2024 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "lib_symbol.h"
+#include "sch_symbol.h"
+#include "sch_pin.h"
+
+struct ORIENT_MIRROR
+{
+    int flag;
+    int n_rots;
+    int mirror_x;
+    int mirror_y;
+};
+
+
+// symbols_orientations_list is the list of possible orientation+mirror values
+// like returned by SCH_SYMBOL::GetOrientation()
+// Some transforms are equivalent, like rotation 180 + mirror X = mirror Y
+
+static ORIENT_MIRROR symbols_orientations_list[] =
+{
+    { SYM_ORIENT_0,                  0, 0, 0 },
+    { SYM_ORIENT_90,                 1, 0, 0 },
+    { SYM_ORIENT_180,                2, 0, 0 },
+    { SYM_ORIENT_270,                3, 0, 0 },
+    { SYM_MIRROR_X + SYM_ORIENT_0,   0, 1, 0 },
+    { SYM_MIRROR_X + SYM_ORIENT_90,  1, 1, 0 },
+    { SYM_MIRROR_Y,                  0, 0, 1 },
+    { SYM_MIRROR_X + SYM_ORIENT_270, 3, 1, 0 },
+    { SYM_MIRROR_Y + SYM_ORIENT_0,   0, 0, 1 },
+    { SYM_MIRROR_Y + SYM_ORIENT_90,  1, 0, 1 },
+    { SYM_MIRROR_Y + SYM_ORIENT_180, 2, 0, 1 },
+    { SYM_MIRROR_Y + SYM_ORIENT_270, 3, 0, 1 }
+};
+
+
+void OrientAndMirrorSymbolItems( LIB_SYMBOL* aSymbol, int aOrientation )
+{
+    ORIENT_MIRROR o = symbols_orientations_list[ 0 ];
+
+    for( ORIENT_MIRROR& i : symbols_orientations_list )
+    {
+        if( i.flag == aOrientation )
+        {
+            o = i;
+            break;
+        }
+    }
+
+    for( SCH_ITEM& item : aSymbol->GetDrawItems() )
+    {
+        for( int i = 0; i < o.n_rots; i++ )
+            item.Rotate( VECTOR2I( 0, 0 ), true );
+
+        if( o.mirror_x )
+            item.MirrorVertically( 0 );
+
+        if( o.mirror_y )
+            item.MirrorHorizontally( 0 );
+    }
+}
+
+
+
+// Rotate and/or mirror a SCH_PIN according to aOrientMirror.
+// aOrientMirror is the orientation/mirror of the parent symbol.
+// The modified pin orientation is the actual pin orientation/mirror
+// when the parent symbol is drawn.
+void RotateAndMirrorPin( SCH_PIN& aPin, int aOrientMirror )
+{
+    ORIENT_MIRROR o = symbols_orientations_list[ 0 ];
+
+    for( ORIENT_MIRROR& i : symbols_orientations_list )
+    {
+        if( i.flag == aOrientMirror )
+        {
+            o = i;
+            break;
+        }
+    }
+
+    for( int i = 0; i < o.n_rots; i++ )
+        aPin.RotatePin( VECTOR2I( 0, 0 ), true );
+
+    if( o.mirror_x )
+        aPin.MirrorVerticallyPin( 0 );
+
+    if( o.mirror_y )
+        aPin.MirrorHorizontallyPin( 0 );
+}

eeschema/symb_transforms_utils.h

@@ -0,0 +1,41 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Author Jean-Pierre Charras, jp.charras at wanadoo.fr
+ * Copyright (C) 2024 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+class LIB_SYMBOL;
+
+/**
+ * Rotate and/or mirror graphic objects of LIB_SYMBOL aSymbol according to aOrientMirror.
+ * @param aLibSymbol is the LIB_SYMBOL to modify
+ * @param aOrientation is the orientation+mirror value like returned by SCH_SYMBOL::GetOrientation()
+ */
+void OrientAndMirrorSymbolItems( LIB_SYMBOL* aLibSymbol, int aOrientation );
+
+
+/**
+ * Rotate and/or mirror a SCH_PIN according to aOrientMirror.
+ * aOrientMirror is usually the orientation/mirror of the parent symbol.
+ * The modified pin orientation is the actual pin orientation/mirror
+ * when the parent symbol is drawn.
+ * @param aPin is the SCH_PIN to modify
+ * @param aOrientation is the orientation+mirror value like returned by SCH_SYMBOL::GetOrientation()
+ */
+void RotateAndMirrorPin( SCH_PIN& aPin, int aOrientMirror );

eeschema/tools/ee_selection_tool.cpp

@@ -64,6 +64,8 @@
 #include <view/view_controls.h>
 #include <wx/log.h>
 
+#include "symb_transforms_utils.h"
+
 
 SELECTION_CONDITION EE_CONDITIONS::SingleSymbol = []( const SELECTION& aSel )
 {
@@ -381,7 +383,18 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
             SCH_PIN* pin = dynamic_cast<SCH_PIN*>( aItem );
 
             if( pin )
-                return pin->GetOrientation();
+            {
+                const SCH_SYMBOL* parent = dynamic_cast<const SCH_SYMBOL*>( pin->GetParentSymbol() );
+
+                if( !parent )
+                    return pin->GetOrientation();
+                else
+                {
+                    SCH_PIN dummy( *pin );
+                    RotateAndMirrorPin( dummy, parent->GetOrientation() );
+                    return dummy.GetOrientation();
+                }
+            }
 
             SCH_SHEET_PIN* sheetPin = dynamic_cast<SCH_SHEET_PIN*>( aItem );