From ff14e6fe086731813db336a6b4db18285ebad8b9 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Thu, 6 Mar 2025 21:20:11 +0100 Subject: [PATCH] QGIM: implement itemData/setItemData for gadgets If the item at index is a gadget (or QObject type), then use the properties that match the names of the roles to get and set the item data. This is transactional for gadgets, where we can modify a copy until all entries have been successfully written, and then swap the copy and the original value if all writes were successful. It cannot be transactional for QObject instances. Change-Id: I514853cbd67aaf6e86c100e815c8af579b8278c2 Reviewed-by: Artem Dyomin --- src/corelib/itemmodels/qgenericitemmodel.cpp | 22 ++++-- src/corelib/itemmodels/qgenericitemmodel.h | 68 +++++++++++++++++++ .../tst_qgenericitemmodel.cpp | 39 +++++++---- 3 files changed, 110 insertions(+), 19 deletions(-) diff --git a/src/corelib/itemmodels/qgenericitemmodel.cpp b/src/corelib/itemmodels/qgenericitemmodel.cpp index 994d0e66fb3..0408f6dea55 100644 --- a/src/corelib/itemmodels/qgenericitemmodel.cpp +++ b/src/corelib/itemmodels/qgenericitemmodel.cpp @@ -358,6 +358,12 @@ bool QGenericItemModel::setData(const QModelIndex &index, const QVariant &data, from either \c{int}, Qt::ItemDataRole, or QString to a QVariant, then the data from that container is returned. + If the item type is a gadget or QObject subclass, then the values of those + properties that match a \l{roleNames()}{role name} are returned. + + If the item is not an associative container, gadget, or QObject subclass, + then this calls the base class implementation. + \sa setItemData(), Qt::ItemDataRole, data() */ QMap QGenericItemModel::itemData(const QModelIndex &index) const @@ -374,14 +380,20 @@ QMap QGenericItemModel::itemData(const QModelIndex &index) const QString to QVariant, then only those values in \a data are stored for which there is a mapping in the \l{roleNames()}{role names} table. + If the item type is a gadget or QObject subclass, then those properties that + match a \l{roleNames()}{role name} are set to the corresponding value in + \a data. + Roles for which there is no entry in \a data are not modified. - This implementation is transactional, and returns true if all the entries - from \a data could be stored. If any entry could not be updated, then the - original container is not modified at all, and the function returns false. + For item types that can be copied, this implementation is transactional, + and returns true if all the entries from \a data could be stored. If any + entry could not be updated, then the original container is not modified at + all, and the function returns false. - If the item is not an associative container, then this calls the base class - implementation, which calls setData() for each entry in \a data. + If the item is not an associative container, gadget, or QObject subclass, + then this calls the base class implementation, which calls setData() for + each entry in \a data. \sa itemData(), setData(), Qt::ItemDataRole */ diff --git a/src/corelib/itemmodels/qgenericitemmodel.h b/src/corelib/itemmodels/qgenericitemmodel.h index 11856f3a838..3c6e2aeb1c8 100644 --- a/src/corelib/itemmodels/qgenericitemmodel.h +++ b/src/corelib/itemmodels/qgenericitemmodel.h @@ -347,6 +347,28 @@ public: result.insert(role, QGenericItemModelDetails::value(it)); } } + } else if constexpr (has_metaobject) { + if (row_traits::fixed_size() <= 1) { + tried = true; + using meta_type = std::remove_pointer_t; + const QMetaObject &mo = meta_type::staticMetaObject; + for (auto &&[role, roleName] : roleNames().asKeyValueRange()) { + QVariant data; + if constexpr (std::is_base_of_v) { + if (value) + data = value->property(roleName); + } else { + const int pi = mo.indexOfProperty(roleName.constData()); + if (pi >= 0) { + const QMetaProperty prop = mo.property(pi); + if (prop.isValid()) + data = prop.readOnGadget(QGenericItemModelDetails::pointerTo(value)); + } + } + if (data.isValid()) + result[role] = data; + } + } } }; @@ -477,6 +499,52 @@ public: target[QString::fromUtf8(roleName(role))] = value; } return true; + } else if constexpr (has_metaobject) { + if (row_traits::fixed_size() <= 1) { + tried = true; + using meta_type = std::remove_pointer_t; + const QMetaObject &mo = meta_type::staticMetaObject; + // transactional: if possible, modify a copy and only + // update target if all values from data could be stored. + auto targetCopy = [](auto &&origin) { + if constexpr (std::is_base_of_v) + return origin; // can't copy, no transaction support + else if constexpr (std::is_pointer_v) + return *origin; + else if constexpr (std::is_copy_assignable_v) + return origin; + else // can't copy - targetCopy is now a pointer + return &origin; + }(target); + for (auto &&[role, value] : data.asKeyValueRange()) { + const QByteArray roleName = roleNames().value(role); + bool written = false; + if constexpr (std::is_base_of_v) { + if (targetCopy) + written = targetCopy->setProperty(roleName, value); + } else { + const int pi = mo.indexOfProperty(roleName.constData()); + if (pi >= 0) { + const QMetaProperty prop = mo.property(pi); + if (prop.isValid()) + written = prop.writeOnGadget(QGenericItemModelDetails::pointerTo(targetCopy), value); + } + } + if (!written) { + qWarning("Failed to write value for %s", roleName.data()); + return false; + } + } + if constexpr (std::is_base_of_v) + target = targetCopy; // nothing actually copied + else if constexpr (std::is_pointer_v) + qSwap(*target, targetCopy); + else if constexpr (std::is_pointer_v) + ; // couldn't copy + else + qSwap(target, targetCopy); + return true; + } } return false; }; diff --git a/tests/auto/corelib/itemmodels/qgenericitemmodel/tst_qgenericitemmodel.cpp b/tests/auto/corelib/itemmodels/qgenericitemmodel/tst_qgenericitemmodel.cpp index fbf980d6989..64c06d2232d 100644 --- a/tests/auto/corelib/itemmodels/qgenericitemmodel/tst_qgenericitemmodel.cpp +++ b/tests/auto/corelib/itemmodels/qgenericitemmodel/tst_qgenericitemmodel.cpp @@ -333,7 +333,7 @@ void tst_QGenericItemModel::createTestData() << 1 << ChangeActions(ChangeAction::SetData); ADD_POINTER(cArrayFixedColumns) - << int(std::tuple_size_v) << ChangeActions(ChangeAction::SetData); + << int(std::tuple_size_v) << (ChangeAction::SetData | ChangeAction::SetItemData); ADD_COPY(vectorOfFixedColumns) << 2 << (ChangeAction::ChangeRows | ChangeAction::SetData); @@ -344,22 +344,24 @@ void tst_QGenericItemModel::createTestData() ADD_POINTER(vectorOfArrays) << 10 << (ChangeAction::ChangeRows | ChangeAction::SetData); ADD_COPY(vectorOfStructs) - << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData); + << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData + | ChangeAction::SetItemData); ADD_POINTER(vectorOfStructs) - << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData); + << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData + | ChangeAction::SetItemData); ADD_COPY(vectorOfConstStructs) << int(std::tuple_size_v) << ChangeActions(ChangeAction::ChangeRows); ADD_POINTER(vectorOfConstStructs) << int(std::tuple_size_v) << ChangeActions(ChangeAction::ChangeRows); ADD_COPY(vectorOfGadgets) - << 3 << (ChangeAction::ChangeRows | ChangeAction::SetData); + << 3 << (ChangeAction::ChangeRows | ChangeAction::SetData | ChangeAction::SetItemData); ADD_POINTER(vectorOfGadgets) - << 3 << (ChangeAction::ChangeRows | ChangeAction::SetData); + << 3 << (ChangeAction::ChangeRows | ChangeAction::SetData | ChangeAction::SetItemData); ADD_COPY(listOfGadgets) - << 1 << (ChangeAction::ChangeRows | ChangeAction::SetData); + << 1 << (ChangeAction::ChangeRows | ChangeAction::SetData | ChangeAction::SetItemData); ADD_POINTER(listOfGadgets) - << 1 << (ChangeAction::ChangeRows | ChangeAction::SetData); + << 1 << (ChangeAction::ChangeRows | ChangeAction::SetData | ChangeAction::SetItemData); ADD_COPY(listOfObjects) << 2 << (ChangeAction::ChangeRows | ChangeAction::SetData); @@ -369,9 +371,10 @@ void tst_QGenericItemModel::createTestData() << 5 << ChangeActions(ChangeAction::All); // only adding as pointer, copy would operate on the same data ADD_POINTER(tableOfPointers) - << 2 << ChangeActions(ChangeAction::All); + << 2 << ChangeActions(ChangeAction::All | ChangeAction::SetItemData); ADD_POINTER(tableOfRowPointers) - << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData); + << int(std::tuple_size_v) << (ChangeAction::ChangeRows | ChangeAction::SetData + | ChangeAction::SetItemData); ADD_COPY(arrayOfConstNumbers) << 1 << ChangeActions(ChangeAction::ReadOnly); @@ -577,8 +580,11 @@ void tst_QGenericItemModel::itemData() const QModelIndex index = model->index(0, 0); const QMap itemData = model->itemData(index); - for (int role = 0; role < Qt::UserRole; ++role) + for (int role = 0; role < Qt::UserRole; ++role) { + if (role == Qt::EditRole) // we fake that in data() + continue; QCOMPARE(itemData.value(role), index.data(role)); + } } void tst_QGenericItemModel::setItemData() @@ -592,14 +598,16 @@ void tst_QGenericItemModel::setItemData() const QModelIndex index = model->index(0, 0); QMap itemData = model->itemData(index); // we only care about multi-role models - if (itemData.keys() == QList{Qt::DisplayRole, Qt::EditRole}) + const auto roles = itemData.keys(); + if (roles == QList{Qt::DisplayRole, Qt::EditRole}) QSKIP("Can't test setItemData on models with single values!"); itemData = {}; - - const auto roles = model->roleNames().keys(); for (int role : roles) { - QVariant data = QStringLiteral("Role %1").arg(role); + if (role == Qt::EditRole) // faked + continue; + QVariant data = role != Qt::DecorationRole ? QVariant(QStringLiteral("Role %1").arg(role)) + : QVariant(QColor(Qt::magenta)); itemData.insert(role, data); } @@ -621,6 +629,9 @@ void tst_QGenericItemModel::setItemData() } for (int role = 0; role < Qt::UserRole; ++role) { + if (role == Qt::EditRole) // faked role + continue; + QVariant data = index.data(role); auto diagnostics = qScopeGuard([&]{ qDebug() << "Mismatch for" << Qt::ItemDataRole(role);