From 8d5e2bc4282c54725a69c630b43b8ce9c51bd4e7 Mon Sep 17 00:00:00 2001 From: David Carter Date: Tue, 16 Apr 2024 14:57:48 -0400 Subject: [PATCH] Material: Problems editing Quantity properties There were several issues here, not just one. The following are fixed: Incorrect display of Quantity items (NaN) Editing and updating quantity items Editing and updating items on the first row. There are still issues with editing lists, but these were known issues at the time of initial merge. This has been split out into issue #13435 fixes #13020 --- src/Mod/Material/App/Materials.cpp | 41 +++++++++++++++++++++++ src/Mod/Material/App/Materials.h | 5 +++ src/Mod/Material/Gui/BaseDelegate.cpp | 12 ++++--- src/Mod/Material/Gui/BaseDelegate.h | 2 +- src/Mod/Material/Gui/ListDelegate.cpp | 37 +++----------------- src/Mod/Material/Gui/ListEdit.cpp | 14 +++----- src/Mod/Material/Gui/ListEdit.ui | 27 +++++++++++++++ src/Mod/Material/Gui/ListModel.cpp | 4 ++- src/Mod/Material/Gui/MaterialDelegate.cpp | 20 +++++++---- src/Mod/Material/Gui/MaterialDelegate.h | 3 +- src/Mod/Material/Gui/MaterialsEditor.cpp | 2 +- src/Mod/Material/Gui/MaterialsEditor.h | 2 +- 12 files changed, 110 insertions(+), 59 deletions(-) diff --git a/src/Mod/Material/App/Materials.cpp b/src/Mod/Material/App/Materials.cpp index 2545ca5825..06216009b9 100644 --- a/src/Mod/Material/App/Materials.cpp +++ b/src/Mod/Material/App/Materials.cpp @@ -865,6 +865,15 @@ void Material::setPhysicalValue(const QString& name, const std::shared_ptrsetValue(value); + } +} + void Material::setAppearanceValue(const QString& name, const QString& value) { setAppearanceEditState(name); @@ -893,6 +902,38 @@ void Material::setAppearanceValue(const QString& name, } } +void Material::setAppearanceValue(const QString& name, const QVariant& value) +{ + setAppearanceEditState(name); + + if (hasAppearanceProperty(name)) { + _appearance[name]->setValue(value); + } +} + +void Material::setValue(const QString& name, const QString& value) +{ + if (hasPhysicalProperty(name)) { + setPhysicalValue(name, value); + } + else if (hasAppearanceProperty(name)) { + setAppearanceValue(name, value); + } + else { + throw PropertyNotFound(); + } +} + +void Material::setValue(const QString& name, const QVariant& value) +{ + if (hasPhysicalProperty(name)) { + setPhysicalValue(name, value); + } + else { + throw PropertyNotFound(); + } +} + void Material::setLegacyValue(const QString& name, const QString& value) { setEditStateAlter(); diff --git a/src/Mod/Material/App/Materials.h b/src/Mod/Material/App/Materials.h index d0a7c6c777..33de6ae629 100644 --- a/src/Mod/Material/App/Materials.h +++ b/src/Mod/Material/App/Materials.h @@ -294,10 +294,15 @@ public: void setPhysicalValue(const QString& name, const Base::Quantity& value); void setPhysicalValue(const QString& name, const std::shared_ptr& value); void setPhysicalValue(const QString& name, const std::shared_ptr>& value); + void setPhysicalValue(const QString& name, const QVariant& value); void setAppearanceValue(const QString& name, const QString& value); void setAppearanceValue(const QString& name, const std::shared_ptr& value); void setAppearanceValue(const QString& name, const std::shared_ptr>& value); + void setAppearanceValue(const QString& name, const QVariant& value); + + void setValue(const QString& name, const QString& value); + void setValue(const QString& name, const QVariant& value); /* * Legacy values are thosed contained in old format files that don't fit in the new diff --git a/src/Mod/Material/Gui/BaseDelegate.cpp b/src/Mod/Material/Gui/BaseDelegate.cpp index 2052291dcf..66bc87c52e 100644 --- a/src/Mod/Material/Gui/BaseDelegate.cpp +++ b/src/Mod/Material/Gui/BaseDelegate.cpp @@ -43,7 +43,6 @@ #include #include #include -// #include #include #include @@ -331,8 +330,9 @@ void BaseDelegate::setEditorData(QWidget* editor, const QModelIndex& index) cons return; } if (type == Materials::MaterialValue::Quantity) { - auto input = dynamic_cast(editor); - input->setQuantityString(item.toString()); + auto input = dynamic_cast(editor); + // input->setQuantityString(item.toString()); + input->setValue(item.value()); return; } if (type == Materials::MaterialValue::List || type == Materials::MaterialValue::ImageList) { @@ -354,10 +354,11 @@ void BaseDelegate::setModelData(QWidget* editor, value = chooser->fileName(); } else if (type == Materials::MaterialValue::Quantity) { - auto input = dynamic_cast(editor); + auto input = dynamic_cast(editor); // value = input->text(); // return; - auto quantity = Base::Quantity::parse(input->text()); + // auto quantity = Base::Quantity::parse(input->text()); + auto quantity = input->value(); value = QVariant::fromValue(quantity); } else if (type == Materials::MaterialValue::Integer) { @@ -383,6 +384,7 @@ void BaseDelegate::setModelData(QWidget* editor, } setValue(model, index, value); + // Q_EMIT model->dataChanged(index, index); } QWidget* BaseDelegate::createEditor(QWidget* parent, diff --git a/src/Mod/Material/Gui/BaseDelegate.h b/src/Mod/Material/Gui/BaseDelegate.h index db29c57898..7d92d8af1a 100644 --- a/src/Mod/Material/Gui/BaseDelegate.h +++ b/src/Mod/Material/Gui/BaseDelegate.h @@ -91,7 +91,7 @@ protected: const QStyleOptionViewItem& option, const QModelIndex& index) const; - bool newRow(const QAbstractItemModel* model, const QModelIndex& index) const; + virtual bool newRow(const QAbstractItemModel* model, const QModelIndex& index) const; QWidget* createWidget(QWidget* parent, const QVariant& item, const QModelIndex& index) const; }; diff --git a/src/Mod/Material/Gui/ListDelegate.cpp b/src/Mod/Material/Gui/ListDelegate.cpp index 52f0ca6d2c..bcef1f752a 100644 --- a/src/Mod/Material/Gui/ListDelegate.cpp +++ b/src/Mod/Material/Gui/ListDelegate.cpp @@ -74,10 +74,12 @@ void ListDelegate::setValue(QAbstractItemModel* model, const QModelIndex& index, const QVariant& value) const { - auto matModel = dynamic_cast(model); - matModel->setData(index, value); + auto matModel = dynamic_cast(model); + if (matModel) { + matModel->setData(index, value); - notifyChanged(model, index); + notifyChanged(model, index); + } } void ListDelegate::notifyChanged(const QAbstractItemModel* model, const QModelIndex& index) const @@ -105,33 +107,4 @@ void ListDelegate::paint(QPainter* painter, QStyledItemDelegate::paint(painter, option, index); } -// bool ListDelegate::editorEvent(QEvent* event, -// QAbstractItemModel* model, -// const QStyleOptionViewItem& option, -// const QModelIndex& index) -// { -// if (event->type() == QEvent::MouseButtonDblClick) { -// auto treeModel = index.model(); - -// auto item = treeModel->data(index); - -// int row = index.row(); - -// QString propertyName = group->child(row, 0)->text(); -// QString propertyType = QString::fromStdString("String"); -// if (group->child(row, 2)) { -// propertyType = group->child(row, 2)->text(); -// } - -// std::string type = propertyType.toStdString(); -// if (_type == Materials::MaterialValue::Image || _type == -// Materials::MaterialValue::ImageList) { -// showImageModal(propertyName, item); -// // Mark as handled -// return true; -// } -// } -// return QStyledItemDelegate::editorEvent(event, model, option, index); -// } - #include "moc_ListDelegate.cpp" diff --git a/src/Mod/Material/Gui/ListEdit.cpp b/src/Mod/Material/Gui/ListEdit.cpp index b8df1722a7..a18022399b 100644 --- a/src/Mod/Material/Gui/ListEdit.cpp +++ b/src/Mod/Material/Gui/ListEdit.cpp @@ -70,6 +70,8 @@ ListEdit::ListEdit(const QString& propertyName, setupListView(); setDelegates(ui->listView); + ui->buttonDeleteRow->setVisible(false); + // connect(ui->buttonDeleteRow, &QPushButton::clicked, this, &ListEdit::onDelete); connect(ui->standardButtons, &QDialogButtonBox::accepted, this, &ListEdit::accept); connect(ui->standardButtons, &QDialogButtonBox::rejected, this, &ListEdit::reject); @@ -109,6 +111,7 @@ void ListEdit::onDataChanged(const QModelIndex& topLeft, Q_UNUSED(roles) _material->setEditStateAlter(); + update(); } bool ListEdit::newRow(const QModelIndex& index) @@ -178,18 +181,9 @@ void ListEdit::reject() void ListEdit::onSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected) { - Q_UNUSED(selected) Q_UNUSED(deselected) - // auto indexList = selected.indexes(); - // if (indexList.size() > 0) { - // auto index = indexList[0]; - // auto listModel = dynamic_cast(index.model()); - // if (listModel->newRow(index)) { - // Base::Console().Log("*** New Row ***\n"); - // const_cast(listModel)->insertRows(index.row(), 1); - // } - // } + ui->buttonDeleteRow->setEnabled(!selected.isEmpty()); } #include "moc_ListEdit.cpp" diff --git a/src/Mod/Material/Gui/ListEdit.ui b/src/Mod/Material/Gui/ListEdit.ui index 55b634faed..e0cd748d8d 100644 --- a/src/Mod/Material/Gui/ListEdit.ui +++ b/src/Mod/Material/Gui/ListEdit.ui @@ -17,6 +17,33 @@ + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + false + + + Delete Row + + + + + diff --git a/src/Mod/Material/Gui/ListModel.cpp b/src/Mod/Material/Gui/ListModel.cpp index e89a6dcadc..19a065c389 100644 --- a/src/Mod/Material/Gui/ListModel.cpp +++ b/src/Mod/Material/Gui/ListModel.cpp @@ -89,7 +89,7 @@ bool ListModel::setData(const QModelIndex& index, const QVariant& value, int rol { Q_UNUSED(role); - if (index.row() == _valuePtr->size()) { + if (newRow(index)) { insertRows(index.row(), 1); } (*_valuePtr)[index.row()] = value; @@ -127,5 +127,7 @@ bool ListModel::removeRows(int row, int count, const QModelIndex& parent) _valuePtr->removeAt(row); } + endRemoveRows(); + return true; } diff --git a/src/Mod/Material/Gui/MaterialDelegate.cpp b/src/Mod/Material/Gui/MaterialDelegate.cpp index c095933fb9..b3e2788001 100644 --- a/src/Mod/Material/Gui/MaterialDelegate.cpp +++ b/src/Mod/Material/Gui/MaterialDelegate.cpp @@ -62,6 +62,15 @@ MaterialDelegate::MaterialDelegate(QObject* parent) : BaseDelegate(parent) {} +bool MaterialDelegate::newRow(const QAbstractItemModel* model, const QModelIndex& index) const +{ + Q_UNUSED(model) + Q_UNUSED(index) + + // New rows are for lists and arrays + return false; +} + Materials::MaterialValue::ValueType MaterialDelegate::getType(const QModelIndex& index) const { auto treeModel = dynamic_cast(index.model()); @@ -131,7 +140,6 @@ void MaterialDelegate::setValue(QAbstractItemModel* model, int row = index.row(); if (group->child(row, 1)) { auto material = group->child(row, 1)->data().value>(); - // auto propertyName = group->child(row, 0)->text(); auto propertyName = group->child(row, 0)->data().toString(); std::string _name = propertyName.toStdString(); auto property = material->getProperty(propertyName); @@ -145,7 +153,6 @@ void MaterialDelegate::setValue(QAbstractItemModel* model, void MaterialDelegate::notifyChanged(const QAbstractItemModel* model, const QModelIndex& index) const { - Base::Console().Log("MaterialDelegate::notifyChanged()\n"); auto treeModel = dynamic_cast(model); auto item = treeModel->itemFromIndex(index); auto group = item->parent(); @@ -160,10 +167,8 @@ void MaterialDelegate::notifyChanged(const QAbstractItemModel* model, auto propertyName = group->child(row, 0)->data().toString(); auto propertyValue = material->getProperty(propertyName)->getValue(); material->setEditStateAlter(); - Base::Console().Log("MaterialDelegate::notifyChanged() - marked altered\n"); - Q_EMIT const_cast(this)->propertyChange(propertyName, - propertyValue.toString()); + Q_EMIT const_cast(this)->propertyChange(propertyName, propertyValue); } } @@ -447,11 +452,12 @@ QWidget* MaterialDelegate::createWidget(QWidget* parent, widget = combo; } else if (type == Materials::MaterialValue::Quantity) { - auto input = new Gui::InputField(parent); + // auto input = new Gui::InputField(parent); + auto input = new Gui::QuantitySpinBox(parent); input->setMinimum(std::numeric_limits::min()); input->setMaximum(std::numeric_limits::max()); input->setUnitText(getUnits(index)); - input->setPrecision(6); + // input->setPrecision(6); input->setValue(item.value()); widget = input; diff --git a/src/Mod/Material/Gui/MaterialDelegate.h b/src/Mod/Material/Gui/MaterialDelegate.h index 9a37cfc091..e373c73855 100644 --- a/src/Mod/Material/Gui/MaterialDelegate.h +++ b/src/Mod/Material/Gui/MaterialDelegate.h @@ -70,10 +70,11 @@ protected: const QModelIndex& index, const QVariant& value) const override; void notifyChanged(const QAbstractItemModel* model, const QModelIndex& index) const override; + bool newRow(const QAbstractItemModel* model, const QModelIndex& index) const override; Q_SIGNALS: /** Emits this signal when a property has changed */ - void propertyChange(const QString& property, const QString value); + void propertyChange(const QString& property, const QVariant& value); private: QWidget* createWidget(QWidget* parent, const QVariant& item, const QModelIndex& index) const; diff --git a/src/Mod/Material/Gui/MaterialsEditor.cpp b/src/Mod/Material/Gui/MaterialsEditor.cpp index 2e77c6eba6..e33630d84b 100644 --- a/src/Mod/Material/Gui/MaterialsEditor.cpp +++ b/src/Mod/Material/Gui/MaterialsEditor.cpp @@ -355,7 +355,7 @@ void MaterialsEditor::onDescription() _material->setDescription(ui->editDescription->toPlainText()); } -void MaterialsEditor::propertyChange(const QString& property, const QString value) +void MaterialsEditor::propertyChange(const QString& property, const QVariant& value) { if (_material->hasPhysicalProperty(property)) { _material->setPhysicalValue(property, value); diff --git a/src/Mod/Material/Gui/MaterialsEditor.h b/src/Mod/Material/Gui/MaterialsEditor.h index 05ce0cd8d4..2378625a6b 100644 --- a/src/Mod/Material/Gui/MaterialsEditor.h +++ b/src/Mod/Material/Gui/MaterialsEditor.h @@ -65,7 +65,7 @@ public: void onSourceReference(const QString& text); void onDescription(); - void propertyChange(const QString& property, const QString value); + void propertyChange(const QString& property, const QVariant& value); void onInheritNewMaterial(bool checked); void onNewMaterial(bool checked); void onFavourite(bool checked);