can I make a temporary, local copy of a property?

mzimmers

Hi all -

My app uses several list models (implemented in C++). I use QML to display data, and allow users to make (really, request) changes.

I've designed the app so that it never actually changes any model data; it merely sends requests to a server, and upon receiving a response, makes the appropriate updates to the model data. This works well in most cases, but I have a problem with a screen where the user can edit a bunch of property values, then send the request.

What I really need is a local copy of the element in the model that the user's editing. The rub is that all my model elements are derived from QObject (needed for the property signals), and I know I'm not supposed to try to copy QObjects.

I can think of a couple hacky ways to accomplish this, but I thought that I'd first ask if there's a preferred way to accomplish this.

Thanks for any suggestions...

EDIT:

Here's one hacky way to do it:

ColumnLayout {
    id: equipmentEdit
    property Equipment equipment
    property Equipment equipmentCopy: equipmentProxyModel.getEquipment(-1) // creates a new Equipment

    Component.onCompleted: {
        equipmentCopy.uuid = equipment.uuid
        equipmentCopy.name = equipment.name
        equipmentCopy.category = equipment.category
        ...

Please someone tell me there's a better way to do this...

GrecKo

@mzimmers said in can I make a temporary, local copy of a property?:

So, according to the doc you cited, am I correct that this will have C++ ownership?

yes.

@mzimmers said in can I make a temporary, local copy of a property?:

So if I modify my function like this:

?
QJSEngine::setObjectOwnership(equipment.get(), QJSEngine::JavaScriptOwnership);
You don't want to do that for an object you manage, you want to keep the ownership on the c++ side for an object on your list, only give it to QML for the cloned object.

What I would do if I were you (since the dangling pointer issue with your temporary shared_ptr is still present in your code):
No need to return a dummy object in getEquipment:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{
   const auto i {getIndex(uuid)};
    if (i == NgaUI::NOT_IN_LIST) {
        return nullptr;
    }
    return m_list.at(i).get();
}

Or if for some obscure reason you don't like guard clauses and want multiple returns:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{   
   Equipment* equipment = nullptr;
   const auto i {getIndex(uuid)};
    if (i != NgaUI::NOT_IN_LIST) {
        equipment = m_list.at(i).get();
    }
    return equipment.
}

Have a cloneequipment function returning a new parent-less object (here or as member function in Equipment).

Equipment* Fn::cloneEquipment(Equipment* sourceEquipment) {
   Equipment* clone = new Equipment();
   // copy from source to clone
   return clone; 
}

And use it like so in QML:

property Equipment equipment
property Equipment equipmentCopy: Fn.cloneEquipment(equipment)

dheerendra

@mzimmers

There is no other way to do this. You need to have copy of the object with you. Either this object is created in QML or C++ side. You need to update this object & send the object to c++ side either as Q_PROPERTY or argument to server object method.

mzimmers

@dheerendra OK, thanks for letting me know.

As far as implementation, I'm thinking of putting this functionality into my C++ code. But, if I create a makeCopy() function in my model, and invoke it from JS/QML, where do I need to perform the garbage collection?

dheerendra

@mzimmers

If you have have created object in C++ & getting in qml, set the object ownership to cpp using QQMLEngine::setObjectOwnership.

GrecKo

But here the thing to do is let the QML engine have the ownership of the clone object, so it can delete it when it is not used anymore. This will leak otherwise.

mzimmers

@GrecKo said in can I make a temporary, local copy of a property?:

But here the thing to do is let the QML engine have the ownership of the clone object

So, in my example, who owns equipmentCopy? The creation stems from QML, but the actual creation occurs in C++, so I'm not sure how this gets sorted out.

GrecKo

@mzimmers
as stated here https://doc.qt.io/qt-6/qtqml-cppintegration-data.html#data-ownership if you return an object from a Q_INVOKABLE function without a QObject parent and an explicit call to QQmlEngine::setObjectOwnership, the QML engine will own it. That's what you want for the clone, not for the real one though.

mzimmers

@GrecKo if you please, I'd like to be really clear on this. Here's my Q_INVOKABLE function:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{
    auto equipment {std::make_shared<Equipment>(this)};
    const auto i {getIndex(uuid)};
    if (i != NgaUI::NOT_IN_LIST) {
        equipment = m_list.at(i);
    }
    return equipment.get();
}

So, this object is indeed parented, correct? Are you saying that this is NOT what I want for my clone?

EDIT: The function above is used in two places: directly in my QML, and also by this function:

Equipment* EquipmentProxyModel::getEquipment(int proxyIndex)
{
    Equipment *equipment { nullptr };
    const auto source {qobject_cast<EquipmentModel *>(sourceModel())};
    if (source != nullptr)
    {
        auto sourceIndex {mapToSource(index(proxyIndex, 0))};
        if (sourceIndex.isValid())
        {
            auto uuid {source->data(sourceIndex, EquipmentModel::EquipmentRoles::UuidRole).toUuid()};
            equipment = source->getEquipment(uuid);
        }
    }
    return equipment == nullptr ? new Equipment(this) : equipment;
}

This function too is called only from QML.

GrecKo

@mzimmers I don't know if it is parented but I assume it is if the Equipment constructor pass this to the QObject constructor.
I also don't know how the equipments in your list are constructed.

Your getEquipment(const QUuid &uuid) is buggy.
When the uuid is not found you return a default constructed Equipment that will get deleted since the shared_ptr will go out of scope.

returning nullptr is fine, what's the point of return equipment == nullptr ? new Equipment(this) : equipment; ? what will you do with a default constructed Equipment?

mzimmers

@GrecKo this is my Equipment c'tor (nothing unusual):

Equipment::Equipment(QObject *parent) : QObject(parent) {}

So, according to the doc you cited, am I correct that this will have C++ ownership?

@GrecKo said in can I make a temporary, local copy of a property?:

what will you do with a default constructed Equipment

I do this in my QML:

property Equipment equipment
property Equipment equipmentCopy: equipmentProxyModel !== null ? equipmentProxyModel.getEquipment(-1) : null

Component.onCompleted: {
    Fn.copyEquipment(equipmentCopy, equipment)
}

So if I modify my function like this:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{
    auto equipment {std::make_shared<Equipment>(this)};
    const auto i {getIndex(uuid)};
    if (i != NgaUI::NOT_IN_LIST) {
        equipment = m_list.at(i);
        QJSEngine::setObjectOwnership(equipment.get(), QJSEngine::JavaScriptOwnership);
    }
    return equipment.get();
}

Does this pass ownership of this Equipment object to the QML? This would seem to be the desired behavior.

GrecKo

@mzimmers said in can I make a temporary, local copy of a property?:

So, according to the doc you cited, am I correct that this will have C++ ownership?

yes.

@mzimmers said in can I make a temporary, local copy of a property?:

So if I modify my function like this:

?
QJSEngine::setObjectOwnership(equipment.get(), QJSEngine::JavaScriptOwnership);
You don't want to do that for an object you manage, you want to keep the ownership on the c++ side for an object on your list, only give it to QML for the cloned object.

What I would do if I were you (since the dangling pointer issue with your temporary shared_ptr is still present in your code):
No need to return a dummy object in getEquipment:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{
   const auto i {getIndex(uuid)};
    if (i == NgaUI::NOT_IN_LIST) {
        return nullptr;
    }
    return m_list.at(i).get();
}

Or if for some obscure reason you don't like guard clauses and want multiple returns:

Equipment *EquipmentModel::getEquipment(const QUuid &uuid)
{   
   Equipment* equipment = nullptr;
   const auto i {getIndex(uuid)};
    if (i != NgaUI::NOT_IN_LIST) {
        equipment = m_list.at(i).get();
    }
    return equipment.
}

Have a cloneequipment function returning a new parent-less object (here or as member function in Equipment).

Equipment* Fn::cloneEquipment(Equipment* sourceEquipment) {
   Equipment* clone = new Equipment();
   // copy from source to clone
   return clone; 
}

And use it like so in QML:

property Equipment equipment
property Equipment equipmentCopy: Fn.cloneEquipment(equipment)

mzimmers

@GrecKo said in can I make a temporary, local copy of a property?:

You don't want to do that for an object you manage, you want to keep the ownership on the c++ side for an object on your list

I should have mentioned that the functions getEquipment(), both in the model and the proxy model, are used only for making a (temporary) clone copy. This object never enters the list; it only serves as a template for creating a HTTP PATCH request. When I receive a reply to this request, I update the correct element in the model list.