From 9ac6ad3bec6cdb8064d29cb67a5a3cc27aad8d1a Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Sun, 11 Oct 2026 15:36:48 +0200 Subject: [PATCH] #14903: Avoid duplicate dock window ids for copied plots Copies and plots created from templates kept the source DockWindowId. The dock manager identifies dock widgets by name, so the earlier dock widget was deleted on dock state restore, leaving a dangling pointer that crashed in setActiveViewer. - Do not copy DockWindowId; copies get a new id - Assign a new id if a dock widget with the same name exists - Use QPointer for the dock widget --- .../RimDockWindowController.cpp | 8 +++++ .../ProjectDataModel/RimMultiPlot.cpp | 2 ++ .../ProjectDataModel/RimViewWindow.cpp | 3 ++ .../ProjectDataModel/RimViewWindow.h | 10 +++--- .../RimDockWindowController-Test.cpp | 34 +++++++++++++++++++ 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/ApplicationLibCode/ProjectDataModel/RimDockWindowController.cpp b/ApplicationLibCode/ProjectDataModel/RimDockWindowController.cpp index 96d65030ed..e20b239603 100644 --- a/ApplicationLibCode/ProjectDataModel/RimDockWindowController.cpp +++ b/ApplicationLibCode/ProjectDataModel/RimDockWindowController.cpp @@ -145,6 +145,14 @@ void RimDockWindowController::updateViewerWidget() return; } + // The dock manager identifies dock widgets by name. A duplicate name (e.g. from a project saved with + // copied views) would unregister the existing dock widget, which is then deleted on next dock state restore. + if ( auto existing = mainWindow->dockManager()->findDockWidget( viewPdmObject()->dockWindowName() ); + existing && existing != dockWidget ) + { + viewPdmObject()->resetDockWindowId(); + } + dockWidget->setWidget( viewWidget ); dockWidget->setObjectName( viewPdmObject()->dockWindowName() ); viewWidget->setObjectName( viewPdmObject()->dockWindowName() ); diff --git a/ApplicationLibCode/ProjectDataModel/RimMultiPlot.cpp b/ApplicationLibCode/ProjectDataModel/RimMultiPlot.cpp index 849faaf039..188ad5b9ba 100644 --- a/ApplicationLibCode/ProjectDataModel/RimMultiPlot.cpp +++ b/ApplicationLibCode/ProjectDataModel/RimMultiPlot.cpp @@ -34,6 +34,8 @@ #include "cafPdmUiComboBoxEditor.h" #include "cafPdmUiToolButtonEditor.h" +#include "DockWidget.h" + #include #include diff --git a/ApplicationLibCode/ProjectDataModel/RimViewWindow.cpp b/ApplicationLibCode/ProjectDataModel/RimViewWindow.cpp index bfd16f701c..fd82104f51 100644 --- a/ApplicationLibCode/ProjectDataModel/RimViewWindow.cpp +++ b/ApplicationLibCode/ProjectDataModel/RimViewWindow.cpp @@ -60,6 +60,9 @@ RimViewWindow::RimViewWindow() CAF_PDM_InitField( &m_dockWindowId, "DockWindowId", QString(), "Dock Window Id" ); m_dockWindowId.uiCapability()->setUiHidden( true ); + // The dock id must be unique, as the dock manager identifies dock widgets by name. Copies (including plots created + // from templates) get a new id. + m_dockWindowId.xmlCapability()->setCopyable( false ); } //-------------------------------------------------------------------------------------------------- diff --git a/ApplicationLibCode/ProjectDataModel/RimViewWindow.h b/ApplicationLibCode/ProjectDataModel/RimViewWindow.h index 4b35770d62..04b86b40b1 100644 --- a/ApplicationLibCode/ProjectDataModel/RimViewWindow.h +++ b/ApplicationLibCode/ProjectDataModel/RimViewWindow.h @@ -25,6 +25,8 @@ #include "cafPdmField.h" #include "cafPdmObject.h" +#include + class QImage; class RimDockWindowController; @@ -117,8 +119,8 @@ private: protected: caf::PdmChildField m_windowController; - caf::PdmField m_showWindow; - ads::CDockWidget* m_dockWidget; - caf::PdmField m_dockWindowId; - bool m_isActiveViewer; + caf::PdmField m_showWindow; + QPointer m_dockWidget; + caf::PdmField m_dockWindowId; + bool m_isActiveViewer; }; diff --git a/ApplicationLibCode/UnitTests/RimDockWindowController-Test.cpp b/ApplicationLibCode/UnitTests/RimDockWindowController-Test.cpp index 5fcc2f9e41..cdc90e1665 100644 --- a/ApplicationLibCode/UnitTests/RimDockWindowController-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimDockWindowController-Test.cpp @@ -1,6 +1,11 @@ #include "gtest/gtest.h" #include "RimDockWindowController.h" +#include "RimSummaryPlot.h" + +#include "cafPdmDefaultObjectFactory.h" + +#include //-------------------------------------------------------------------------------------------------- /// handleViewerDeletion() used to dereference viewPdmObject() (m_viewToControl) unconditionally. @@ -12,3 +17,32 @@ TEST( RimDockWindowControllerTest, HandleViewerDeletionWithoutViewToControlDoesN EXPECT_NO_FATAL_FAILURE( controller.handleViewerDeletion() ); } + +//-------------------------------------------------------------------------------------------------- +/// The dock manager identifies dock widgets by the dock window id. Copied views, including plots created from a plot +/// template, must get a new id, while reading a project (or undoing a delete) must keep it. +//-------------------------------------------------------------------------------------------------- +TEST( RimDockWindowControllerTest, DockWindowIdIsNotCopied ) +{ + RimSummaryPlot plot; + const QString dockId = plot.dockWindowName(); + ASSERT_FALSE( dockId.isEmpty() ); + + std::unique_ptr copy( plot.copyObject() ); + ASSERT_TRUE( copy ); + EXPECT_NE( dockId, copy->dockWindowName() ); + + const QString xml = plot.writeObjectToXmlString(); + + std::unique_ptr fromTemplate( + caf::PdmXmlObjectHandle::readUnknownObjectFromXmlString( xml, caf::PdmDefaultObjectFactory::instance(), true ) ); + auto templatePlot = dynamic_cast( fromTemplate.get() ); + ASSERT_TRUE( templatePlot ); + EXPECT_NE( dockId, templatePlot->dockWindowName() ); + + std::unique_ptr fromProject( + caf::PdmXmlObjectHandle::readUnknownObjectFromXmlString( xml, caf::PdmDefaultObjectFactory::instance(), false ) ); + auto projectPlot = dynamic_cast( fromProject.get() ); + ASSERT_TRUE( projectPlot ); + EXPECT_EQ( dockId, projectPlot->dockWindowName() ); +}