Changing UI file between 2 save/restore could lead to failing save due to minSize() bigger than size()

[liszto]

Environment

• OS: Windows
• Qt Version: 5.15.2
• KDDockWidgets version: 1.6
• KDDockWidgets QtQuick or QtWidgets ? QtWidgets

Self reproducible example

As it implies at least 2 files manipulation. To have a repro.
I will first explain the bug (and the fix I found).

In our Qt app a user saved its layout with 3 panes (A,B,C) only A will interest us.

• On its first layout save everything is fine (save & restore both works)

Now a developer changed the .ui file associated to the pane A and will add more widget inside. This will lead to a size change and so the minSize will changed.
In our case the old minSize() heigh was 512px and the one is 576px. If the previous layout save the pane with its minimum possible height it will be 512px for the size() and minSize() of the item.
Following this change the user will retrieve developer modification and load its layout.

• Visually everything seems to be fine but in fact it's not. When he will try to save it, it will fail because the "minSize()" will be higher than the "size()" in the checkSanity method from Item.cpp at this specific moment:

if (minSize().width() > width() || minSize().height() > height()) {
    root()->dumpLayout();
    qWarning() << Q_FUNC_INFO << "Size constraints not honoured" << this
               << "; min=" << minSize() << "; size=" << size();
    return false;
}

to fix that we changed this code from Item.cpp :

void Item::setMinSize(QSize sz)
{
    if (sz != m_sizingInfo.minSize) {
        m_sizingInfo.minSize = sz;
        Q_EMIT minSizeChanged(this);
        if (!m_isSettingGuest)
            setSize_recursive(size().expandedTo(sz));
    }
}

to this :

void Item::setMinSize(QSize sz)
{
    if (sz != m_sizingInfo.minSize) {
        m_sizingInfo.minSize = sz;
        Q_EMIT minSizeChanged(this);
        if (!m_isSettingGuest || minSize().width() > width() || minSize().height() > height() )
            setSize_recursive(size().expandedTo(sz));
    }
}

Is this something legit ? Or do we have to handle this in another way ?
For us it's a good place to handle this, because in the case the UI will change externally to KDDockWidget and its serialization file, the deserialization need to handle that kind of thing to avoid any issue at user level during save/restore step.

NB : I see the code is still the same in the last KDDockWidgets code base (2.X) so the "issue" is still there too.

[iamsergio]

Hi Liszto, remind me, are you a commercial customer, or using the GPL version in an opensource project ?

[liszto]

Hello !
Sorry for the late answer, we use a GPL version in our project and you are the one who integrated it in our company codebase :D
It's only an internal project for internal use (so not opensource)

[iamsergio]

I'm almost certain this was fixed in the 2.x series, even though you see similar code there.

The best way forward would be to get me a minimal-repro (patch over examples/ main.cpp or so), but for version 2.3 (current main)

[liszto]

As soon as I can (maybe not in a near future) I'll attempt to repro that on that version