From 3d715cd18e9e4e9674a7f823f63f7e02aea516df Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Thu, 29 Oct 2026 08:21:53 +0100 Subject: [PATCH 1/5] Guard missing view and case in box intersection cell bounding box Avoid asserting when the box intersection has no owning 3D view or the view has no case. --- .../Intersections/RimBoxIntersection.cpp | 106 ++++++++++-------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/ApplicationLibCode/ProjectDataModel/Intersections/RimBoxIntersection.cpp b/ApplicationLibCode/ProjectDataModel/Intersections/RimBoxIntersection.cpp index a6d0b3fc78..c0856bff07 100644 --- a/ApplicationLibCode/ProjectDataModel/Intersections/RimBoxIntersection.cpp +++ b/ApplicationLibCode/ProjectDataModel/Intersections/RimBoxIntersection.cpp @@ -261,7 +261,9 @@ RimIntersectionCurtain RimBoxIntersection::surfaceCurtain() const void RimBoxIntersection::setToDefaultSizeBox() { cvf::BoundingBox boundingBox = currentCellBoundingBox(); - cvf::Vec3d center = boundingBox.center(); + if ( !boundingBox.isValid() ) return; + + cvf::Vec3d center = boundingBox.center(); double defaultWidthFactor = 0.5; @@ -281,30 +283,33 @@ void RimBoxIntersection::setToDefaultSizeSlice( SinglePlaneState plane, const cv m_singlePlaneState = plane; cvf::BoundingBox boundingBox = currentCellBoundingBox(); - cvf::Vec3d center = position; + if ( boundingBox.isValid() ) + { + cvf::Vec3d center = position; - if ( center.isUndefined() ) center = boundingBox.center(); + if ( center.isUndefined() ) center = boundingBox.center(); - double defaultWidthFactor = 0.5; + double defaultWidthFactor = 0.5; - m_minXCoord = center[0] - 0.5 * boundingBox.extent().x() * defaultWidthFactor; - m_minYCoord = center[1] - 0.5 * boundingBox.extent().y() * defaultWidthFactor; - m_minDepth = -( center[2] + 0.5 * boundingBox.extent().z() * defaultWidthFactor ); - m_maxXCoord = center[0] + 0.5 * boundingBox.extent().x() * defaultWidthFactor; - m_maxYCoord = center[1] + 0.5 * boundingBox.extent().y() * defaultWidthFactor; - m_maxDepth = -( center[2] - 0.5 * boundingBox.extent().z() * defaultWidthFactor ); + m_minXCoord = center[0] - 0.5 * boundingBox.extent().x() * defaultWidthFactor; + m_minYCoord = center[1] - 0.5 * boundingBox.extent().y() * defaultWidthFactor; + m_minDepth = -( center[2] + 0.5 * boundingBox.extent().z() * defaultWidthFactor ); + m_maxXCoord = center[0] + 0.5 * boundingBox.extent().x() * defaultWidthFactor; + m_maxYCoord = center[1] + 0.5 * boundingBox.extent().y() * defaultWidthFactor; + m_maxDepth = -( center[2] - 0.5 * boundingBox.extent().z() * defaultWidthFactor ); - switch ( plane ) - { - case PLANE_STATE_X: - m_minXCoord = m_maxXCoord = center[0]; - break; - case PLANE_STATE_Y: - m_minYCoord = m_maxYCoord = center[1]; - break; - case PLANE_STATE_Z: - m_minDepth = m_maxDepth = -center[2]; - break; + switch ( plane ) + { + case PLANE_STATE_X: + m_minXCoord = m_maxXCoord = center[0]; + break; + case PLANE_STATE_Y: + m_minYCoord = m_maxYCoord = center[1]; + break; + case PLANE_STATE_Z: + m_minDepth = m_maxDepth = -center[2]; + break; + } } updateVisibility(); @@ -447,23 +452,27 @@ void RimBoxIntersection::defineUiOrdering( QString uiConfigName, caf::PdmUiOrder } cvf::BoundingBox cellsBoundingBox = currentCellBoundingBox(); + const bool hasCellRange = cellsBoundingBox.isValid(); + + auto rangeText = []( double minimum, double maximum ) { return QString( " [%1 %2]" ).arg( minimum ).arg( maximum ); }; + { caf::PdmUiGroup* group = uiOrdering.addNewGroup( - "X Coordinates " + QString( " [%1 %2]" ).arg( cellsBoundingBox.min().x() ).arg( cellsBoundingBox.max().x() ) ); + "X Coordinates " + ( hasCellRange ? rangeText( cellsBoundingBox.min().x(), cellsBoundingBox.max().x() ) : QString() ) ); group->add( &m_minXCoord ); group->add( &m_maxXCoord ); } { caf::PdmUiGroup* group = uiOrdering.addNewGroup( - "Y Coordinates" + QString( " [%1 %2]" ).arg( cellsBoundingBox.min().y() ).arg( cellsBoundingBox.max().y() ) ); + "Y Coordinates" + ( hasCellRange ? rangeText( cellsBoundingBox.min().y(), cellsBoundingBox.max().y() ) : QString() ) ); group->add( &m_minYCoord ); group->add( &m_maxYCoord ); } { - caf::PdmUiGroup* group = - uiOrdering.addNewGroup( "Depth" + QString( " [%1 %2]" ).arg( -cellsBoundingBox.max().z() ).arg( -cellsBoundingBox.min().z() ) ); + caf::PdmUiGroup* group = uiOrdering.addNewGroup( + "Depth" + ( hasCellRange ? rangeText( -cellsBoundingBox.max().z(), -cellsBoundingBox.min().z() ) : QString() ) ); group->add( &m_minDepth ); group->add( &m_maxDepth ); } @@ -474,28 +483,31 @@ void RimBoxIntersection::defineUiOrdering( QString uiConfigName, caf::PdmUiOrder group->add( &m_depthSliderStepSize ); } - auto setSliderAttributes = []( auto& field, double minimum, double maximum, int tickCount ) + if ( hasCellRange ) { - field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::MINIMUM, minimum ); - field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::MAXIMUM, maximum ); - field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::SLIDER_TICK_COUNT, cvf::Math::abs( tickCount ) ); - }; + auto setSliderAttributes = []( auto& field, double minimum, double maximum, int tickCount ) + { + field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::MINIMUM, minimum ); + field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::MAXIMUM, maximum ); + field.uiCapability()->setAttribute( caf::PdmUiDoubleSliderEditor::Keys::SLIDER_TICK_COUNT, cvf::Math::abs( tickCount ) ); + }; - const int xTickCount = static_cast( static_cast( cellsBoundingBox.extent().x() ) / m_xySliderStepSize ); - const int yTickCount = static_cast( static_cast( cellsBoundingBox.extent().y() ) / m_xySliderStepSize ); - const int depthTickCount = static_cast( static_cast( cellsBoundingBox.extent().z() ) / m_depthSliderStepSize ); + const int xTickCount = static_cast( static_cast( cellsBoundingBox.extent().x() ) / m_xySliderStepSize ); + const int yTickCount = static_cast( static_cast( cellsBoundingBox.extent().y() ) / m_xySliderStepSize ); + const int depthTickCount = static_cast( static_cast( cellsBoundingBox.extent().z() ) / m_depthSliderStepSize ); - for ( auto* field : { &m_minXCoord, &m_maxXCoord } ) - { - setSliderAttributes( *field, cellsBoundingBox.min().x(), cellsBoundingBox.max().x(), xTickCount ); - } - for ( auto* field : { &m_minYCoord, &m_maxYCoord } ) - { - setSliderAttributes( *field, cellsBoundingBox.min().y(), cellsBoundingBox.max().y(), yTickCount ); - } - for ( auto* field : { &m_minDepth, &m_maxDepth } ) - { - setSliderAttributes( *field, -cellsBoundingBox.max().z(), -cellsBoundingBox.min().z(), depthTickCount ); + for ( auto* field : { &m_minXCoord, &m_maxXCoord } ) + { + setSliderAttributes( *field, cellsBoundingBox.min().x(), cellsBoundingBox.max().x(), xTickCount ); + } + for ( auto* field : { &m_minYCoord, &m_maxYCoord } ) + { + setSliderAttributes( *field, cellsBoundingBox.min().y(), cellsBoundingBox.max().y(), yTickCount ); + } + for ( auto* field : { &m_minDepth, &m_maxDepth } ) + { + setSliderAttributes( *field, -cellsBoundingBox.max().z(), -cellsBoundingBox.min().z(), depthTickCount ); + } } uiOrdering.addNewButton( m_show3DManipulator ? "Hide 3D manipulator" : "Show 3D manipulator", @@ -681,7 +693,11 @@ void RimBoxIntersection::switchSingelPlaneState() //-------------------------------------------------------------------------------------------------- cvf::BoundingBox RimBoxIntersection::currentCellBoundingBox() { - auto rimCase = firstAncestorOrThisOfTypeAsserted()->ownerCase(); + auto rimView = firstAncestorOrThisOfType(); + if ( !rimView ) return {}; + + auto rimCase = rimView->ownerCase(); + if ( !rimCase ) return {}; return rimCase->activeCellsBoundingBox(); } From 797838e425bf21ef41ad49c49164a4ae22ca1067 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Thu, 29 Oct 2026 08:21:53 +0100 Subject: [PATCH 2/5] Hide stale property editor widgets before deferred deletion Deferred deletes are not processed by nested event loops, such as the processEvents() calls when the regression test runner selects all project objects. Stale labels, buttons and field editor widgets stayed visible in reused containers and accumulated, until QWidgetPrivate::paintSiblingsRecursive overflowed the stack. Hide and detach the widgets before calling deleteLater(). --- .../cafPdmUiFormLayoutObjectEditor.cpp | 73 ++++++++++++------- .../cafPdmUiFormLayoutObjectEditorTest.cpp | 66 +++++++++++++++++ 2 files changed, 111 insertions(+), 28 deletions(-) diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp index 5f12f642c0..b4a19f2456 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp @@ -65,24 +65,51 @@ namespace { //-------------------------------------------------------------------------------------------------- -/// Schedule deletion of a group box without destroying the widgets inside it synchronously. +/// Hide and detach a widget that is about to be deleted with deleteLater(). +/// +/// The widget is taken out of the layout, the visual tree and the focus chain immediately. This is what plain +/// deleteLater() failed to do in https://github.com/OPM/ResInsight/issues/9719 +/// +/// Deferred deletes are only processed when control returns to the event loop that called deleteLater(), not +/// by nested QApplication::processEvents() calls. Widgets left visible in a reused parent accumulate, and Qt +/// paints visible siblings recursively, which eventually overflows the stack (e.g. in the regression test +/// runner selecting all project objects). +//-------------------------------------------------------------------------------------------------- +auto detachWidget = []( QWidget* widget ) +{ + if ( !widget ) return; + + widget->hide(); + widget->setParent( nullptr ); +}; + +//-------------------------------------------------------------------------------------------------- +/// Schedule deletion of a widget without destroying it or its child widgets synchronously. /// /// A plain delete destroys the child widgets immediately, which defeats the deleteLater() used by /// PdmUiFieldEditorHandle to keep field editor widgets alive. When the rebuild is triggered from inside a /// widget event handler, Qt is still using those widgets further up the stack. /// See https://github.com/OPM/ResInsight/issues/14505 -/// -/// The group box is hidden and detached from its parent before the deferred delete is scheduled, so it is -/// out of the layout, the visual tree and the focus chain immediately. This is what plain deleteLater() -/// failed to do in https://github.com/OPM/ResInsight/issues/9719 //-------------------------------------------------------------------------------------------------- -auto scheduleGroupBoxDeletion = []( QMinimizePanel* groupBox ) +auto scheduleWidgetDeletion = []( QWidget* widget ) { - if ( !groupBox ) return; + if ( !widget ) return; - groupBox->hide(); - groupBox->setParent( nullptr ); - groupBox->deleteLater(); + detachWidget( widget ); + widget->deleteLater(); +}; + +//-------------------------------------------------------------------------------------------------- +/// Detach all widgets of a field editor before it is deleted. The field editor destructor schedules the +/// deletion of the widgets with deleteLater(). +//-------------------------------------------------------------------------------------------------- +auto detachFieldEditorWidgets = []( caf::PdmUiFieldEditorHandle* fieldEditor ) +{ + if ( !fieldEditor ) return; + + detachWidget( fieldEditor->combinedWidget() ); + detachWidget( fieldEditor->editorWidget() ); + detachWidget( fieldEditor->labelWidget() ); }; } // namespace @@ -572,6 +599,7 @@ caf::PdmUiFieldEditorHandle* caf::PdmUiFormLayoutObjectEditor::findOrCreateField // The editor's field was deleted and a new field was allocated at the same address if ( it != m_fieldViews.end() && it->second->uiField() != field ) { + detachFieldEditorWidgets( it->second ); delete it->second; m_fieldViews.erase( it ); it = m_fieldViews.end(); @@ -687,10 +715,7 @@ void caf::PdmUiFormLayoutObjectEditor::cleanupBeforeSettingPdmObject() // https://github.com/OPM/ResInsight/issues/12599 for ( const auto& [fieldHandle, fieldEditor] : m_fieldViews ) { - if ( fieldEditor && fieldEditor->editorWidget() ) - { - fieldEditor->editorWidget()->hide(); - } + detachFieldEditorWidgets( fieldEditor ); } // Delete all field editors, make sure this is done after the field editors have been hidden @@ -711,7 +736,7 @@ void caf::PdmUiFormLayoutObjectEditor::cleanupBeforeSettingPdmObject() QMinimizePanel* groupBox = groupIt->second; if ( groupBox ) { - scheduleGroupBoxDeletion( groupBox ); + scheduleWidgetDeletion( groupBox ); } } @@ -730,28 +755,19 @@ void caf::PdmUiFormLayoutObjectEditor::deleteLabelsAndButtons() { for ( auto& label : m_labels ) { - if ( label ) - { - label->deleteLater(); - } + scheduleWidgetDeletion( label ); } m_labels.clear(); for ( auto& button : m_buttons ) { - if ( button ) - { - button->deleteLater(); - } + scheduleWidgetDeletion( button ); } m_buttons.clear(); for ( auto& buttonBox : m_buttonBoxes ) { - if ( buttonBox ) - { - buttonBox->deleteLater(); - } + scheduleWidgetDeletion( buttonBox ); } m_buttonBoxes.clear(); } @@ -790,6 +806,7 @@ void caf::PdmUiFormLayoutObjectEditor::configureAndUpdateUi( const QString& uiCo if ( m_usedFields.count( oldFvIt->first ) == 0 ) { // The old field editor is not present anymore, get rid of it + detachFieldEditorWidgets( oldFvIt->second ); delete oldFvIt->second; fvhToRemoveFromMap.push_back( oldFvIt->first ); } @@ -811,7 +828,7 @@ void caf::PdmUiFormLayoutObjectEditor::configureAndUpdateUi( const QString& uiCo if ( itNew == m_newGroupBoxes.end() ) { // The old groupBox is not present anymore, get rid of it - if ( !itOld->second.isNull() ) scheduleGroupBoxDeletion( itOld->second ); + if ( !itOld->second.isNull() ) scheduleWidgetDeletion( itOld->second ); } } m_groupBoxes = m_newGroupBoxes; diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp index 726c0d309b..53e9f2dd4b 100644 --- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp @@ -6,6 +6,8 @@ #include "cafPdmUiOrdering.h" #include "cafPdmUiPropertyView.h" +#include + #include using namespace caf; @@ -95,3 +97,67 @@ TEST( PdmUiFormLayoutObjectEditorTest, FieldReplacedAtSameAddress ) view.showProperties( nullptr ); } + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +class FormLayoutTestLabelObject : public caf::PdmObject +{ + CAF_PDM_HEADER_INIT; + +public: + FormLayoutTestLabelObject() + { + CAF_PDM_InitObject( "FormLayoutTestLabelObject" ); + CAF_PDM_InitField( &m_text, "Text", QString( "text" ), "FieldLabel" ); + } + + caf::PdmField m_text; + +protected: + void defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering ) override + { + uiOrdering.addNewLabel( "InfoLabel" ); + uiOrdering.add( &m_text ); + uiOrdering.skipRemainingFields(); + } +}; +CAF_PDM_SOURCE_INIT( FormLayoutTestLabelObject, "FormLayoutTestLabelObject" ); + +namespace +{ +int visibleLabelCount( const QWidget& root, const QString& text ) +{ + int count = 0; + for ( auto label : root.findChildren() ) + { + if ( !label->isHidden() && label->text().startsWith( text ) ) count++; + } + return count; +} +} // namespace + +//-------------------------------------------------------------------------------------------------- +/// Widgets scheduled for deferred deletion must be hidden immediately. Deferred deletes are not processed by nested +/// event loops, and many visible stale siblings overflow the stack in QWidgetPrivate::paintSiblingsRecursive. +//-------------------------------------------------------------------------------------------------- +TEST( PdmUiFormLayoutObjectEditorTest, StaleWidgetsAreHiddenWithoutEventLoop ) +{ + FormLayoutTestLabelObject first; + FormLayoutTestLabelObject second; + + PdmUiPropertyView view; + view.showProperties( &first ); + + for ( int i = 0; i < 5; i++ ) + { + first.updateConnectedEditors(); + } + EXPECT_EQ( 1, visibleLabelCount( view, "InfoLabel" ) ); + + view.showProperties( &second ); + EXPECT_EQ( 1, visibleLabelCount( view, "InfoLabel" ) ); + EXPECT_EQ( 1, visibleLabelCount( view, "FieldLabel" ) ); + + view.showProperties( nullptr ); +} From 7b2cef9f79f6dd444529c851c76881f0ceb75b43 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Thu, 29 Oct 2026 09:54:57 +0100 Subject: [PATCH 3/5] Make summary table row ordering deterministic Rows with equal first and last time step kept the incoming address order, which depends on string pool indices and varies between sessions. Break ties on category name. --- .../Summary/RimSummaryTableTools.cpp | 16 ++++---- ApplicationLibCode/UnitTests/CMakeLists.txt | 1 + .../UnitTests/RimSummaryTableTools-Test.cpp | 37 +++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 ApplicationLibCode/UnitTests/RimSummaryTableTools-Test.cpp diff --git a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryTableTools.cpp b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryTableTools.cpp index ec55e22151..a93482ea3f 100644 --- a/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryTableTools.cpp +++ b/ApplicationLibCode/ProjectDataModel/Summary/RimSummaryTableTools.cpp @@ -18,6 +18,8 @@ #include "RimSummaryTableTools.h" +#include +#include #include //-------------------------------------------------------------------------------------------------- @@ -52,18 +54,16 @@ std::vector //-------------------------------------------------------------------------------------------------- void RimSummaryTableTools::sortVectorDataOnDate( TableData& rTableData ) { - // Sort vector data on date (vectors with no values above threshold placed last): + // Sort vector data on date (vectors with no values above threshold placed last). Ties are sorted on category name, as the + // incoming order depends on string pool indices and is not deterministic. std::sort( rTableData.vectorDataCollection.begin(), rTableData.vectorDataCollection.end(), [&rTableData]( const VectorData& v1, const VectorData& v2 ) { - const bool firstHasValueAboveThreshold = hasValueAboveThreshold( v1.values, rTableData.thresholdValue ); - const bool secondHasValueAboveThreshold = hasValueAboveThreshold( v2.values, rTableData.thresholdValue ); - if ( !firstHasValueAboveThreshold ) return false; - if ( firstHasValueAboveThreshold && !secondHasValueAboveThreshold ) return true; - if ( v1.firstTimeStep < v2.firstTimeStep ) return true; - if ( v1.firstTimeStep == v2.firstTimeStep && v1.lastTimeStep < v2.lastTimeStep ) return true; - return false; + const bool firstHasNoValueAboveThreshold = !hasValueAboveThreshold( v1.values, rTableData.thresholdValue ); + const bool secondHasNoValueAboveThreshold = !hasValueAboveThreshold( v2.values, rTableData.thresholdValue ); + return std::tie( firstHasNoValueAboveThreshold, v1.firstTimeStep, v1.lastTimeStep, v1.category ) < + std::tie( secondHasNoValueAboveThreshold, v2.firstTimeStep, v2.lastTimeStep, v2.category ); } ); } diff --git a/ApplicationLibCode/UnitTests/CMakeLists.txt b/ApplicationLibCode/UnitTests/CMakeLists.txt index 2ab2ada0ea..6333406504 100644 --- a/ApplicationLibCode/UnitTests/CMakeLists.txt +++ b/ApplicationLibCode/UnitTests/CMakeLists.txt @@ -49,6 +49,7 @@ set(SOURCE_UNITTEST_FILES ${CMAKE_CURRENT_LIST_DIR}/RigWellPathIntersectionTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RimHistogramDataSource-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RimPlotAxisTools-Test.cpp + ${CMAKE_CURRENT_LIST_DIR}/RimSummaryTableTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RimWellLogExtractionCurveImpl-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RivAnnotationTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RivNamedVisualizationModels-Test.cpp diff --git a/ApplicationLibCode/UnitTests/RimSummaryTableTools-Test.cpp b/ApplicationLibCode/UnitTests/RimSummaryTableTools-Test.cpp new file mode 100644 index 0000000000..60eaffd9f3 --- /dev/null +++ b/ApplicationLibCode/UnitTests/RimSummaryTableTools-Test.cpp @@ -0,0 +1,37 @@ +#include "gtest/gtest.h" + +#include "RimSummaryTableTools.h" + +#include + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RimSummaryTableToolsTest, SortVectorDataOnDateIsIndependentOfInputOrder ) +{ + using namespace RimSummaryTableTools; + + auto vector = []( const QString& category, double value, time_t first, time_t last ) + { return VectorData{ .category = category, .name = "WOPR", .values = { value }, .firstTimeStep = first, .lastTimeStep = last }; }; + + const std::vector input = { vector( "E-2H", 1.0, 20, 30 ), + vector( "Z-1H", 0.0, 0, 0 ), + vector( "B-1H", 1.0, 20, 30 ), + vector( "C-1H", 1.0, 10, 40 ), + vector( "A-1H", 0.0, 0, 0 ), + vector( "D-1H", 1.0, 20, 25 ) }; + + const std::vector expected = { "C-1H", "D-1H", "B-1H", "E-2H", "A-1H", "Z-1H" }; + + for ( bool reverse : { false, true } ) + { + TableData tableData; + tableData.thresholdValue = 0.0; + tableData.vectorDataCollection = input; + if ( reverse ) std::reverse( tableData.vectorDataCollection.begin(), tableData.vectorDataCollection.end() ); + + sortVectorDataOnDate( tableData ); + + EXPECT_EQ( expected, categoryNames( tableData.vectorDataCollection ) ); + } +} From 30161f8fb96e4b62750241f352cf785a80d20103 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Thu, 29 Oct 2026 10:29:25 +0100 Subject: [PATCH 4/5] Avoid updating editors of deleted items in UpdateEditorsScheduler Remove deleted PdmUiItems from the scheduler, and process scheduled updates one item at a time so items deleted during an update are skipped. --- .../cafPdmUiCore/cafPdmUiItem.cpp | 2 + .../cafUpdateEditorsScheduler.cpp | 58 ++++++++-- .../cafPdmUiCore/cafUpdateEditorsScheduler.h | 11 ++ .../cafUserInterface_UnitTests/CMakeLists.txt | 1 + .../cafUpdateEditorsSchedulerTest.cpp | 102 ++++++++++++++++++ 5 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafUpdateEditorsSchedulerTest.cpp diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiItem.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiItem.cpp index 597014e8d9..3f49eed4e1 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiItem.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiItem.cpp @@ -517,6 +517,8 @@ void PdmUiItem::updateAllRequiredEditors() const //-------------------------------------------------------------------------------------------------- PdmUiItem::~PdmUiItem() { + UpdateEditorsScheduler::unscheduleDeletedItem( this ); + std::set::iterator it; for ( it = m_editors.begin(); it != m_editors.end(); ++it ) { diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.cpp index 528ac12a7d..b3e529a224 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.cpp @@ -22,11 +22,25 @@ namespace caf { +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool UpdateEditorsScheduler::sm_isAlive = false; + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- UpdateEditorsScheduler::UpdateEditorsScheduler() { + sm_isAlive = true; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +UpdateEditorsScheduler::~UpdateEditorsScheduler() +{ + sm_isAlive = false; } //-------------------------------------------------------------------------------------------------- @@ -44,25 +58,57 @@ UpdateEditorsScheduler* UpdateEditorsScheduler::instance() //-------------------------------------------------------------------------------------------------- void UpdateEditorsScheduler::scheduleUpdateConnectedEditors( const PdmUiItem* uiItem ) { - m_itemsToUpdate.insert( uiItem ); + if ( !uiItem ) return; + + { + std::lock_guard lock( m_mutex ); + m_itemsToUpdate.insert( uiItem ); + } startTimer( 0 ); } +//-------------------------------------------------------------------------------------------------- +/// Remove a deleted item, to avoid updating editors of a deleted item +//-------------------------------------------------------------------------------------------------- +void UpdateEditorsScheduler::unscheduleDeletedItem( const PdmUiItem* uiItem ) +{ + if ( !sm_isAlive ) return; + + auto scheduler = instance(); + + std::lock_guard lock( scheduler->m_mutex ); + scheduler->m_itemsToUpdate.erase( uiItem ); + scheduler->m_itemsBeingUpdated.erase( uiItem ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- void UpdateEditorsScheduler::performScheduledUpdates() { - for ( auto uiItem : m_itemsToUpdate ) + // Updating editors can delete other scheduled items or schedule new updates. Process a snapshot one item at a time, + // so deleted items are removed by unscheduleDeletedItem() before they are visited. + // The lock is not held while updating editors, as the update can delete items or schedule new updates. { - if ( uiItem ) + std::lock_guard lock( m_mutex ); + m_itemsBeingUpdated.insert( m_itemsToUpdate.begin(), m_itemsToUpdate.end() ); + m_itemsToUpdate.clear(); + } + + while ( true ) + { + const PdmUiItem* uiItem = nullptr; { - uiItem->updateConnectedEditors(); + std::lock_guard lock( m_mutex ); + if ( m_itemsBeingUpdated.empty() ) break; + + uiItem = *m_itemsBeingUpdated.begin(); + m_itemsBeingUpdated.erase( m_itemsBeingUpdated.begin() ); } - } - m_itemsToUpdate.clear(); + uiItem->updateConnectedEditors(); + } } } //namespace caf diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.h index d16f93d486..9fe183d50f 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.h @@ -20,6 +20,7 @@ #include "cafScheduler.h" +#include #include namespace caf @@ -37,8 +38,18 @@ class UpdateEditorsScheduler : public Scheduler void scheduleUpdateConnectedEditors( const PdmUiItem* uiItem ); void performScheduledUpdates() override; + // Called from the PdmUiItem destructor. Safe to call also after the scheduler instance is destroyed, and from + // worker threads deleting PDM objects. + static void unscheduleDeletedItem( const PdmUiItem* uiItem ); + private: + ~UpdateEditorsScheduler() override; + + static bool sm_isAlive; + + std::mutex m_mutex; std::set m_itemsToUpdate; + std::set m_itemsBeingUpdated; }; } // namespace caf diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt index 061d1497c5..843e78804c 100644 --- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt @@ -26,6 +26,7 @@ set(PROJECT_FILES cafPdmUiPropertyViewDialogTest.cpp cafPdmUiEditorHandleTest.cpp cafPdmUiFormLayoutObjectEditorTest.cpp + cafUpdateEditorsSchedulerTest.cpp cafIconProviderTest.cpp cafProgressInfoTest.cpp gtest/gtest-all.cpp diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafUpdateEditorsSchedulerTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafUpdateEditorsSchedulerTest.cpp new file mode 100644 index 0000000000..4b643aaf11 --- /dev/null +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafUpdateEditorsSchedulerTest.cpp @@ -0,0 +1,102 @@ + +#include "gtest/gtest.h" + +#include "cafPdmUiEditorHandle.h" +#include "cafPdmUiItem.h" +#include "cafUpdateEditorsScheduler.h" + +#include +#include + +using namespace caf; + +namespace +{ +class CountingEditor : public caf::PdmUiEditorHandle +{ +public: + int configureCount = 0; + std::function onConfigure; + + void bind( PdmUiItem* item ) { bindToPdmItem( item ); } + +protected: + void configureAndUpdateUi( const QString& ) override + { + configureCount++; + if ( onConfigure ) onConfigure(); + } +}; +} // namespace + +//-------------------------------------------------------------------------------------------------- +/// A scheduled item deleted before the scheduled update used to be accessed after deletion +//-------------------------------------------------------------------------------------------------- +TEST( UpdateEditorsSchedulerTest, DeletedItemIsNotUpdated ) +{ + CountingEditor survivingEditor; + CountingEditor deletedEditor; + + auto survivingItem = std::make_unique(); + auto deletedItem = std::make_unique(); + survivingEditor.bind( survivingItem.get() ); + deletedEditor.bind( deletedItem.get() ); + + survivingItem->scheduleUpdateConnectedEditors(); + deletedItem->scheduleUpdateConnectedEditors(); + deletedItem.reset(); + + UpdateEditorsScheduler::instance()->performScheduledUpdates(); + + EXPECT_EQ( 1, survivingEditor.configureCount ); + EXPECT_EQ( 0, deletedEditor.configureCount ); +} + +//-------------------------------------------------------------------------------------------------- +/// Updating the editors of one item can delete another scheduled item, which must then be skipped +//-------------------------------------------------------------------------------------------------- +TEST( UpdateEditorsSchedulerTest, ItemDeletedDuringUpdateIsSkipped ) +{ + CountingEditor firstEditor; + CountingEditor secondEditor; + + auto firstItem = std::make_unique(); + auto secondItem = std::make_unique(); + firstEditor.bind( firstItem.get() ); + secondEditor.bind( secondItem.get() ); + + // The processing order depends on the item addresses, so each update deletes the other item + firstEditor.onConfigure = [&secondItem]() { secondItem.reset(); }; + secondEditor.onConfigure = [&firstItem]() { firstItem.reset(); }; + + firstItem->scheduleUpdateConnectedEditors(); + secondItem->scheduleUpdateConnectedEditors(); + + UpdateEditorsScheduler::instance()->performScheduledUpdates(); + + EXPECT_EQ( 1, firstEditor.configureCount + secondEditor.configureCount ); +} + +//-------------------------------------------------------------------------------------------------- +/// An update scheduled while updating editors is performed by the next scheduled update +//-------------------------------------------------------------------------------------------------- +TEST( UpdateEditorsSchedulerTest, UpdateScheduledDuringUpdateIsKept ) +{ + CountingEditor editor; + + auto item = std::make_unique(); + editor.bind( item.get() ); + + editor.onConfigure = [&item, &editor]() + { + if ( editor.configureCount == 1 ) item->scheduleUpdateConnectedEditors(); + }; + + item->scheduleUpdateConnectedEditors(); + + UpdateEditorsScheduler::instance()->performScheduledUpdates(); + EXPECT_EQ( 1, editor.configureCount ); + + UpdateEditorsScheduler::instance()->performScheduledUpdates(); + EXPECT_EQ( 2, editor.configureCount ); +} From 107d495cb546151349159e2487639159d6b3c084 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Thu, 29 Oct 2026 10:56:28 +0100 Subject: [PATCH 5/5] Set version to 2026.11.0-RC_1 --- ResInsightVersion.cmake | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ResInsightVersion.cmake b/ResInsightVersion.cmake index 54aa7a9e0b..4a3210d0b5 100644 --- a/ResInsightVersion.cmake +++ b/ResInsightVersion.cmake @@ -1,17 +1,17 @@ set(RESINSIGHT_MAJOR_VERSION 2026) -set(RESINSIGHT_MINOR_VERSION 09) -set(RESINSIGHT_PATCH_VERSION 2) +set(RESINSIGHT_MINOR_VERSION 11) +set(RESINSIGHT_PATCH_VERSION 0) # Opional text with no restrictions -set(RESINSIGHT_VERSION_TEXT "-dev") -#set(RESINSIGHT_VERSION_TEXT "-RC_1") +#set(RESINSIGHT_VERSION_TEXT "-dev") +set(RESINSIGHT_VERSION_TEXT "-RC_1") # Optional text # Must be unique and increasing within one combination of major/minor/patch version # The uniqueness of this text is independent of RESINSIGHT_VERSION_TEXT # Format of text must be ".xx" -set(RESINSIGHT_DEV_VERSION ".05") +#set(RESINSIGHT_DEV_VERSION ".05") set(STRPRODUCTVER ${RESINSIGHT_MAJOR_VERSION}.${RESINSIGHT_MINOR_VERSION}.${RESINSIGHT_PATCH_VERSION}${RESINSIGHT_VERSION_TEXT}${RESINSIGHT_DEV_VERSION})