[Development] Behavior-changing bugfixes in patch-level releases
EXT Mitch Curtis
mitch.curtis at qt.io
Thu Jul 13 07:37:04 CEST 2023
> -----Original Message-----
> From: Arno Rehn <a.rehn at menlosystems.com>
> Sent: Wednesday, July 12, 2023 4:13 PM
> To: EXT Mitch Curtis <mitch.curtis at qt.io>; Qt development mailing list
> <development at qt-project.org>
> Subject: Re: [Development] Behavior-changing bugfixes in patch-level releases
>
> Hi Mitch,
>
> On 12.07.2023 09:52, EXT Mitch Curtis wrote:
> >> Context: We have been hit by
> >> https://codereview.qt-project.org/c/qt/qtdeclarative/+/472596
> >> (which is even marked as "Important Behavior Change") ending up only
> >> in 6.5.1. It was quite the headache figuring out that 6.5.0 ->
> >> 6.5.1 has broken part of our ListViews.
> >
> > I'm sorry to hear that.
> >
> > As the developer who approved the change, I've left my thoughts
> > here:
> >
> > https://bugreports.qt.io/browse/QTBUG-
> 114166?focusedId=734562&page=com
> > .atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-
> > 734562
> >
> > In summary, I believe the official branch policy was followed here.
> >
> > I'm not sure if your use case is the same as the snippet in the bug
> > report, so it would be useful if you could share it so that we can see
> > if there's a legitimate usage that we've broken. That's not to say
> > that it's good to introduce regressions, of course, but often it's
> > hard to know how users are using APIs and we often only find out after
> > the change has been merged, because auto tests can't cover every use
> > case for the same reason.
>
> We have a vertical ListView which shows icon-only buttons. The buttons all
> have the same width, but depending on the icon size and/or the style, that
> width may differ. So we update the contentWidth of the ListView as soon as
> the first button is loaded. Something like this:
>
>
> ListView {
> id: sidebar
> Layout.fillHeight: true
> implicitWidth: contentWidth
>
> model: // ...
>
> delegate: IconButton {
> icon.name: // ...
> onImplicitWidthChanged: if (sidebar.contentWidth < 0) {
> sidebar.contentWidth = implicitWidth
> }
> }
> }
>
> Maybe we're doing this wrong; but I didn't find any other viable way to have a
> ListView auto-determine the size in the non-scrollable dimension based on its
> content.
I'd recommend setting a size on the ListView that doesn't depend on the implicit size of the delegates; one that is calculated with TextMetrics, for example. The reason for this is that it's not always possible to know the size of every delegate at startup. While scrolling a view, delegates that are larger or smaller might be loaded.
To use a more recent example where this problem had to be solved, https://doc.qt.io/qt-6/qml-qtquick-tableview.html#row-heights-and-column-widths says:
"When a new column is flicked into view, TableView will determine its width by calling the columnWidthProvider. If set, this function will alone decide the width of the column. Otherwise, it will check if an explicit width has been set with setColumnWidth(). If not, implicitColumnWidth() will be used. The implicit width of a column is the same as the largest implicit width found among the currently loaded delegate items in that column. Trying to set an explicit width directly on a delegate has no effect, and will be ignored and overwritten. The same logic also applies to row heights."
Note the part that says:
"The implicit width of a column is the same as the largest implicit width found among the currently loaded delegate items in that column."
There's also a note below:
"Note: The resolved width of a column is discarded when the whole column is flicked out of the view, and is recalculated again if it's flicked back in. This means that if the width depends on the implicitColumnWidth(), the calculation can be different each time, depending on which row you're at when the column enters (since implicitColumnWidth() only considers the delegate items that are currently loaded). To avoid this, you should use a columnWidthProvider, or ensure that all the delegate items in the same column have the same implicitWidth."
This is also described in e.g. https://bugreports.qt.io/browse/QTBUG-76830, and documented in https://doc-snapshots.qt.io/qt6-dev/qml-qtquick-listview.html#variable-delegate-size-and-section-labels. The cacheBuffer workaround mentioned there may help but is not a complete solution.
From testing locally with a runnable adaptation of your example, I get a ListView whose width is 12:
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
ApplicationWindow {
visible: true
width: 800
height: 600
title: "width: " + sidebar.width
ColumnLayout {
width: 200
height: parent.height
Label {
text: "Some extra stuff here"
}
ListView {
id: sidebar
implicitWidth: contentWidth
// clip: true
Layout.fillHeight: true
model: 100
delegate: ItemDelegate {
text: "ItemDelegate" + modelData
// width: sidebar.width
onImplicitWidthChanged: if (sidebar.contentWidth < 0) {
sidebar.contentWidth = implicitWidth
}
}
}
}
}
This is because the first non-zero implicit width of the delegate is probably only taking into account the horizontal padding, and the actual text implicit width comes shortly afterwards. Your actual example might be different (and I'm curious how it works), but without the complete code we'll continue with ItemDelegate as an example. Two other issues here are that enabling clipping will make the ListView clipped to 12 pixels wide, and setting an explicit width on each delegate (to ensure that the interactive area for each delegate is consistent) will consequently also result in them being 12 pixels wide.
You can instead keep track of the maximum, but it will still result in the width of the view changing as it is scrolled, which isn't desirable:
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
ApplicationWindow {
visible: true
width: 800
height: 600
title: "width: " + sidebar.width
ColumnLayout {
width: 200
height: parent.height
Label {
text: "Some extra stuff here"
}
ListView {
id: sidebar
implicitWidth: contentWidth
clip: true
Layout.fillHeight: true
model: 1000
delegate: ItemDelegate {
text: "ItemDelegate" + modelData
width: sidebar.width
onImplicitWidthChanged: {
if (implicitWidth > sidebar.contentWidth)
sidebar.contentWidth = implicitWidth
}
}
}
}
}
Using contentItem.childrenRect.width for the view's size has the same problem. In addition, it won't allow you to make the delegates fill the width of the view, perhaps due to the childrenRect relying on size of the delegates, which itself is bound to the view.
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
ApplicationWindow {
visible: true
width: 800
height: 600
title: "width: " + sidebar.width
ColumnLayout {
width: 200
height: parent.height
Label {
text: "Some extra stuff here"
}
ListView {
id: sidebar
implicitWidth: contentItem.childrenRect.width
clip: true
Layout.fillHeight: true
model: 100
delegate: ItemDelegate {
text: "ItemDelegate" + modelData
}
}
}
}
Using the first item is also not going to be sufficient, because it might not be the widest item:
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
ApplicationWindow {
visible: true
width: 800
height: 600
title: "width: " + sidebar.width
ColumnLayout {
width: 200
height: parent.height
Label {
text: "Some extra stuff here"
}
ListView {
id: sidebar
implicitWidth: sidebar.contentItem.children[0].implicitWidth
clip: true
Layout.fillHeight: true
model: 100
delegate: ItemDelegate {
text: "ItemDelegate" + modelData
width: sidebar.width
}
}
}
}
The most foolproof solution is to load each item to find the largest one. This can be very expensive for large models, however. This approach was used for fixing https://bugreports.qt.io/browse/QTBUG-78281, and resulted in https://doc.qt.io/qt-6/qml-qtquick-controls-combobox.html#implicitContentWidthPolicy-prop.
Another approach which I think would be relevant here is LayoutGroup: https://bugreports.qt.io/browse/QTBUG-44078. This would delegate the responsibility of finding the largest implicit size to the LayoutGroup type, and then delegates could simply set their width to the implicitWidth property of LayoutGroup. This needs more research to see if it will work.
So, this is definitely an area that could be improved, but the simplest and most performant solution will always be to set a pre-calculated size on the ListView.
> --
> Arno Rehn
> Tel +49 89 189 166 0
> Fax +49 89 189 166 111
> a.rehn at menlosystems.com
> www.menlosystems.com
>
> Menlo Systems GmbH
> Bunsenstrasse 5, D-82152 Martinsried, Germany Amtsgericht München HRB
> 138145
> Geschäftsführung: Dr. Michael Mei, Dr. Ronald Holzwarth USt.-IdNr.
> DE217772017, St.-Nr. 14316170324
More information about the Development
mailing list