From a6552f14f178f2b30198df326a1204c289e53472 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 5 Nov 2020 15:05:52 +0000 Subject: [PATCH] Improve SNR (code, status bar and user messages). Fixes https://gitlab.com/kicad/code/kicad/issues/6304 --- .../dialog_graphic_item_properties.cpp | 20 ++--- pcbnew/pcb_shape.cpp | 29 +------ pcbnew/tools/drawing_tool.cpp | 82 ++++++++++++------- pcbnew/tools/drawing_tool.h | 3 +- 4 files changed, 64 insertions(+), 70 deletions(-) diff --git a/pcbnew/dialogs/dialog_graphic_item_properties.cpp b/pcbnew/dialogs/dialog_graphic_item_properties.cpp index fe1d576530..57596a1d59 100644 --- a/pcbnew/dialogs/dialog_graphic_item_properties.cpp +++ b/pcbnew/dialogs/dialog_graphic_item_properties.cpp @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2019 Jean-Pierre Charras jp.charras at wanadoo.fr - * Copyright (C) 1992-2019 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 1992-2020 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 @@ -26,13 +26,11 @@ * Edit properties of Lines, Circles, Arcs and Polygons for PCBNew and ModEdit */ -#include #include #include #include #include #include -#include #include #include #include @@ -171,8 +169,11 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataToWindow() case S_CIRCLE: SetTitle( _( "Circle Properties" ) ); m_startPointLabel->SetLabel( _( "Center" ) ); + m_endPointLabel->SetLabel( _( "Radius" ) ); - m_endY.SetCoordType( ORIGIN_TRANSFORMS::NOT_A_COORD ); + m_endXLabel->Show( false ); + m_endX.SetCoordType( ORIGIN_TRANSFORMS::NOT_A_COORD ); + m_endY.Show( false ); break; @@ -247,12 +248,7 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataToWindow() m_thickness.SetValue( m_item->GetWidth() ); - if( m_LayerSelectionCtrl->SetLayerSelection( m_item->GetLayer() ) < 0 ) - { - wxMessageBox( _( "This item was on a non-existing or forbidden layer.\n" - "It has been moved to the first allowed layer. Please fix it." ) ); - m_LayerSelectionCtrl->SetSelection( 0 ); - } + m_LayerSelectionCtrl->SetLayerSelection( m_item->GetLayer() ); return DIALOG_GRAPHIC_ITEM_PROPERTIES_BASE::TransferDataToWindow(); } @@ -364,13 +360,13 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::Validate() case S_CIRCLE: // Check radius. if( m_startX.GetValue() == m_endX.GetValue() && m_startY.GetValue() == m_endY.GetValue() ) - error_msgs.Add( _( "The radius must be greater than zero." ) ); + error_msgs.Add( _( "The radius cannot be zero." ) ); break; case S_RECT: // Check for null rect. if( m_startX.GetValue() == m_endX.GetValue() && m_startY.GetValue() == m_endY.GetValue() ) - error_msgs.Add( _( "The rectangle can not be empty." ) ); + error_msgs.Add( _( "The rectangle cannot be empty." ) ); break; case S_POLYGON: diff --git a/pcbnew/pcb_shape.cpp b/pcbnew/pcb_shape.cpp index 34b57a2f39..76bbdbd3be 100644 --- a/pcbnew/pcb_shape.cpp +++ b/pcbnew/pcb_shape.cpp @@ -537,30 +537,6 @@ void PCB_SHAPE::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vectorx, iter->y ); + wxPoint pt( iter->x, iter->y ); if( module ) // Transform, if we belong to a module { diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 85fbd5ce83..a2cd8af55e 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -241,6 +241,7 @@ int DRAWING_TOOL::DrawLine( const TOOL_EVENT& aEvent ) SCOPED_DRAW_MODE scopedDrawMode( m_mode, MODE::LINE ); OPT startingPoint = boost::make_optional( false, VECTOR2D( 0, 0 ) ); + line->SetShape( S_SEGMENT ); line->SetFlags( IS_NEW ); if( aEvent.HasPosition() ) @@ -250,7 +251,7 @@ int DRAWING_TOOL::DrawLine( const TOOL_EVENT& aEvent ) m_frame->PushTool( tool ); Activate(); - while( drawSegment( tool, S_SEGMENT, &line, startingPoint ) ) + while( drawSegment( tool, &line, startingPoint ) ) { if( line ) { @@ -267,6 +268,7 @@ int DRAWING_TOOL::DrawLine( const TOOL_EVENT& aEvent ) } line = m_editModules ? new FP_SHAPE( module ) : new PCB_SHAPE; + line->SetShape( S_SEGMENT ); line->SetFlags( IS_NEW ); } @@ -285,6 +287,7 @@ int DRAWING_TOOL::DrawRectangle( const TOOL_EVENT& aEvent ) SCOPED_DRAW_MODE scopedDrawMode( m_mode, MODE::RECTANGLE ); OPT startingPoint = boost::make_optional( false, VECTOR2D( 0, 0 ) ); + rect->SetShape( S_RECT ); rect->SetFlags(IS_NEW ); if( aEvent.HasPosition() ) @@ -294,7 +297,7 @@ int DRAWING_TOOL::DrawRectangle( const TOOL_EVENT& aEvent ) m_frame->PushTool( tool ); Activate(); - while( drawSegment( tool, S_RECT, &rect, startingPoint ) ) + while( drawSegment( tool, &rect, startingPoint ) ) { if( rect ) { @@ -308,6 +311,7 @@ int DRAWING_TOOL::DrawRectangle( const TOOL_EVENT& aEvent ) } rect = m_editModules ? new FP_SHAPE( module ) : new PCB_SHAPE; + rect->SetShape( S_RECT ); rect->SetFlags(IS_NEW ); startingPoint = NULLOPT; } @@ -327,6 +331,7 @@ int DRAWING_TOOL::DrawCircle( const TOOL_EVENT& aEvent ) SCOPED_DRAW_MODE scopedDrawMode( m_mode, MODE::CIRCLE ); OPT startingPoint = boost::make_optional( false, VECTOR2D( 0, 0 ) ); + circle->SetShape( S_CIRCLE ); circle->SetFlags( IS_NEW ); if( aEvent.HasPosition() ) @@ -336,7 +341,7 @@ int DRAWING_TOOL::DrawCircle( const TOOL_EVENT& aEvent ) m_frame->PushTool( tool ); Activate(); - while( drawSegment( tool, S_CIRCLE, &circle, startingPoint ) ) + while( drawSegment( tool, &circle, startingPoint ) ) { if( circle ) { @@ -350,6 +355,7 @@ int DRAWING_TOOL::DrawCircle( const TOOL_EVENT& aEvent ) } circle = m_editModules ? new FP_SHAPE( module ) : new PCB_SHAPE; + circle->SetShape( S_CIRCLE ); circle->SetFlags( IS_NEW ); startingPoint = NULLOPT; } @@ -369,6 +375,7 @@ int DRAWING_TOOL::DrawArc( const TOOL_EVENT& aEvent ) SCOPED_DRAW_MODE scopedDrawMode( m_mode, MODE::ARC ); bool immediateMode = aEvent.HasPosition(); + arc->SetShape( S_ARC ); arc->SetFlags( IS_NEW ); std::string tool = aEvent.GetCommandStr().get(); @@ -389,6 +396,7 @@ int DRAWING_TOOL::DrawArc( const TOOL_EVENT& aEvent ) } arc = m_editModules ? new FP_SHAPE( module ) : new PCB_SHAPE; + arc->SetShape( S_ARC ); arc->SetFlags( IS_NEW ); immediateMode = false; } @@ -667,12 +675,15 @@ int DRAWING_TOOL::DrawDimension( const TOOL_EVENT& aEvent ) // Main loop: keep receiving events while( TOOL_EVENT* evt = Wait() ) { + if( step > SET_ORIGIN ) + frame()->SetMsgPanel( dimension ); + setCursor(); grid.SetSnap( !evt->Modifier( MD_SHIFT ) ); grid.SetUseGrid( m_frame->IsGridVisible() ); - VECTOR2I cursorPos = grid.BestSnapAnchor( - evt->IsPrime() ? evt->Position() : m_controls->GetMousePosition(), nullptr ); + VECTOR2I cursorPos = evt->IsPrime() ? evt->Position() : m_controls->GetMousePosition(); + cursorPos = grid.BestSnapAnchor( cursorPos, nullptr ); m_controls->ForceCursorPosition( true, cursorPos ); auto cleanup = [&]() @@ -801,6 +812,7 @@ int DRAWING_TOOL::DrawDimension( const TOOL_EVENT& aEvent ) dimension->SetEnd( (wxPoint) cursorPos ); preview.Add( dimension ); + frame()->SetMsgPanel( dimension ); m_controls->SetAutoPan( true ); m_controls->CaptureCursor( true ); @@ -1214,22 +1226,24 @@ int DRAWING_TOOL::SetAnchor( const TOOL_EVENT& aEvent ) static void updateSegmentFromGeometryMgr( const KIGFX::PREVIEW::TWO_POINT_GEOMETRY_MANAGER& aMgr, PCB_SHAPE* aGraphic ) { - auto vec = aMgr.GetOrigin(); - - aGraphic->SetStart( { vec.x, vec.y } ); - - vec = aMgr.GetEnd(); - aGraphic->SetEnd( { vec.x, vec.y } ); + if( !aMgr.IsReset() ) + { + aGraphic->SetStart( (wxPoint) aMgr.GetOrigin() ); + aGraphic->SetEnd( (wxPoint) aMgr.GetEnd() ); + } } -bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE** aGraphic, +bool DRAWING_TOOL::drawSegment( const std::string& aTool, PCB_SHAPE** aGraphic, OPT aStartingPoint ) { + int shape = (*aGraphic)->GetShape(); // Only three shapes are currently supported - assert( aShape == S_SEGMENT || aShape == S_CIRCLE || aShape == S_RECT ); - GRID_HELPER grid( m_toolMgr, m_frame->GetMagneticItemsSettings() ); - PCB_SHAPE*& graphic = *aGraphic; + assert( shape == S_SEGMENT || shape == S_CIRCLE || shape == S_RECT ); + + EDA_UNITS userUnits = m_frame->GetUserUnits(); + GRID_HELPER grid( m_toolMgr, m_frame->GetMagneticItemsSettings() ); + PCB_SHAPE*& graphic = *aGraphic; m_lineWidth = getSegmentWidth( m_frame->GetActiveLayer() ); @@ -1238,9 +1252,8 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* // drawing assistant overlay // TODO: workaround because PCB_SHAPE_TYPE_T is not visible from commons. - KIGFX::PREVIEW::GEOM_SHAPE geomShape( static_cast( aShape ) ); - KIGFX::PREVIEW::TWO_POINT_ASSISTANT twoPointAsst( twoPointManager, m_frame->GetUserUnits(), - geomShape ); + KIGFX::PREVIEW::GEOM_SHAPE geomShape( static_cast( shape ) ); + KIGFX::PREVIEW::TWO_POINT_ASSISTANT twoPointAsst( twoPointManager, userUnits, geomShape ); // Add a VIEW_GROUP that serves as a preview for the new item PCBNEW_SELECTION preview; @@ -1260,8 +1273,6 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* if( aStartingPoint ) m_toolMgr->RunAction( ACTIONS::cursorClick ); - frame()->SetMsgPanel( graphic ); - auto setCursor = [&]() { @@ -1275,7 +1286,9 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* while( TOOL_EVENT* evt = Wait() ) { setCursor(); - m_frame->SetMsgPanel( graphic ); + + if( started ) + m_frame->SetMsgPanel( graphic ); grid.SetSnap( !evt->Modifier( MD_SHIFT ) ); grid.SetUseGrid( m_frame->IsGridVisible() ); @@ -1368,7 +1381,7 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* m_lineWidth = getSegmentWidth( m_frame->GetActiveLayer() ); // Init the new item attributes - graphic->SetShape( (PCB_SHAPE_TYPE_T) aShape ); + graphic->SetShape( (PCB_SHAPE_TYPE_T) shape ); graphic->SetWidth( m_lineWidth ); graphic->SetLayer( m_frame->GetActiveLayer() ); grid.SetSkipPoint( cursorPos ); @@ -1380,7 +1393,7 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* m_frame->GetScreen()->m_LocalOrigin = cursorPos; preview.Add( graphic ); - frame()->SetMsgPanel( board() ); + frame()->SetMsgPanel( graphic ); m_controls->SetAutoPan( true ); m_controls->CaptureCursor( true ); @@ -1393,7 +1406,7 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* PCB_SHAPE* snapItem = dyn_cast( grid.GetSnapped() ); if( twoPointManager.GetOrigin() == twoPointManager.GetEnd() - || ( evt->IsDblClick( BUT_LEFT ) && aShape == S_SEGMENT ) || snapItem ) + || ( evt->IsDblClick( BUT_LEFT ) && shape == S_SEGMENT ) || snapItem ) // User has clicked twice in the same spot // or clicked on the end of an existing segment (closing a path) { @@ -1401,7 +1414,7 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* // If the user clicks on an existing snap point from a drawsegment // we finish the segment as they are likely closing a path - if( snapItem && ( aShape == S_RECT || graphic->GetLength() > 0.0 ) ) + if( snapItem && ( shape == S_RECT || graphic->GetLength() > 0.0 ) ) { commit.Add( graphic ); commit.Push( _( "Draw a line segment" ) ); @@ -1416,6 +1429,7 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* } preview.Clear(); + twoPointManager.Reset(); break; } @@ -1424,13 +1438,13 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* else if( evt->IsMotion() ) { // 45 degree lines - if( started && ( ( limit45 && aShape == S_SEGMENT ) - || ( evt->Modifier( MD_CTRL ) && aShape == S_RECT ) ) ) + if( started && ( ( limit45 && shape == S_SEGMENT ) + || ( evt->Modifier( MD_CTRL ) && shape == S_RECT ) ) ) { const VECTOR2I lineVector( cursorPos - VECTOR2I( twoPointManager.GetOrigin() ) ); // get a restricted 45/H/V line from the last fixed point to the cursor - auto newEnd = GetVectorSnapped45( lineVector, ( aShape == S_RECT ) ); + auto newEnd = GetVectorSnapped45( lineVector, ( shape == S_RECT ) ); m_controls->ForceCursorPosition( true, VECTOR2I( twoPointManager.GetEnd() ) ); twoPointManager.SetEnd( twoPointManager.GetOrigin() + (wxPoint) newEnd ); twoPointManager.SetAngleSnap( true ); @@ -1466,8 +1480,12 @@ bool DRAWING_TOOL::drawSegment( const std::string& aTool, int aShape, PCB_SHAPE* } else if( evt->IsAction( &ACTIONS::updateUnits ) ) { - twoPointAsst.SetUnits( frame()->GetUserUnits() ); - m_view->Update( &twoPointAsst ); + if( frame()->GetUserUnits() != userUnits ) + { + userUnits = frame()->GetUserUnits(); + twoPointAsst.SetUnits( userUnits ); + m_view->Update( &twoPointAsst ); + } evt->SetPassEvent(); } else @@ -1551,6 +1569,9 @@ bool DRAWING_TOOL::drawArc( const std::string& aTool, PCB_SHAPE** aGraphic, bool // Main loop: keep receiving events while( TOOL_EVENT* evt = Wait() ) { + if( firstPoint ) + m_frame->SetMsgPanel( graphic ); + setCursor(); PCB_LAYER_ID layer = m_frame->GetActiveLayer(); @@ -1617,6 +1638,7 @@ bool DRAWING_TOOL::drawArc( const std::string& aTool, PCB_SHAPE** aGraphic, bool graphic->SetWidth( m_lineWidth ); preview.Add( graphic ); + frame()->SetMsgPanel( graphic ); firstPoint = true; } diff --git a/pcbnew/tools/drawing_tool.h b/pcbnew/tools/drawing_tool.h index 26c88cc245..18f2dc7285 100644 --- a/pcbnew/tools/drawing_tool.h +++ b/pcbnew/tools/drawing_tool.h @@ -167,7 +167,6 @@ private: /** * Starts drawing a selected shape (i.e. PCB_SHAPE). - * @param aShape is the type of created shape (@see PCB_SHAPE_TYPE_T). * @param aGraphic is an object that is going to be used by the tool for drawing. Must be * already created. The tool deletes the object if it is not added to a BOARD. * @param aStartingPoint is a starting point for this new PCB_SHAPE. If it exists the new @@ -176,7 +175,7 @@ private: * @return False if the tool was cancelled before the origin was set or origin and end are * the same point. */ - bool drawSegment( const std::string& aTool, int aShape, PCB_SHAPE** aGraphic, + bool drawSegment( const std::string& aTool, PCB_SHAPE** aGraphic, OPT aStartingPoint ); /**