From 1d2752ee4788b4c65ce4054a6afcbea034e605b8 Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Mon, 8 Jan 2018 10:48:33 +0100 Subject: [PATCH] Symbol editor: rework on multi-unit symbols are pin edition synchronization between units (work in progress) User Interface: better tooltips and better messages. Code: better comments, and better member name. Fix a few minor issues. --- eeschema/block_libedit.cpp | 8 ++--- eeschema/libedit.cpp | 15 +++++++-- eeschema/libeditframe.cpp | 19 +++++++---- eeschema/libeditframe.h | 16 ++++++--- eeschema/pinedit.cpp | 69 ++++++++++++++++++++++---------------- eeschema/symbdraw.cpp | 22 ++++++++---- eeschema/tool_lib.cpp | 7 +++- 7 files changed, 101 insertions(+), 55 deletions(-) diff --git a/eeschema/block_libedit.cpp b/eeschema/block_libedit.cpp index c5275ac647..e0b1562567 100644 --- a/eeschema/block_libedit.cpp +++ b/eeschema/block_libedit.cpp @@ -124,7 +124,7 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* aDC ) if( GetCurPart() ) ItemCount = GetCurPart()->SelectItems( *block, m_unit, m_convert, - m_editPinsPerPartOrConvert ); + m_editPinsSeparately ); if( ItemCount ) { nextCmd = true; @@ -151,7 +151,7 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* aDC ) case BLOCK_CUT: if( GetCurPart() ) ItemCount = GetCurPart()->SelectItems( *block, m_unit, m_convert, - m_editPinsPerPartOrConvert ); + m_editPinsSeparately ); if( ItemCount ) { @@ -176,7 +176,7 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* aDC ) if( GetCurPart() ) ItemCount = GetCurPart()->SelectItems( *block, m_unit, m_convert, - m_editPinsPerPartOrConvert ); + m_editPinsSeparately ); if( ItemCount ) SaveCopyInUndoList( GetCurPart() ); @@ -200,7 +200,7 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* aDC ) if( GetCurPart() ) ItemCount = GetCurPart()->SelectItems( *block, m_unit, m_convert, - m_editPinsPerPartOrConvert ); + m_editPinsSeparately ); if( ItemCount ) SaveCopyInUndoList( GetCurPart() ); diff --git a/eeschema/libedit.cpp b/eeschema/libedit.cpp index 32f7763ba6..75e44d92a3 100644 --- a/eeschema/libedit.cpp +++ b/eeschema/libedit.cpp @@ -130,7 +130,7 @@ bool LIB_EDIT_FRAME::LoadComponentFromCurrentLib( const wxString& aAliasName, in if( aConvert > 0 ) m_convert = aConvert; - m_editPinsPerPartOrConvert = GetCurPart()->UnitsLocked() ? true : false; + m_editPinsSeparately = GetCurPart()->UnitsLocked() ? true : false; GetScreen()->ClearUndoRedoList(); Zoom_Automatique( false ); @@ -313,8 +313,6 @@ void LIB_EDIT_FRAME::OnCreateNewPart( wxCommandEvent& event ) // Initialize new_part.m_TextInside member: // if 0, pin text is outside the body (on the pin) // if > 0, pin text is inside the body - new_part.SetConversion( dlg.GetAlternateBodyStyle() ); - SetShowDeMorgan( dlg.GetAlternateBodyStyle() ); if( dlg.GetPinNameInside() ) { @@ -338,6 +336,12 @@ void LIB_EDIT_FRAME::OnCreateNewPart( wxCommandEvent& event ) m_libMgr->UpdatePart( &new_part, lib ); loadPart( name, lib, 1 ); + + new_part.SetConversion( dlg.GetAlternateBodyStyle() ); + // must be called after loadPart, that calls SetShowDeMorgan, but + // because the symbol is empty,it looks like it has no alternate body + SetShowDeMorgan( dlg.GetAlternateBodyStyle() ); + } @@ -496,6 +500,11 @@ void LIB_EDIT_FRAME::loadPart( const wxString& aAlias, const wxString& aLibrary, m_aliasName = aAlias; m_unit = ( aUnit <= part->GetUnitCount() ? aUnit : 1 ); + // Optimize default edit options for this symbol + // Usually if units are locked, graphic items are specific to each unit + // and if units are interchangeable, graphic items are common to units + m_drawSpecificUnit = part->UnitsLocked() ? true : false; + LoadOneLibraryPartAux( alias, aLibrary ); } diff --git a/eeschema/libeditframe.cpp b/eeschema/libeditframe.cpp index 508245b5d4..9c6a9a2145 100644 --- a/eeschema/libeditframe.cpp +++ b/eeschema/libeditframe.cpp @@ -210,7 +210,7 @@ LIB_EDIT_FRAME::LIB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) : m_drawSpecificConvert = true; m_drawSpecificUnit = false; m_hotkeysDescrList = g_Libedit_Hokeys_Descr; - m_editPinsPerPartOrConvert = false; + m_editPinsSeparately = false; m_repeatPinStep = DEFAULT_REPEAT_OFFSET_PIN; SetShowElectricalType( true ); @@ -619,7 +619,7 @@ void LIB_EDIT_FRAME::OnUpdatePinByPin( wxUpdateUIEvent& event ) event.Enable( part && ( part->GetUnitCount() > 1 || m_showDeMorgan ) ); - event.Check( m_editPinsPerPartOrConvert ); + event.Check( m_editPinsSeparately ); } @@ -805,7 +805,7 @@ void LIB_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event ) break; case ID_LIBEDIT_EDIT_PIN_BY_PIN: - m_editPinsPerPartOrConvert = m_mainToolBar->GetToolToggled( ID_LIBEDIT_EDIT_PIN_BY_PIN ); + m_editPinsSeparately = m_mainToolBar->GetToolToggled( ID_LIBEDIT_EDIT_PIN_BY_PIN ); break; case ID_POPUP_LIBEDIT_END_CREATE_ITEM: @@ -1134,11 +1134,16 @@ void LIB_EDIT_FRAME::OnEditComponentProperties( wxCommandEvent& event ) if( dlg.ShowModal() == wxID_CANCEL ) return; + // if m_UnitSelectionLocked has changed, set some edit options or defaults + // to the best value if( partLocked != GetCurPart()->UnitsLocked() ) { - // m_editPinsPerPartOrConvert is set to the better value, if m_UnitSelectionLocked - // has changed - m_editPinsPerPartOrConvert = GetCurPart()->UnitsLocked() ? true : false; + // m_editPinsSeparately is set to the better value + m_editPinsSeparately = GetCurPart()->UnitsLocked() ? true : false; + // also set default edit options to the better value + // Usually if units are locked, graphic items are specific to each unit + // and if units are interchangeable, graphic items are common to units + m_drawSpecificUnit = GetCurPart()->UnitsLocked() ? true : false; } m_libMgr->UpdatePart( GetCurPart(), GetCurLib(), oldName ); @@ -1490,7 +1495,7 @@ bool LIB_EDIT_FRAME::SynchronizePins() { LIB_PART* part = GetCurPart(); - return !m_editPinsPerPartOrConvert && ( part && + return !m_editPinsSeparately && ( part && ( part->HasConversion() || part->IsMulti() ) ); } diff --git a/eeschema/libeditframe.h b/eeschema/libeditframe.h index f59e1c9a03..bcbaf48598 100644 --- a/eeschema/libeditframe.h +++ b/eeschema/libeditframe.h @@ -79,12 +79,20 @@ class LIB_EDIT_FRAME : public SCH_BASE_FRAME /** * Set to true to not synchronize pins at the same position when editing - * components with multiple parts or multiple body styles. Setting this - * to false allows editing each pin per part or body style individually. + * symbols with multiple units or multiple body styles. + * Therefore deleting, moving pins are made for all pins at the same location + * When units are interchangeable, synchronizing edition of pins is usually + * the best way, because if units are interchangeable, it imply all similar + * pins are on the same location + * When units are non interchangeable, do not synchronize edition of pins, because + * each part is specific, and there are no similar pins between units + * + * Setting this to false allows editing each pin per part or body style + * regardless other pins at the same location. * This requires the user to open each part or body style to make changes - * to the pin at the same location. + * to the other pins at the same location. */ - bool m_editPinsPerPartOrConvert; + bool m_editPinsSeparately; /** * the option to show the pin electrical name in the component editor diff --git a/eeschema/pinedit.cpp b/eeschema/pinedit.cpp index 7513ae1822..28a38614e3 100644 --- a/eeschema/pinedit.cpp +++ b/eeschema/pinedit.cpp @@ -171,7 +171,7 @@ void LIB_EDIT_FRAME::OnEditPin( wxCommandEvent& event ) LastPinCommonUnit = dlg.GetAddToAllParts(); LastPinVisible = dlg.GetVisible(); - pin->EnableEditMode( true, m_editPinsPerPartOrConvert ); + pin->EnableEditMode( true, m_editPinsSeparately ); pin->SetName( dlg.GetPinName() ); pin->SetNameTextSize( GetLastPinNameSize() ); pin->SetNumber( dlg.GetPadName() ); @@ -199,7 +199,7 @@ void LIB_EDIT_FRAME::OnEditPin( wxCommandEvent& event ) m_canvas->Refresh(); } - pin->EnableEditMode( false, m_editPinsPerPartOrConvert ); + pin->EnableEditMode( false, m_editPinsSeparately ); // Restore pin flags, that can be changed by the dialog editor pin->ClearFlags(); @@ -265,8 +265,11 @@ void LIB_EDIT_FRAME::PlacePin() if( ask_for_pin && SynchronizePins() ) { m_canvas->SetIgnoreMouseEvents( true ); + wxString msg; + msg.Printf( _( "This position is already occupied by another pin, in unit %d.\n" + "Continue?" ), pin->GetUnit() ); - status = IsOK( this, _( "This position is already occupied by another pin. Continue?" ) ); + status = IsOK( this, msg ); m_canvas->MoveCursorToCrossHair(); m_canvas->SetIgnoreMouseEvents( false ); @@ -354,7 +357,6 @@ void LIB_EDIT_FRAME::StartMovePin( wxDC* DC ) startPos.x = OldPos.x; startPos.y = -OldPos.y; -// m_canvas->CrossHairOff( DC ); SetCrossHairPosition( startPos ); m_canvas->MoveCursorToCrossHair(); @@ -363,7 +365,6 @@ void LIB_EDIT_FRAME::StartMovePin( wxDC* DC ) cur_pin->GetMsgPanelInfo( items ); SetMsgPanel( items ); m_canvas->SetMouseCapture( DrawMovePin, AbortPinMove ); -// m_canvas->CrossHairOn( DC ); // Refresh the screen to avoid color artifacts when drawing // the pin in Edit mode and moving it from its start position @@ -491,56 +492,66 @@ void LIB_EDIT_FRAME::CreatePin( wxDC* DC ) void LIB_EDIT_FRAME::CreateImagePins( LIB_PIN* aPin, int aUnit, int aConvert, bool aDeMorgan ) { int ii; - LIB_PIN* NewPin; + LIB_PIN* newPin; + // if "synchronize pins edition" option is off, do not create any similar pin for other + // units and/or shapes: each unit is edited regardless other units or body if( !SynchronizePins() ) return; - // Create "convert" pin at the current position. + // When units are interchangeable, all units are expected to have similar pins + // at the same position + // to facilitate pin edition, create pins for all other units and all other shapes + // at the same position as aPin + + // For the current unit, provide a pin for the "convert" body style, + // at the current position, if the pin is not common to all body styles. if( aDeMorgan && ( aPin->GetConvert() != 0 ) ) { - NewPin = (LIB_PIN*) aPin->Clone(); + newPin = (LIB_PIN*) aPin->Clone(); if( aPin->GetConvert() > 1 ) - NewPin->SetConvert( 1 ); + newPin->SetConvert( 1 ); else - NewPin->SetConvert( 2 ); + newPin->SetConvert( 2 ); - aPin->GetParent()->AddDrawItem( NewPin ); + aPin->GetParent()->AddDrawItem( newPin ); } + // to facilitate pin edition, create similar pins for all other units at + // the same position as aPin + if( aPin->GetUnit() == 0 ) // Pin common to all units: no need to create similar pins. + return; + for( ii = 1; ii <= aPin->GetParent()->GetUnitCount(); ii++ ) { - if( ii == aUnit || aPin->GetUnit() != 0 ) - continue; // Pin common to all units. + if( ii == aUnit ) + continue; - NewPin = (LIB_PIN*) aPin->Clone(); + newPin = (LIB_PIN*) aPin->Clone(); // To avoid mistakes, gives this pin a new pin number because // it does no have the save pin number as the master pin - // Because we do not know the actual number, give it '??' - wxString unknownNum( wxT( "??" ) ); - NewPin->SetNumber( unknownNum ); + // Because we do not know the actual number, give it a temporary number + wxString unknownNum; + unknownNum.Printf( "%s-U%c", aPin->GetNumber(), wxChar( 'A' + ii - 1 ) ); + newPin->SetNumber( unknownNum ); if( aConvert != 0 ) - NewPin->SetConvert( 1 ); + newPin->SetConvert( 1 ); - NewPin->SetUnit( ii ); - aPin->GetParent()->AddDrawItem( NewPin ); + newPin->SetUnit( ii ); + aPin->GetParent()->AddDrawItem( newPin ); + // If this new pin is common to shapes, no need to create a similar pin for other body style if( !( aDeMorgan && ( aPin->GetConvert() != 0 ) ) ) continue; - NewPin = (LIB_PIN*) aPin->Clone(); - NewPin->SetConvert( 2 ); - // Gives this pin a new pin number - // Because we do not know the actual number, give it '??' - NewPin->SetNumber( unknownNum ); + // add a similar pin to the other body style + newPin = (LIB_PIN*) newPin->Clone(); + newPin->SetConvert( 2 ); - if( aPin->GetUnit() != 0 ) - NewPin->SetUnit( ii ); - - aPin->GetParent()->AddDrawItem( NewPin ); + aPin->GetParent()->AddDrawItem( newPin ); } } diff --git a/eeschema/symbdraw.cpp b/eeschema/symbdraw.cpp index 76da79b4eb..6bf8c2851f 100644 --- a/eeschema/symbdraw.cpp +++ b/eeschema/symbdraw.cpp @@ -25,7 +25,7 @@ /** * @file symbdraw.cpp - * @brief Create, move .. graphic shapes used to build and draw a component (lines, arcs ..) + * @brief Create, move .. graphic shapes used to build and draw a symbol (lines, arcs ..) */ #include @@ -60,7 +60,7 @@ void LIB_EDIT_FRAME::EditGraphicSymbol( wxDC* DC, LIB_ITEM* DrawItem ) if( DrawItem == NULL ) return; - LIB_PART* component = DrawItem->GetParent(); + LIB_PART* symbol = DrawItem->GetParent(); DIALOG_LIB_EDIT_DRAW_ITEM dialog( this, DrawItem->GetTypeName() ); @@ -69,9 +69,17 @@ void LIB_EDIT_FRAME::EditGraphicSymbol( wxDC* DC, LIB_ITEM* DrawItem ) wxString val = StringFromValue( g_UserUnit, DrawItem->GetWidth() ); dialog.SetWidth( val ); dialog.SetApplyToAllUnits( DrawItem->GetUnit() == 0 ); - dialog.EnableApplyToAllUnits( component && component->GetUnitCount() > 1 ); + dialog.EnableApplyToAllUnits( symbol && symbol->GetUnitCount() > 1 ); dialog.SetApplyToAllConversions( DrawItem->GetConvert() == 0 ); - dialog.EnableApplyToAllConversions( component && component->HasConversion() ); + bool enblConvOptStyle = symbol && symbol->HasConversion(); + // if a symbol contains no graphic items, symbol->HasConversion() returns false. + // but when creating a new symbol, with DeMorgan option set, the ApplyToAllConversions + // must be enabled even if symbol->HasConversion() returns false in order to be able + // to create graphic items shared by all body styles + if( GetShowDeMorgan() ) + enblConvOptStyle = true; + + dialog.EnableApplyToAllConversions( enblConvOptStyle ); dialog.SetFillStyle( DrawItem->GetFillMode() ); dialog.EnableFillStyle( DrawItem->IsFillable() ); @@ -151,8 +159,8 @@ LIB_ITEM* LIB_EDIT_FRAME::CreateGraphicItem( LIB_PART* LibEntry, wxDC* DC ) m_canvas->SetMouseCapture( SymbolDisplayDraw, AbortSymbolTraceOn ); wxPoint drawPos = GetCrossHairPosition( true ); - // no temp copy -> the current version of component will be used for Undo - // This is normal when adding new items to the current component + // no temp copy -> the current version of symbol will be used for Undo + // This is normal when adding new items to the current symbol ClearTempCopyComponent(); switch( GetToolId() ) @@ -358,7 +366,7 @@ void LIB_EDIT_FRAME::EndDrawGraphicItem( wxDC* DC ) else { // When creating a new item, there is still no change for the - // current component. So save it. + // current symbol. So save it. SaveCopyInUndoList( part ); } diff --git a/eeschema/tool_lib.cpp b/eeschema/tool_lib.cpp index 222c8a80f7..ef320fea59 100644 --- a/eeschema/tool_lib.cpp +++ b/eeschema/tool_lib.cpp @@ -199,7 +199,12 @@ void LIB_EDIT_FRAME::ReCreateHToolbar() m_mainToolBar->AddControl( m_aliasSelectBox ); m_mainToolBar->AddSeparator(); - msg = _( "Edit pins per symbol or body style (Use carefully!)" ); + msg = _( "Allows disabling pin edition coupling between units or body styles\n" + "When not disabled, adding, deleting and moving pins are synchronized\n" + "between units and body styles for pins at the same location\n" + "For instance, adding a pin to a unit also add a similar pin to other units at the same location\n" + "However, pins can have a different number or size because they are specific to a unit\n" + "Usually synchronization is enabled when units are interchangeable and disabled if not" ); m_mainToolBar->AddTool( ID_LIBEDIT_EDIT_PIN_BY_PIN, wxEmptyString, KiBitmap( pin2pin_xpm ), msg, wxITEM_CHECK ); m_mainToolBar->AddTool( ID_LIBEDIT_EDIT_PIN_BY_TABLE, wxEmptyString, KiBitmap( pin_table_xpm ),