#14884 Reparent hidden field labels when rebuilding form layout

A hidden label is never added to a layout, so it stayed parented to the
group box it was created in. When the field moved directly to another
group, the old group box was deleted together with the label, and the
next update hit a null label in PdmUiCheckBoxEditor.

Add GroupVisibilityByEnum to cafTestApplication to reproduce the crash.
This commit is contained in:
Magne Sjaastad committed 2026-10-09 13:31:39 +02:00
1 parent 4d68f66455
commit 277322cbcb
7 files changed
+142 -4

No files matched your search

@@ -298,8 +298,12 @@ void RicSummaryPlotEditorWidgetCreator::configureAndUpdateFields( int
}
else
{
QWidget* fieldLabelWidget = fieldEditor->labelWidget();
if ( fieldLabelWidget ) fieldLabelWidget->hide();
if ( auto fieldLabelWidget = fieldEditor->labelWidget() )
{
// Reparent so the label is not deleted with a previous group box
fieldLabelWidget->setParent( widget() );
fieldLabelWidget->hide();
}
}
if ( fieldEditorWidget )
@@ -189,8 +189,12 @@ void RiuSummaryVectorSelectionWidgetCreator::configureAndUpdateFields( int
}
else
{
QWidget* fieldLabelWidget = fieldEditor->labelWidget();
if ( fieldLabelWidget ) fieldLabelWidget->hide();
if ( auto fieldLabelWidget = fieldEditor->labelWidget() )
{
// Reparent so the label is not deleted with a previous group box
fieldLabelWidget->setParent( widget() );
fieldLabelWidget->hide();
}
}
if ( fieldEditorWidget )
@@ -66,6 +66,8 @@ set(PROJECT_FILES
PdmObjects/ValidationTest.cpp
PdmObjects/ReentrantEditorRebuild.h
PdmObjects/ReentrantEditorRebuild.cpp
PdmObjects/GroupVisibilityByEnum.h
PdmObjects/GroupVisibilityByEnum.cpp
)
qt_add_executable(
@@ -9,6 +9,7 @@
#include "PdmObjects/ColorTriplet.h"
#include "PdmObjects/DemoPdmObject.h"
#include "PdmObjects/DemoPdmObjectGroup.h"
#include "PdmObjects/GroupVisibilityByEnum.h"
#include "PdmObjects/LabelsAndHyperlinks.h"
#include "PdmObjects/LineEditAndPushButtons.h"
#include "PdmObjects/ManyGroups.h"
@@ -198,6 +199,7 @@ void MainWindow::buildTestModel()
m_testRoot->objects.push_back( new LabelsAndHyperlinks );
m_testRoot->objects.push_back( new OptionalFields );
m_testRoot->objects.push_back( new ReentrantEditorRebuild );
m_testRoot->objects.push_back( new GroupVisibilityByEnum );
auto tamComboBox = new TamComboBox;
m_testRoot->objects.push_back( tamComboBox );
@@ -0,0 +1,65 @@
#include "GroupVisibilityByEnum.h"
#include "cafPdmUiCheckBoxEditor.h"
#include "cafPdmUiOrdering.h"
template <>
void caf::AppEnum<GroupVisibilityByEnum::SchedulerMode>::setUp()
{
addItem( GroupVisibilityByEnum::SchedulerMode::LOCAL, "LOCAL", "Local" );
addItem( GroupVisibilityByEnum::SchedulerMode::SLURM, "SLURM", "Slurm" );
addItem( GroupVisibilityByEnum::SchedulerMode::LSF, "LSF", "LSF" );
setDefault( GroupVisibilityByEnum::SchedulerMode::LOCAL );
}
CAF_PDM_SOURCE_INIT( GroupVisibilityByEnum, "GroupVisibilityByEnum" );
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
GroupVisibilityByEnum::GroupVisibilityByEnum()
{
CAF_PDM_InitObject( "Group Visibility By Enum", "", "", "" );
CAF_PDM_InitFieldNoDefault( &m_mode, "Mode", "Mode" );
CAF_PDM_InitField( &m_maxParallelJobs, "MaxParallelJobs", 1, "Max Parallel Jobs", "", "", "" );
CAF_PDM_InitFieldNoDefault( &m_queueName, "QueueName", "Queue Name" );
m_queueName = "default";
CAF_PDM_InitField( &m_exclusive, "Exclusive", false, "Exclusive", "", "", "" );
caf::PdmUiNativeCheckBoxEditor::configureFieldForEditor( &m_exclusive );
CAF_PDM_InitFieldNoDefault( &m_schedulerOptions, "SchedulerOptions", "Scheduler Options" );
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
void GroupVisibilityByEnum::defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering )
{
auto genGroup = uiOrdering.addNewGroup( "General Settings" );
genGroup->add( &m_mode );
if ( m_mode() == SchedulerMode::LOCAL )
{
auto localGroup = uiOrdering.addNewGroup( "Local Options" );
localGroup->add( &m_maxParallelJobs );
}
else if ( m_mode() == SchedulerMode::SLURM )
{
auto slurmGroup = uiOrdering.addNewGroup( "Slurm Options" );
slurmGroup->add( &m_queueName );
slurmGroup->add( &m_exclusive );
slurmGroup->add( &m_schedulerOptions );
}
else if ( m_mode() == SchedulerMode::LSF )
{
auto lsfGroup = uiOrdering.addNewGroup( "LSF Options" );
lsfGroup->add( &m_queueName );
lsfGroup->add( &m_exclusive );
lsfGroup->add( &m_schedulerOptions );
}
}
@@ -0,0 +1,58 @@
#pragma once
#include "cafPdmField.h"
#include "cafPdmObject.h"
//==================================================================================================
/// Reproduction of https://github.com/OPM/ResInsight/issues/14884
///
/// Modeled directly after RiaPreferencesHpc::appendItems() (branch origin/hpc_interface), which
/// crashed in caf::PdmUiCheckBoxEditor::configureAndUpdateUi() when switching the "Batch Scheduler"
/// enum directly between SLURM and LSF: both options share the exact same three fields
/// (queue name, exclusive checkbox, scheduler options) inside two *differently named* ui groups.
///
/// The critical detail is that the shared fields must move directly from one group to the other,
/// without ever passing through an intermediate state where they are absent from the ui ordering.
/// If they pass through an absent state first, the field editors are cleanly destroyed and
/// recreated (safe). Going directly from one group to the other instead reuses/reparents the live
/// field editor widgets from the old (about-to-be-deleted) group box into the newly created one.
///
/// Steps to reproduce:
/// 1. Select this object in the tree view so the property editor is shown.
/// 2. Change "Mode" from "Local" to "Slurm" (no crash expected -- fresh group/fields created).
/// 3. Change "Mode" directly from "Slurm" to "LSF" (skip "Local"). This reparents the shared
/// queue name / exclusive / options fields from the "Slurm Options" group into a brand new
/// "LSF Options" group within the same ui rebuild. This is the step that reproduced the
/// crash in PdmUiCheckBoxEditor::configureAndUpdateUi().
/// 4. Switch back and forth directly between "Slurm" and "LSF" a few times if step 3 doesn't
/// crash immediately.
//==================================================================================================
class GroupVisibilityByEnum : public caf::PdmObject
{
CAF_PDM_HEADER_INIT;
public:
enum class SchedulerMode
{
LOCAL,
SLURM,
LSF
};
public:
GroupVisibilityByEnum();
private:
void defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering ) override;
private:
caf::PdmField<caf::AppEnum<SchedulerMode>> m_mode;
// Local-only member
caf::PdmField<int> m_maxParallelJobs;
// Shared between the Slurm Options group and the LSF Options group.
caf::PdmField<QString> m_queueName;
caf::PdmField<bool> m_exclusive;
caf::PdmField<QString> m_schedulerOptions;
};
@@ -283,6 +283,9 @@ int caf::PdmUiFormLayoutObjectEditor::recursivelyConfigureAndUpdateUiOrderingInG
if ( labelPos == PdmUiItemInfo::LabelPosition::HIDDEN )
{
// Not added to the layout, so reparent explicitly. Otherwise the label stays in a
// previous group box and is deleted with it when the field moves to another group.
fieldLabelWidget->setParent( containerWidgetWithGridLayout );
fieldLabelWidget->hide();
}
else if ( labelPos == PdmUiItemInfo::LabelPosition::TOP )