Stability fixes from regression test runs

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<int>( static_cast<int>( cellsBoundingBox.extent().x() ) / m_xySliderStepSize );
-    const int yTickCount     = static_cast<int>( static_cast<int>( cellsBoundingBox.extent().y() ) / m_xySliderStepSize );
-    const int depthTickCount = static_cast<int>( static_cast<int>( cellsBoundingBox.extent().z() ) / m_depthSliderStepSize );
+        const int xTickCount     = static_cast<int>( static_cast<int>( cellsBoundingBox.extent().x() ) / m_xySliderStepSize );
+        const int yTickCount     = static_cast<int>( static_cast<int>( cellsBoundingBox.extent().y() ) / m_xySliderStepSize );
+        const int depthTickCount = static_cast<int>( static_cast<int>( 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<Rim3dView>()->ownerCase();
+    auto rimView = firstAncestorOrThisOfType<Rim3dView>();
+    if ( !rimView ) return {};
+
+    auto rimCase = rimView->ownerCase();
+    if ( !rimCase ) return {};
 
     return rimCase->activeCellsBoundingBox();
 }

ApplicationLibCode/ProjectDataModel/Summary/RimSummaryTableTools.cpp

@@ -18,6 +18,8 @@
 
 #include "RimSummaryTableTools.h"
 
+#include <algorithm>
+#include <tuple>
 #include <vector>
 
 //--------------------------------------------------------------------------------------------------
@@ -52,18 +54,16 @@ std::vector<RimSummaryTableTools::VectorData>
 //--------------------------------------------------------------------------------------------------
 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 );
                } );
 }
 

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

ApplicationLibCode/UnitTests/RimSummaryTableTools-Test.cpp

@@ -0,0 +1,37 @@
+#include "gtest/gtest.h"
+
+#include "RimSummaryTableTools.h"
+
+#include <algorithm>
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+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<VectorData> 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<QString> 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 ) );
+    }
+}

Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiItem.cpp

@@ -517,6 +517,8 @@ void PdmUiItem::updateAllRequiredEditors() const
 //--------------------------------------------------------------------------------------------------
 PdmUiItem::~PdmUiItem()
 {
+    UpdateEditorsScheduler::unscheduleDeletedItem( this );
+
     std::set<PdmUiEditorHandle*>::iterator it;
     for ( it = m_editors.begin(); it != m_editors.end(); ++it )
     {

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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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

Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafUpdateEditorsScheduler.h

@@ -20,6 +20,7 @@
 
 #include "cafScheduler.h"
 
+#include <mutex>
 #include <set>
 
 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<const PdmUiItem*> m_itemsToUpdate;
+    std::set<const PdmUiItem*> m_itemsBeingUpdated;
 };
 
 } // namespace caf