Need a suggestion for displaying song information

VRonin

How are you laying out each row at the moment?

jsmolka

Currently I am looping over every audio file with this loop

void MusicLibrary::loadAudios(Audios audios)
{
    clear();

    for (Audio *audio : audios)
    {
        SongInfo *info = new SongInfo(audio, this);
        info->showTitle();
        info->showArtist();
        info->showAlbum();
        info->showYear();
        info->showGenre();
        info->showLength(Qt::AlignRight);
        info->init({10, 10, 10, 1, 10, 1});

        QListWidgetItem *item = new QListWidgetItem(this);
        item->setSizeHint(QSize(0, Config::Library::itemHeight()));
        addItem(item);
        setItemWidget(item, info);
    }
}

The show functions create a label each. The init function loops over every created label and places it in a box layout.

void SongInfo::init(const QVector<int> &stretches)
{
    QHBoxLayout *layout = new QHBoxLayout(this);
    for (int i = 0; i < m_labels.size(); i++)
        layout->addWidget(m_labels[i], stretches[i]);
    setLayout(layout);
}

VRonin

setItemWidget should come with a huge "nuclear hazard" label on it.

The correct way to go is using QTableWidget to have different columns instead of QListWidget and use a subclass of QStyledItemDelegate to customise the painting if you need to.

You can store your data in the model using QTableWidgetItem::setData, you should not store it in the item widget (as it defeats the purpose of having a model at all)

jsmolka

@VRonin

Thanks you! I have implemented your suggestions and the load times decreased noticeably. I had to add a QStyleItemDelegate to extend the hover style to a row level.

Let's say I start my app and load the songs in the background. What's the best way to update the table without blocking the user interface or even just load the needed items?

jsulm

@jsmolka You could scan your songs in another thread and emit signals to GUI thread where you update the UI

jsmolka

@jsulm I have already tried that using the QListWidget. Problem there was that 1500 songs were scanned in 500ms and I emitted a signal for each one of them. As a result of that the UI updated at once (and got blocked) instead of inserting the songs one by one. But I will try again with the QTableWidget. Thank you.

VRonin

I'm still a bit perplexed, 1500 data points is not a lot, it should work seamlessly.

Can we try one thing? remove the delegate and just store the song title in the first column as a string in Qt::EditRole and nothing else, see if that works faster

jsmolka

@VRonin

I am doing the following now (without delegate)

void MusicLibrary::loadAudios(Audios audios)
{
    clear();

    setRowCount(audios.size());
    setColumnCount(1);

    setUpdatesEnabled(false);
    for (int i = 0; i < audios.size(); i++)
    {
        QTableWidgetItem *item = new QTableWidgetItem;
        item->setData(Qt::EditRole, audios[i]->title());
        setItem(i, 0, item);
    }
    setUpdatesEnabled(true);
}

It's not slow but it's not seemless either. Not sure if it helps but those are the changes I made to the QTableWidget.

setAlternatingRowColors(true);
setEditTriggers(QAbstractItemView::NoEditTriggers);
setFocusPolicy(Qt::NoFocus);
setFrameStyle(QFrame::NoFrame);
//setItemDelegate(new RowHoverDelegate(this, this));
setSelectionMode(QAbstractItemView::NoSelection);
setShowGrid(false);
setStyleSheet(loadStyleSheet());
setVerticalScrollMode(QAbstractItemView::ScrollPerPixel);   
setWordWrap(false);

horizontalHeader()->hide();
horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch);
horizontalScrollBar()->hide();

verticalHeader()->hide();

jsulm

@jsmolka You could emit a signal for a set of songs, lets say for 10.

J.Hilk

@jsulm said in Need a suggestion for displaying song information:

@jsmolka You could emit a signal for a set of songs, lets say for 10.

Mmh, correct me if I'm wrong here, but, as long as the loop is going on, the Repaint event is postponed until the next time the event loop ist processed 🤔

The op would need to do something like calling processEvents() to update the ui. QThread is not an option, as QTableWidgetItem is a gui element and therefore may only be processed in the main thread!?

jsulm

@J.Hilk I suggested to use an extra thread for music file scanning. From this thread he is emitting a signal with music data which is then handled in the GUI thread. His problem is now that he is emitting the signal for each song and thus flooding the GUI thread. That's why I suggested to group the song data to not to emit too many signals. I did NOT suggest to use GUI related classes in other threads than GUI thread. If he is using such classes in this second thread then it is clearly wrong (I did not check his code that thoroughly).

Mr Gisa

Perhaps you could use Qt Concurrent, specially if you want to add a sugar progress to the process. https://doc.qt.io/qt-5.10/qtconcurrent-index.html

I don't think you need to create a worker thread class just to make this simple thing as you already have the Qt Concurrent to do that for you in a simply way. You can even, pause/continue and stop the process with it.

jsmolka

Thanks for all your suggestion. With a seperate thread for loading and the QTableWidget instead of QListWidget all runs smoothly.

Just another question out of curiosity. If I create a QTableWidgetItem on the heap, do I have to delete it manually? Because there is no parent property unlike in most of Qt's classes.

Mr Gisa

@jsmolka I don't think that you need to delete it manually as the widget will take the ownership of the item.

JonB

@jsmolka

@Mr-Gisa is correct.

http://doc.qt.io/qt-5/qtablewidget.html#setItem

The table takes ownership of the item

See also https://stackoverflow.com/a/21998917/489865