From ee4bc8b1d086485ec7ae31fb43eb7420f5fd0bd6 Mon Sep 17 00:00:00 2001
From: Mikolaj Wielgus <wielgusmikolaj@gmail.com>
Date: Sat, 13 Mar 2021 01:07:58 +0100
Subject: [PATCH] Fix default line width for symbol editor graphic items, add
 validation

There was a mils to iu conversion missing for line width (the default
values are in mils).

Fixes https://gitlab.com/kicad/code/kicad/issues/7813
---
 .../dialogs/dialog_lib_edit_draw_item.cpp     | 45 +++++++++++++++----
 eeschema/dialogs/dialog_lib_edit_draw_item.h  |  3 +-
 .../tools/symbol_editor_drawing_tools.cpp     |  2 +-
 eeschema/tools/symbol_editor_edit_tool.cpp    | 25 +++--------
 4 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/eeschema/dialogs/dialog_lib_edit_draw_item.cpp b/eeschema/dialogs/dialog_lib_edit_draw_item.cpp
index 01f09eabb5..c229346209 100644
--- a/eeschema/dialogs/dialog_lib_edit_draw_item.cpp
+++ b/eeschema/dialogs/dialog_lib_edit_draw_item.cpp
@@ -28,6 +28,7 @@
 #include <lib_item.h>
 #include <dialog_lib_edit_draw_item.h>
 #include <symbol_edit_frame.h>
+#include <confirm.h>
 
 
 DIALOG_LIB_EDIT_DRAW_ITEM::DIALOG_LIB_EDIT_DRAW_ITEM( SYMBOL_EDIT_FRAME* aParent,
@@ -62,6 +63,9 @@ DIALOG_LIB_EDIT_DRAW_ITEM::DIALOG_LIB_EDIT_DRAW_ITEM( SYMBOL_EDIT_FRAME* aParent
 
 bool DIALOG_LIB_EDIT_DRAW_ITEM::TransferDataToWindow()
 {
+    if( !wxDialog::TransferDataToWindow() )
+        return false;
+
     LIB_PART* symbol = m_item->GetParent();
 
     m_lineWidth.SetValue( m_item->GetWidth() );
@@ -87,9 +91,40 @@ bool DIALOG_LIB_EDIT_DRAW_ITEM::TransferDataToWindow()
 }
 
 
-int DIALOG_LIB_EDIT_DRAW_ITEM::GetWidth()
+bool DIALOG_LIB_EDIT_DRAW_ITEM::TransferDataFromWindow()
 {
-    return m_lineWidth.GetValue();
+    if( !wxDialog::TransferDataFromWindow() )
+        return false;
+
+    // Min. 1 um and max. 1 m are more than enough.
+    if( !m_lineWidth.Validate( 0, 1000.0, EDA_UNITS::MILLIMETRES ) )
+        return false;
+
+    if( m_lineWidth.GetValue() == 0
+        && ( (FILL_TYPE) m_fillCtrl->GetSelection() ) != FILL_TYPE::FILLED_SHAPE )
+    {
+        DisplayError( this, _( "Line width may not be 0 for shapes other than filled with body "
+            "outline color." ) );
+        m_widthCtrl->SetFocus();
+        return false;
+    }
+
+    if( m_item->IsFillable() )
+        m_item->SetFillMode( (FILL_TYPE) std::max( m_fillCtrl->GetSelection(), 0 ) );
+
+    m_item->SetWidth( m_lineWidth.GetValue() );
+
+    if( GetApplyToAllConversions() )
+        m_item->SetConvert( 0 );
+    else
+        m_item->SetConvert( m_frame->GetConvert() );
+
+    if( GetApplyToAllUnits() )
+        m_item->SetUnit( 0 );
+    else
+        m_item->SetUnit( m_frame->GetUnit() );
+
+    return true;
 }
 
 
@@ -104,9 +139,3 @@ bool DIALOG_LIB_EDIT_DRAW_ITEM::GetApplyToAllUnits()
     return m_checkApplyToAllUnits->IsChecked();
 }
 
-
-int DIALOG_LIB_EDIT_DRAW_ITEM::GetFillStyle( void )
-{
-    return std::max( m_fillCtrl->GetSelection(), 0 );
-}
-
diff --git a/eeschema/dialogs/dialog_lib_edit_draw_item.h b/eeschema/dialogs/dialog_lib_edit_draw_item.h
index 9ec106ba90..6c7f86d725 100644
--- a/eeschema/dialogs/dialog_lib_edit_draw_item.h
+++ b/eeschema/dialogs/dialog_lib_edit_draw_item.h
@@ -43,11 +43,10 @@ public:
     DIALOG_LIB_EDIT_DRAW_ITEM( SYMBOL_EDIT_FRAME* parent, LIB_ITEM* aItem );
 
     bool TransferDataToWindow() override;
+    bool TransferDataFromWindow() override;
 
-    int GetWidth();
     bool GetApplyToAllConversions();
     bool GetApplyToAllUnits();
-    int GetFillStyle();
 
 private:
     SYMBOL_EDIT_FRAME* m_frame;
diff --git a/eeschema/tools/symbol_editor_drawing_tools.cpp b/eeschema/tools/symbol_editor_drawing_tools.cpp
index c0abf25745..eff1a259ee 100644
--- a/eeschema/tools/symbol_editor_drawing_tools.cpp
+++ b/eeschema/tools/symbol_editor_drawing_tools.cpp
@@ -354,7 +354,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent )
 
             wxCHECK( item, 0 );
 
-            item->SetWidth( settings->m_Defaults.line_width );
+            item->SetWidth( Mils2iu( settings->m_Defaults.line_width ) );
             item->SetFillMode( m_lastFillStyle );
             item->SetFlags( IS_NEW );
             item->BeginEdit( wxPoint( cursorPos.x, -cursorPos.y ) );
diff --git a/eeschema/tools/symbol_editor_edit_tool.cpp b/eeschema/tools/symbol_editor_edit_tool.cpp
index e1e57f4775..ccd88cc2a8 100644
--- a/eeschema/tools/symbol_editor_edit_tool.cpp
+++ b/eeschema/tools/symbol_editor_edit_tool.cpp
@@ -472,33 +472,18 @@ void SYMBOL_EDITOR_EDIT_TOOL::editGraphicProperties( LIB_ITEM* aItem )
     if( aItem == NULL )
         return;
 
-    DIALOG_LIB_EDIT_DRAW_ITEM dialog( m_frame, aItem );
+    DIALOG_LIB_EDIT_DRAW_ITEM dlg( m_frame, aItem );
 
-    if( dialog.ShowModal() != wxID_OK )
+    if( dlg.ShowModal() != wxID_OK )
         return;
 
-    if( aItem->IsFillable() )
-        aItem->SetFillMode( (FILL_TYPE) dialog.GetFillStyle() );
-
-    aItem->SetWidth( dialog.GetWidth() );
-
-    if( dialog.GetApplyToAllConversions() )
-        aItem->SetConvert( 0 );
-    else
-        aItem->SetConvert( m_frame->GetConvert() );
-
-    if( dialog.GetApplyToAllUnits() )
-        aItem->SetUnit( 0 );
-    else
-        aItem->SetUnit( m_frame->GetUnit() );
-
     updateItem( aItem, true );
     m_frame->GetCanvas()->Refresh();
-    m_frame->OnModify( );
+    m_frame->OnModify();
 
     SYMBOL_EDITOR_DRAWING_TOOLS* drawingTools = m_toolMgr->GetTool<SYMBOL_EDITOR_DRAWING_TOOLS>();
-    drawingTools->SetDrawSpecificConvert( !dialog.GetApplyToAllConversions() );
-    drawingTools->SetDrawSpecificUnit( !dialog.GetApplyToAllUnits() );
+    drawingTools->SetDrawSpecificConvert( !dlg.GetApplyToAllConversions() );
+    drawingTools->SetDrawSpecificUnit( !dlg.GetApplyToAllUnits() );
 
     MSG_PANEL_ITEMS items;
     aItem->GetMsgPanelInfo( m_frame, items );