From cb80ff746b687a3ad29b53d9f633cbdc6b142c71 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Thu, 27 Feb 2020 19:46:48 +0100 Subject: [PATCH] crash fix: Don't cast to CylindersModel or CylindersModelFiltered The tank-info-delegate cast its model to CylindersModelFiltered, since this is what the equipment-tab uses since implementing the filtering of unused cylinders. However, the planner users the same delegate and still uses the unfiltered CylindersModel. This means that the (dynamic) cast returns a null pointer and crashes. One possibility would be to derive CylindersModelFiltered and CylindersModel from the same class that defines virtual functions and cast to that class. This is a different attempt: don't cast (i.e. stay with a QAbstractItemModel and play it via Qt's model-view system. Firstly, replace the passInData function by a role to setData(). Secondly, read the working-pressure and size via new columns using data(). Signed-off-by: Berthold Stoeger --- desktop-widgets/modeldelegates.cpp | 22 ++++++------ qt-models/cylindermodel.cpp | 55 +++++++++++++++--------------- qt-models/cylindermodel.h | 7 ++-- 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/desktop-widgets/modeldelegates.cpp b/desktop-widgets/modeldelegates.cpp index ed4833550..2717f37bc 100644 --- a/desktop-widgets/modeldelegates.cpp +++ b/desktop-widgets/modeldelegates.cpp @@ -238,7 +238,7 @@ static struct RevertCylinderData { void TankInfoDelegate::setModelData(QWidget*, QAbstractItemModel*, const QModelIndex&) const { - CylindersModelFiltered *mymodel = qobject_cast(currCombo.model); + QAbstractItemModel *mymodel = currCombo.model; TankInfoModel *tanks = TankInfoModel::instance(); QModelIndexList matches = tanks->match(tanks->index(0, 0), Qt::DisplayRole, currCombo.activeText); int row; @@ -255,8 +255,8 @@ void TankInfoDelegate::setModelData(QWidget*, QAbstractItemModel*, const QModelI int tankPressure = tanks->data(tanks->index(row, TankInfoModel::BAR)).toInt(); mymodel->setData(IDX(CylindersModel::TYPE), cylinderName, Qt::EditRole); - mymodel->passInData(IDX(CylindersModel::WORKINGPRESS), tankPressure); - mymodel->passInData(IDX(CylindersModel::SIZE), tankSize); + mymodel->setData(IDX(CylindersModel::WORKINGPRESS), tankPressure, CylindersModel::PASS_IN_ROLE); + mymodel->setData(IDX(CylindersModel::SIZE), tankSize, CylindersModel::PASS_IN_ROLE); } TankInfoDelegate::TankInfoDelegate(QObject *parent) : ComboBoxDelegate(TankInfoModel::instance(), parent, true) @@ -277,10 +277,10 @@ void TankInfoDelegate::editorClosed(QWidget*, QAbstractItemDelegate::EndEditHint { if (hint == QAbstractItemDelegate::NoHint || hint == QAbstractItemDelegate::RevertModelCache) { - CylindersModelFiltered *mymodel = qobject_cast(currCombo.model); + QAbstractItemModel *mymodel = currCombo.model; mymodel->setData(IDX(CylindersModel::TYPE), currCylinderData.type, Qt::EditRole); - mymodel->passInData(IDX(CylindersModel::WORKINGPRESS), currCylinderData.pressure); - mymodel->passInData(IDX(CylindersModel::SIZE), currCylinderData.size); + mymodel->setData(IDX(CylindersModel::WORKINGPRESS), currCylinderData.pressure, CylindersModel::PASS_IN_ROLE); + mymodel->setData(IDX(CylindersModel::SIZE), currCylinderData.size, CylindersModel::PASS_IN_ROLE); } } @@ -289,11 +289,11 @@ QWidget *TankInfoDelegate::createEditor(QWidget *parent, const QStyleOptionViewI // ncreate editor needs to be called before because it will populate a few // things in the currCombo global var. QWidget *delegate = ComboBoxDelegate::createEditor(parent, option, index); - CylindersModelFiltered *mymodel = qobject_cast(currCombo.model); - cylinder_t *cyl = mymodel->cylinderAt(index); - currCylinderData.type = cyl->type.description; - currCylinderData.pressure = cyl->type.workingpressure.mbar; - currCylinderData.size = cyl->type.size.mliter; + QAbstractItemModel *model = currCombo.model; + int row = index.row(); + currCylinderData.type = model->data(model->index(row, CylindersModel::TYPE)).value(); + currCylinderData.pressure = model->data(model->index(row, CylindersModel::WORKINGPRESS_INT)).value(); + currCylinderData.size = model->data(model->index(row, CylindersModel::SIZE_INT)).value(); MainWindow::instance()->graphics->setReplot(false); return delegate; } diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp index 4f5a4ab77..4c56d9914 100644 --- a/qt-models/cylindermodel.cpp +++ b/qt-models/cylindermodel.cpp @@ -236,6 +236,10 @@ QVariant CylindersModel::data(const QModelIndex &index, int role) const break; case USE: return gettextFromC::tr(cylinderuse_text[cyl->cylinder_use]); + case WORKINGPRESS_INT: + return static_cast(cyl->type.workingpressure.mbar); + case SIZE_INT: + return static_cast(cyl->type.size.mliter); } break; case Qt::DecorationRole: @@ -283,33 +287,35 @@ cylinder_t *CylindersModel::cylinderAt(const QModelIndex &index) return get_cylinder(&displayed_dive, index.row()); } -// this is our magic 'pass data in' function that allows the delegate to get -// the data here without silly unit conversions; -// so we only implement the two columns we care about -void CylindersModel::passInData(const QModelIndex &index, const QVariant &value) -{ - cylinder_t *cyl = cylinderAt(index); - switch (index.column()) { - case SIZE: - if (cyl->type.size.mliter != value.toInt()) { - cyl->type.size.mliter = value.toInt(); - dataChanged(index, index); - } - break; - case WORKINGPRESS: - if (cyl->type.workingpressure.mbar != value.toInt()) { - cyl->type.workingpressure.mbar = value.toInt(); - dataChanged(index, index); - } - break; - } -} - bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, int role) { QString vString; cylinder_t *cyl = cylinderAt(index); + if (!cyl) + return false; + + if (role == PASS_IN_ROLE) { + // this is our magic 'pass data in' function that allows the delegate to get + // the data here without silly unit conversions; + // so we only implement the two columns we care about + switch (index.column()) { + case SIZE: + if (cyl->type.size.mliter != value.toInt()) { + cyl->type.size.mliter = value.toInt(); + dataChanged(index, index); + } + return true; + case WORKINGPRESS: + if (cyl->type.workingpressure.mbar != value.toInt()) { + cyl->type.workingpressure.mbar = value.toInt(); + dataChanged(index, index); + } + return true; + } + return false; + } + switch (index.column()) { case TYPE: if (!value.isNull()) { @@ -641,11 +647,6 @@ void CylindersModelFiltered::remove(QModelIndex index) source.remove(mapToSource(index)); } -void CylindersModelFiltered::passInData(const QModelIndex &index, const QVariant &value) -{ - source.passInData(mapToSource(index), value); -} - cylinder_t *CylindersModelFiltered::cylinderAt(const QModelIndex &index) { return source.cylinderAt(mapToSource(index)); diff --git a/qt-models/cylindermodel.h b/qt-models/cylindermodel.h index 9534af8ad..41de5fae5 100644 --- a/qt-models/cylindermodel.h +++ b/qt-models/cylindermodel.h @@ -25,16 +25,20 @@ public: MOD, MND, USE, + WORKINGPRESS_INT, + SIZE_INT, COLUMNS }; + enum Roles { + PASS_IN_ROLE = Qt::UserRole + 1 // For setting data: don't do any conversions + }; explicit CylindersModel(QObject *parent = 0); QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; int rowCount(const QModelIndex &parent = QModelIndex()) const override; Qt::ItemFlags flags(const QModelIndex &index) const override; bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole) override; - void passInData(const QModelIndex &index, const QVariant &value); void add(); void clear(); void updateDive(); @@ -67,7 +71,6 @@ public: void add(); void updateDive(); cylinder_t *cylinderAt(const QModelIndex &index); - void passInData(const QModelIndex &index, const QVariant &value); public slots: void remove(QModelIndex index);