Search code examples
qtqwidgetqtablewidget

QTableWidget cell widgets dissapear after swap

I have QTableWidget with 3 columns. First column contains QTableWidgetItem with row name, two other columns contains two cell widgets.

enter image description here

I tried to swap table rows with the following code:

connect(upButton, &QPushButton::clicked, this, [this, protectionWidget]()
    {
        int index = m_protectionWidgets->indexOf(protectionWidget);
        int upperIndex = index - 1;
        if (upperIndex >= 0)
        {
            QTableWidgetItem *nameItem = m_protectionsTable->takeItem(index, 0);
            ProtectionWidget *protectionWidgetCopy = protectionWidget;
            QWidget *buttons = m_protectionsTable->cellWidget(index, 2);

            QTableWidgetItem *upperNameItem = m_protectionsTable->takeItem(upperIndex, 0);
            ProtectionWidget *upperProtectionWidget = m_protectionWidgets->at(upperIndex);
            QWidget *upperButtons = m_protectionsTable->cellWidget(upperIndex, 2);

            m_protectionsTable->setItem(upperIndex, 0, nameItem);
            m_protectionsTable->setCellWidget(upperIndex, 1, protectionWidgetCopy);
            m_protectionsTable->setCellWidget(upperIndex, 2, buttons);

            m_protectionsTable->setItem(index, 0, upperNameItem);
            m_protectionsTable->setCellWidget(index, 1, upperProtectionWidget);
            m_protectionsTable->setCellWidget(index, 2, upperButtons);

            m_protectionWidgets->removeAt(index);
            m_protectionWidgets->insert(upperIndex, 1, protectionWidget);
        }
    });

But when I click upButton cellWidgets disappear.

enter image description here

What's wrong?

Solution

  • What is happening to you, documented for setCellWidget is that you cannot set a widget on a cell without deleting the widget already present. It is a rather tricky thing to detect with debugging tools because the deletion is done on the next execution of the event loop thanks to a call to QObject::deleteLater, long after your lambda has already finished executing.

    There is no way around the deletion taking place, however there are ways to make the deletion harmless. I see 2 ways:

    1. Place your protectionWidget and buttons inside sacrifical widgets. by resetting their parents at the right time, you can achieve an effect where only empty sacrifical widgets get deleted during the next calls to setCellWidget.
    2. Same as the above but you stop calling setCellWidget completely and instead just swaps the parent of the widgets.

    The below code demonstrates your mistake (A & B), the first method (C & D) and the 2nd method (E & F). Note that you need to consider whether to keep the lambdas createWidget, extractButton and/or swapParent as is or turn them into protected/private methods.

    int main(int argc, char* argv[])
{
    QApplication app(argc, argv);

    QTableWidget t;
    t.setRowCount(6);
    t.setColumnCount(2);

    t.setItem(0, 0, new QTableWidgetItem("Original: A"));
    t.setItem(1, 0, new QTableWidgetItem("Original: B"));
    t.setItem(2, 0, new QTableWidgetItem("Method 1: C"));
    t.setItem(3, 0, new QTableWidgetItem("Method 1: D"));
    t.setItem(4, 0, new QTableWidgetItem("Method 2: E"));
    t.setItem(5, 0, new QTableWidgetItem("Method 2: F"));

    auto createWidget = [](QPushButton* button) -> QWidget*
    {
        QWidget* result = new QWidget();
        QLayout* l = new QVBoxLayout(result);
        l->setContentsMargins(QMargins());
        l->addWidget(button);
        button->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
        return result;
    };
    auto extractButton = [] (QWidget* widget) -> QPushButton*
    {
        QPushButton* result = widget->findChild<QPushButton*>();
        result->setParent(nullptr);
        return result;
    };
    auto swapParent = [](QWidget* from, QWidget* to) -> void
    {
        auto fromChild = from->findChild<QWidget*>(), toChild = to->findChild<QWidget*>();
        from->layout()->removeWidget(fromChild);
        to->layout()->removeWidget(toChild);
        fromChild->setParent(to);
        toChild->setParent(from);
        from->layout()->addWidget(toChild);
        to->layout()->addWidget(fromChild);
    };

    auto bA = new QPushButton("A"), bB = new QPushButton("B"),
         bC = new QPushButton("C"), bD = new QPushButton("D"),
         bE = new QPushButton("E"), bF = new QPushButton("F");
    QObject::connect(bA, &QObject::destroyed, []() { qDebug() << "Button A destroyed "; });
    QObject::connect(bB, &QObject::destroyed, []() { qDebug() << "Button B destroyed "; });
    QObject::connect(bC, &QObject::destroyed, []() { qDebug() << "Button C destroyed "; });
    QObject::connect(bD, &QObject::destroyed, []() { qDebug() << "Button D destroyed "; });
    QObject::connect(bE, &QObject::destroyed, []() { qDebug() << "Button E destroyed "; });
    QObject::connect(bF, &QObject::destroyed, []() { qDebug() << "Button F destroyed "; });

    t.setCellWidget(0, 1, bA);
    t.setCellWidget(1, 1, bB);
    t.setCellWidget(2, 1, createWidget(bC));
    t.setCellWidget(3, 1, createWidget(bD));
    t.setCellWidget(4, 1, createWidget(bE));
    t.setCellWidget(5, 1, createWidget(bF));
    //End of the initialization.
    //The next lines simulate the lambdas connected to the button signals with:
    // - what not do to: swap A<->B, unprotected
    // - method 1: swap C<->D, each protected by a sacrifical widget
    // - method 2: reparent E and F instead of literally swapping them
    auto b0 = t.cellWidget(0, 1), b1 = t.cellWidget(1, 1),
         b2 = t.cellWidget(2, 1), b3 = t.cellWidget(3, 1),
         b4 = t.cellWidget(4, 1), b5 = t.cellWidget(5, 1);
    //b0 == bA && b1 == bB && b2 == bC && b3 == bD && b4 == bE && b5 == bF

    //Incorrect for A and B (see the qDebug messages)
    t.setCellWidget(0, 1, b1);
    t.setCellWidget(1, 1, b0);

    //Method 1
    t.setCellWidget(2, 1, createWidget(extractButton(b3)));
    t.setCellWidget(3, 1, createWidget(extractButton(b2)));
    //Method 2
    swapParent(b4, b5);

    // The next 2 lines are only to exit cleanly.
    t.setCellWidget(0, 1, nullptr);
    t.setCellWidget(1, 1, nullptr);

    t.show();
    return app.exec();
}

    It should go without saying but this demonstration seriously need a lot of pointer checks.