From 9808889c9cdf934fe7fe8b1c50b84db66ea084d0 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 25 Aug 2025 16:34:47 +0100 Subject: [PATCH] Make 3D view/toolbar/contextmenu command lists more consistent. Fixes https://gitlab.com/kicad/code/kicad/-/issues/21347 --- 3d-viewer/3d_viewer/3d_menubar.cpp | 45 ++++++++---- .../3d_viewer/tools/eda_3d_controller.cpp | 69 ++++++++++--------- 3d-viewer/3d_viewer/tools/eda_3d_controller.h | 33 +++------ common/tool/action_menu.cpp | 24 +++---- 4 files changed, 84 insertions(+), 87 deletions(-) diff --git a/3d-viewer/3d_viewer/3d_menubar.cpp b/3d-viewer/3d_viewer/3d_menubar.cpp index 7c2a07ac13..1629c101d5 100644 --- a/3d-viewer/3d_viewer/3d_menubar.cpp +++ b/3d-viewer/3d_viewer/3d_menubar.cpp @@ -86,22 +86,39 @@ void EDA_3D_VIEWER_FRAME::doReCreateMenuBar() viewMenu->Add( gridSubmenu ); viewMenu->AppendSeparator(); - viewMenu->Add( EDA_3D_ACTIONS::rotateXCW ); - viewMenu->Add( EDA_3D_ACTIONS::rotateXCCW ); + viewMenu->Add( EDA_3D_ACTIONS::viewTop ); + viewMenu->Add( EDA_3D_ACTIONS::viewBottom ); + viewMenu->Add( EDA_3D_ACTIONS::viewRight ); + viewMenu->Add( EDA_3D_ACTIONS::viewLeft ); + viewMenu->Add( EDA_3D_ACTIONS::viewFront ); + viewMenu->Add( EDA_3D_ACTIONS::viewBack ); + + ACTION_MENU* rotateSubmenu = new ACTION_MENU( false, tool ); + rotateSubmenu->SetTitle( _( "Rotate Board" ) ); + rotateSubmenu->SetIcon( BITMAPS::rotate_cw ); + + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateXCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateXCCW ); + rotateSubmenu->AppendSeparator(); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateYCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateYCCW ); + rotateSubmenu->AppendSeparator(); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateZCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateZCCW ); + + ACTION_MENU* moveSubmenu = new ACTION_MENU( false, tool ); + moveSubmenu->SetTitle( _( "Move Board" ) ); + moveSubmenu->SetIcon( BITMAPS::move ); + + moveSubmenu->Add( EDA_3D_ACTIONS::moveLeft ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveRight ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveUp ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveDown ); viewMenu->AppendSeparator(); - viewMenu->Add( EDA_3D_ACTIONS::rotateYCW ); - viewMenu->Add( EDA_3D_ACTIONS::rotateYCCW ); - - viewMenu->AppendSeparator(); - viewMenu->Add( EDA_3D_ACTIONS::rotateZCW ); - viewMenu->Add( EDA_3D_ACTIONS::rotateZCCW ); - - viewMenu->AppendSeparator(); - viewMenu->Add( EDA_3D_ACTIONS::moveLeft ); - viewMenu->Add( EDA_3D_ACTIONS::moveRight ); - viewMenu->Add( EDA_3D_ACTIONS::moveUp ); - viewMenu->Add( EDA_3D_ACTIONS::moveDown ); + viewMenu->Add( rotateSubmenu ); + viewMenu->Add( EDA_3D_ACTIONS::flipView ); + viewMenu->Add( moveSubmenu ); viewMenu->AppendSeparator(); viewMenu->Add( EDA_3D_ACTIONS::showLayersManager, ACTION_MENU::CHECK ); diff --git a/3d-viewer/3d_viewer/tools/eda_3d_controller.cpp b/3d-viewer/3d_viewer/tools/eda_3d_controller.cpp index 4a6b0f19c0..c06828b12a 100644 --- a/3d-viewer/3d_viewer/tools/eda_3d_controller.cpp +++ b/3d-viewer/3d_viewer/tools/eda_3d_controller.cpp @@ -38,6 +38,30 @@ bool EDA_3D_CONTROLLER::Init() { + std::shared_ptr rotateSubmenu = std::make_shared( true, this ); + rotateSubmenu->SetTitle( _( "Rotate Board" ) ); + rotateSubmenu->SetIcon( BITMAPS::rotate_cw ); + m_menu->RegisterSubMenu( rotateSubmenu ); + + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateXCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateXCCW ); + rotateSubmenu->AppendSeparator(); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateYCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateYCCW ); + rotateSubmenu->AppendSeparator(); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateZCW ); + rotateSubmenu->Add( EDA_3D_ACTIONS::rotateZCCW ); + + std::shared_ptr moveSubmenu = std::make_shared( true, this ); + moveSubmenu->SetTitle( _( "Move Board" ) ); + moveSubmenu->SetIcon( BITMAPS::move ); + m_menu->RegisterSubMenu( moveSubmenu ); + + moveSubmenu->Add( EDA_3D_ACTIONS::moveLeft ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveRight ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveUp ); + moveSubmenu->Add( EDA_3D_ACTIONS::moveDown ); + CONDITIONAL_MENU& ctxMenu = m_menu->GetMenu(); ctxMenu.AddItem( ACTIONS::zoomInCenter, SELECTION_CONDITIONS::ShowAlways ); @@ -46,23 +70,15 @@ bool EDA_3D_CONTROLLER::Init() ctxMenu.AddSeparator(); ctxMenu.AddItem( EDA_3D_ACTIONS::viewTop, SELECTION_CONDITIONS::ShowAlways ); ctxMenu.AddItem( EDA_3D_ACTIONS::viewBottom, SELECTION_CONDITIONS::ShowAlways ); - - ctxMenu.AddSeparator(); ctxMenu.AddItem( EDA_3D_ACTIONS::viewRight, SELECTION_CONDITIONS::ShowAlways ); ctxMenu.AddItem( EDA_3D_ACTIONS::viewLeft, SELECTION_CONDITIONS::ShowAlways ); - - ctxMenu.AddSeparator(); ctxMenu.AddItem( EDA_3D_ACTIONS::viewFront, SELECTION_CONDITIONS::ShowAlways ); ctxMenu.AddItem( EDA_3D_ACTIONS::viewBack, SELECTION_CONDITIONS::ShowAlways ); ctxMenu.AddSeparator(); + ctxMenu.AddMenu( rotateSubmenu.get(), SELECTION_CONDITIONS::ShowAlways ); ctxMenu.AddItem( EDA_3D_ACTIONS::flipView, SELECTION_CONDITIONS::ShowAlways ); - - ctxMenu.AddSeparator(); - ctxMenu.AddItem( EDA_3D_ACTIONS::moveLeft, SELECTION_CONDITIONS::ShowAlways ); - ctxMenu.AddItem( EDA_3D_ACTIONS::moveRight, SELECTION_CONDITIONS::ShowAlways ); - ctxMenu.AddItem( EDA_3D_ACTIONS::moveUp, SELECTION_CONDITIONS::ShowAlways ); - ctxMenu.AddItem( EDA_3D_ACTIONS::moveDown, SELECTION_CONDITIONS::ShowAlways ); + ctxMenu.AddMenu( moveSubmenu.get(), SELECTION_CONDITIONS::ShowAlways ); return true; } @@ -116,12 +132,6 @@ int EDA_3D_CONTROLLER::UpdateMenu( const TOOL_EVENT& aEvent ) int EDA_3D_CONTROLLER::Main( const TOOL_EVENT& aEvent ) { - // Track right mouse button state and movement - VECTOR2D rightButtonDownPos; - - const int DRAG_THRESHOLD = 4; // pixels - bool rightButtonDragged = false; - // Main loop: keep receiving events while( TOOL_EVENT* evt = Wait() ) { @@ -247,8 +257,7 @@ int EDA_3D_CONTROLLER::ToggleVisibility( const TOOL_EVENT& aEvent ) auto flipLayer = [&]( int layer ) { - appearanceManager->OnLayerVisibilityChanged( layer, - !visibilityFlags.test( layer ) ); + appearanceManager->OnLayerVisibilityChanged( layer, !visibilityFlags.test( layer ) ); }; EDA_BASE_FRAME* frame = dynamic_cast( m_toolMgr->GetToolHolder() ); @@ -258,20 +267,13 @@ int EDA_3D_CONTROLLER::ToggleVisibility( const TOOL_EVENT& aEvent ) if( appearanceManager ) { - if( aEvent.IsAction( &EDA_3D_ACTIONS::showTHT ) ) - flipLayer( LAYER_3D_TH_MODELS ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showSMD ) ) - flipLayer( LAYER_3D_SMD_MODELS ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showVirtual ) ) - flipLayer( LAYER_3D_VIRTUAL_MODELS ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showNotInPosFile ) ) - flipLayer( LAYER_3D_MODELS_NOT_IN_POS ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showDNP ) ) - flipLayer( LAYER_3D_MODELS_MARKED_DNP ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showNavigator ) ) - flipLayer( LAYER_3D_NAVIGATOR ); - else if( aEvent.IsAction( &EDA_3D_ACTIONS::showBBoxes ) ) - flipLayer( LAYER_3D_BOUNDING_BOXES ); + if( aEvent.IsAction( &EDA_3D_ACTIONS::showTHT ) ) flipLayer( LAYER_3D_TH_MODELS ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showSMD ) ) flipLayer( LAYER_3D_SMD_MODELS ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showVirtual ) ) flipLayer( LAYER_3D_VIRTUAL_MODELS ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showNotInPosFile ) ) flipLayer( LAYER_3D_MODELS_NOT_IN_POS ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showDNP ) ) flipLayer( LAYER_3D_MODELS_MARKED_DNP ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showNavigator ) ) flipLayer( LAYER_3D_NAVIGATOR ); + else if( aEvent.IsAction( &EDA_3D_ACTIONS::showBBoxes ) ) flipLayer( LAYER_3D_BOUNDING_BOXES ); } return 0; @@ -325,8 +327,7 @@ int EDA_3D_CONTROLLER::doZoomInOut( bool aDirection, bool aCenterOnCursor ) { if( m_canvas ) { - m_canvas->SetView3D( aDirection ? VIEW3D_TYPE::VIEW3D_ZOOM_IN - : VIEW3D_TYPE::VIEW3D_ZOOM_OUT ); + m_canvas->SetView3D( aDirection ? VIEW3D_TYPE::VIEW3D_ZOOM_IN : VIEW3D_TYPE::VIEW3D_ZOOM_OUT ); m_canvas->DisplayStatus(); } diff --git a/3d-viewer/3d_viewer/tools/eda_3d_controller.h b/3d-viewer/3d_viewer/tools/eda_3d_controller.h index c1e620e429..2f7a92d227 100644 --- a/3d-viewer/3d_viewer/tools/eda_3d_controller.h +++ b/3d-viewer/3d_viewer/tools/eda_3d_controller.h @@ -22,8 +22,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef _3D_VIEWER_CONTROL_H -#define _3D_VIEWER_CONTROL_H +#pragma once #include @@ -38,12 +37,12 @@ class BOARD_ADAPTER; class EDA_3D_CONTROLLER : public TOOL_INTERACTIVE { public: - EDA_3D_CONTROLLER() - : TOOL_INTERACTIVE( "3DViewer.Control" ), - m_canvas( nullptr ), - m_boardAdapter( nullptr ), - m_camera( nullptr ), - m_rotationIncrement( 10.0 ) + EDA_3D_CONTROLLER() : + TOOL_INTERACTIVE( "3DViewer.Control" ), + m_canvas( nullptr ), + m_boardAdapter( nullptr ), + m_camera( nullptr ), + m_rotationIncrement( 10.0 ) { } ~EDA_3D_CONTROLLER() override { } @@ -63,20 +62,8 @@ public: * * @param aRotIncrement is the rotation increment in degrees */ - void SetRotationIncrement( double aRotIncrement ) - { - m_rotationIncrement = aRotIncrement; - } - - /** - * Get the increment used by the RotateView actions. - * - * @return the rotation increment in degrees - */ - double GetRotationIncrement() - { - return m_rotationIncrement; - } + void SetRotationIncrement( double aRotIncrement ) { m_rotationIncrement = aRotIncrement; } + double GetRotationIncrement() { return m_rotationIncrement; } // View controls int ZoomRedraw( const TOOL_EVENT& aEvent ); @@ -112,5 +99,3 @@ private: CAMERA* m_camera; double m_rotationIncrement; ///< Rotation increment for the rotate actions (degrees) }; - -#endif diff --git a/common/tool/action_menu.cpp b/common/tool/action_menu.cpp index 903b9c9e45..65eceb9963 100644 --- a/common/tool/action_menu.cpp +++ b/common/tool/action_menu.cpp @@ -63,8 +63,7 @@ ACTION_MENU::ACTION_MENU( bool isContextMenu, TOOL_INTERACTIVE* aTool ) : ACTION_MENU::~ACTION_MENU() { - Disconnect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), - nullptr, this ); + Disconnect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), nullptr, this ); Disconnect( wxEVT_IDLE, wxIdleEventHandler( ACTION_MENU::OnIdle ), nullptr, this ); // Set parent to NULL to prevent submenus from unregistering from a nonexistent object @@ -86,8 +85,7 @@ void ACTION_MENU::SetIcon( BITMAPS aIcon ) void ACTION_MENU::setupEvents() { - Connect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), nullptr, - this ); + Connect( wxEVT_COMMAND_MENU_SELECTED, wxMenuEventHandler( ACTION_MENU::OnMenuEvent ), nullptr, this ); Connect( wxEVT_IDLE, wxIdleEventHandler( ACTION_MENU::OnIdle ), nullptr, this ); } @@ -159,8 +157,8 @@ wxMenuItem* ACTION_MENU::Add( const wxString& aLabel, const wxString& aTooltip, { wxASSERT_MSG( FindItem( aId ) == nullptr, wxS( "Duplicate menu IDs!" ) ); - wxMenuItem* item = new wxMenuItem( this, aId, aLabel, aTooltip, - aIsCheckmarkEntry ? wxITEM_CHECK : wxITEM_NORMAL ); + wxMenuItem* item = new wxMenuItem( this, aId, aLabel, aTooltip, aIsCheckmarkEntry ? wxITEM_CHECK + : wxITEM_NORMAL ); if( !!aIcon ) KIUI::AddBitmapToMenuItem( item, KiBitmapBundle( aIcon ) ); @@ -178,8 +176,7 @@ wxMenuItem* ACTION_MENU::Add( const TOOL_ACTION& aAction, bool aIsCheckmarkEntry // Allow the label to be overridden at point of use wxString menuLabel = aOverrideLabel.IsEmpty() ? aAction.GetMenuItem() : aOverrideLabel; - wxMenuItem* item = new wxMenuItem( this, aAction.GetUIId(), menuLabel, - aAction.GetTooltip(), + wxMenuItem* item = new wxMenuItem( this, aAction.GetUIId(), menuLabel, aAction.GetTooltip(), aIsCheckmarkEntry ? wxITEM_CHECK : wxITEM_NORMAL ); if( !!icon ) KIUI::AddBitmapToMenuItem( item, KiBitmapBundle( icon ) ); @@ -194,8 +191,7 @@ wxMenuItem* ACTION_MENU::Add( ACTION_MENU* aMenu ) { m_submenus.push_back( aMenu ); - wxASSERT_MSG( !aMenu->m_title.IsEmpty(), - wxS( "Set a title for ACTION_MENU using SetTitle()" ) ); + wxASSERT_MSG( !aMenu->m_title.IsEmpty(), wxS( "Set a title for ACTION_MENU using SetTitle()" ) ); if( !!aMenu->m_icon ) { @@ -334,8 +330,7 @@ ACTION_MENU* ACTION_MENU::create() const ACTION_MENU* menu = new ACTION_MENU( false ); wxASSERT_MSG( typeid( *this ) == typeid( *menu ), - wxString::Format( "You need to override create() method for class %s", - typeid( *this ).name() ) ); + wxString::Format( "You need to override create() method for class %s", typeid( *this ).name() ) ); return menu; } @@ -524,9 +519,8 @@ void ACTION_MENU::OnMenuEvent( wxMenuEvent& aEvent ) #define ID_CONTEXT_MENU_ID_MAX wxID_LOWEST /* = 100 should be plenty */ - if( !evt && - ( ( m_selected >= 0 && m_selected < ID_CONTEXT_MENU_ID_MAX ) || - ( m_selected >= ID_POPUP_MENU_START && m_selected <= ID_POPUP_MENU_END ) ) ) + if( !evt && ( ( m_selected >= 0 && m_selected < ID_CONTEXT_MENU_ID_MAX ) + || ( m_selected >= ID_POPUP_MENU_START && m_selected <= ID_POPUP_MENU_END ) ) ) { ACTION_MENU* actionMenu = dynamic_cast( GetParent() );