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(); } 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 ) ); + } +} 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/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/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/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 ); +} 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 ); +} 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})