How to simplify a complex item delegate?

I have a QListView which paints (and measures) its items with a custom QStyledItemDelegate.

But over time the item grew quite complex. When it was still a thumbnail and to the right of it a bunch of header/value label pairs aligned in a column the code was long and ugly but it was still manageable. But when now I have to add some additional icons the measuring and painting code got really awful. It feels more than natural to use a custom QWidget instead since it can easily use all the goodies of UI Designer, styling and so on.

However by reading documentation and tutorials I felt like it is the *View family of widgets that gets "more love" than the *Widget family. While my personal experience leads me to think that QAbstractItemModel-based approach to Model/View framework is actually quite limited and doing anything more than what Qt has thought about quickly becomes quite difficult.

Or is there maybe some other approach to writing custom QStyledItemDelegate which deals with complexity better?

It really depends on what you want to do. For some cases it might be feasible to replace QListView with QDeclarativeView and implement the list in QML where it is much easier to code delegates. But again, maybe it is easy to do what you want with item views and you just need to restructure your code.

Quote:

---

Originally Posted by wysota

It really depends on what you want to do.

---

For example something like the Windows Media Player list view shown here. Or even its sub-case: just the album thumbnail with labels on the right. The more data you have the more difficult it gets to do it with QAbstractItemDelegate. Not to mention how styling gets difficult.

If you have a single constant layout then implementing such a delegate is trivial. I thought you wanted something more complex. Just bear in mind this is a table (or even a tree) and not a list.

Quote:

---

Originally Posted by wysota

If you have a single constant layout then implementing such a delegate is trivial.

---

I would not agree with that. Especially after writing such a delegate just few days ago (and mine had just a thumbnail + 2 pairs of header/value labels). It took me some time (OK, I'm know to Qt) and resulting code is long, difficult to understand and maintain. Even thou it contains mostly just calls to measuring functions. Adding another 2 labels or some extra icons, especially by someone else after some time would be really painful.

And such layout code felt like a natural candidate for generalization and extraction as a separate class/function. With this question I was trying to know those generalization.

Quote:

---

Originally Posted by wysota

I thought you wanted something more complex. Just bear in mind this is a table (or even a tree) and not a list.

---

It is not really important. After all it was just an example.

Quote:

---

Originally Posted by Adam Badura

I would not agree with that. Especially after writing such a delegate just few days ago (and mine had just a thumbnail + 2 pairs of header/value labels). It took me some time (OK, I'm know to Qt) and resulting code is long, difficult to understand and maintain.

---

Well, you have every right to disagree however in my opinion the delegate for the thumbnail and those three labels is around 3-6 lines long (depending how you choose to implement it).

Quote:

---

And such layout code felt like a natural candidate for generalization and extraction as a separate class/function. With this question I was trying to know those generalization.

---

You can write such layout engine however in my opinion it is an overkill since your "layout" is static and always the same. Having a layout engine makes sense if you expect to have different layouts in every item or you expect that some pieces of data will change their visual position and the layout will have to be recalculated.

Quote:

---

It is not really important. After all it was just an example.

---

On the contrary, this is very important. In a list you have to render the whole row in one go. With a table or a tree every column is rendered separately thus implementation of each delegate is greatly simplified. You have to choose proper tools for each job.

Quote:

---

Originally Posted by wysota

Well, you have every right to disagree however in my opinion the delegate for the thumbnail and those three labels is around 3-6 lines long (depending how you choose to implement it).

---

Could you provide an example? Since maybe I just did it in some very sub-optimal way.

In my case it is a QTreeView but it could be limited to QListView as we are using it that way currently. There is a thumbnail in Qt::DecorationRole. To the right of it there are two text fields stacked vertically: "Name: ..." with value taken from Qt::DisplayRole and "Length: ..." with value from custom role. The header labels are aligned to the left, and the values are aligned to the left so there are two columns.

Dear wysota, you wrote:

Quote:

---

Originally Posted by wysota

If you have a single constant layout then implementing such a delegate is trivial.

---

and

Quote:

---

Originally Posted by wysota

Well, you have every right to disagree however in my opinion the delegate for the thumbnail and those three labels is around 3-6 lines long (depending how you choose to implement it).

---

Since I found it highly questionable I asked for that simple thing:

Quote:

---

Originally Posted by Adam Badura

Could you provide an example? Since maybe I just did it in some very sub-optimal way.

In my case it is a QTreeView but it could be limited to QListView as we are using it that way currently. There is a thumbnail in Qt::DecorationRole. To the right of it there are two text fields stacked vertically: "Name: ..." with value taken from Qt::DisplayRole and "Length: ..." with value from custom role. The header labels are aligned to the left, and the values are aligned to the left so there are two columns.

---

Over 10 days passed. You were here. And yet you did not provide those trivial 3-6 lines... Let others who come here with similar issue judge that...

Quote:

---

Let others who come here with similar issue judge that...

---

Well it is because he don't want to write the delegate for you and one annoying thing is when people requesting code but didn't provide what they have tried so far. This leads to the conclusion - which my not be valid in every case - that nothing has be done so far... So I can understand him. And the attitude you are showing is not helpful either. (Beside, if someone has more than 28.000 posts and more then 4.000 thanks, which is the most here, he probably is a nice guy. In this case, I can confirm that;))

As to your problem: Have you seen the Star delegate Example? From there:

Code:

---

void StarDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
                          const QModelIndex &index) const
 {
     if (qVariantCanConvert<StarRating>(index.data())) {
         StarRating starRating = qVariantValue<StarRating>(index.data());
 
         if (option.state & QStyle::State_Selected)
             painter->fillRect(option.rect, option.palette.highlight());
 
         starRating.paint(painter, option.rect, option.palette,
                          StarRating::ReadOnly);
     } else {
         QStyledItemDelegate::paint(painter, option, index);
     }
 }

---

Which total represent your example link of a music player.

Quote:

---

Originally Posted by Lykurg

Which total represent your example link of a music player.

---

It does not. The star sample skips the most important (for me) part: the thumbnail and stacked labels on the right of it. And that is what I actually want to achieve (the Windows Media Player sample was just first that same to my mind - and that I found - when trying to provide some sample). What I wanted to achieve I have described in the very first post (second paragraph) and in the one that asked explicitly for the "trivial code" (also second paragraph).

By the way: In my Qt 4.8.4 when I build Examples and Demos with qmake -tp vc -r the resulting Visual Studio solution did not open cleanly in Visual Studio since some projects had same names and the solution didn't use its folders. Yet I believe this is only a "GUI issue". MS Build shouldn't care about it anyway. More important issue was that the star example wasn't build at all! It is not included in its "parent .pro file".

I didn't post more details on my current state since I considered that irrelevant and "obfuscated" to much with particular details of my project while my investigations and research lead me to think it is a general issue rather than my specific one. But sure, I can give more details. This is how it looks now:

Attachment 8742

It is a QTreeView (from historical reasons and for future extensions; for what it is now it could very well be just a QListView). The white header at the begining (with "Adam Badura") is a header with single column. Bellow are the items. The backgrounds alternate (although even-background is not visible here). Various states are represented with various background colors. On the left of each item there is a thumbnail (in this quickly-taken snapshot all thumbnails are same). To the right of it are the stacked labels (likely soon we will want to add at least two more labels). The green arrow is a quickly added icon. It does not align to anythig (yet).

The view itself works in two modes. This is the "expanded" one. The "simple" one has shorter single-line items done mostly with default drawing. It is done by alternating item delegate. But I'm considering switching to alternating QTreeViews instead (that would just share single model) as it seems simpler and does not require use of apparently undocumented QTreeView::doItemsLayout() function.

Now the crucial part of the code it auxiliary delegate function measureItem (it is my own function). It measures and lays out all the parts of the item and the item itself. Then I call it from QStyledItemDelegate::sizeHint (for the item size) and from QStyledItemDelegate::paint (which next just calls a bunch of drawing functions within the already computed rectangles). Code is following:

Code:

---

void ElExtendedPlaylistEditorDelegate::measureItem(
  QStyleOptionViewItemV4 const& option,
  QStyle const* stylePtr,
  QModelIndex const& index,
  QRect& item,
  QRect& thumbnail,
  QRect& nameHeaderLabel,
  QRect& nameDataLabel,
  QRect& lengthHeaderLabel,
  QRect& lengthDataLabel,
  QRect& videoEffectImageRect
) const
{
  // More or less arbitrarily selected values.
 
  // Margins within the item in which components will be laid out.
  QMargins padding( 2, 2, 2, 2 );
  // Enough to show a reasonable icon.
  int minimumThumbnailHeight = 64;
  // HD aspect ratio.
  boost::rational< int > thumbnailAspectRatio( 1920, 1080 );
 
  int thumbnailLabelsMargin =
    stylePtr->layoutSpacing(
      QSizePolicy::DefaultType,
      QSizePolicy::Label,
      Qt::Horizontal,
      &option,
      option.widget
    );
  if ( thumbnailLabelsMargin == -1 )
    thumbnailLabelsMargin = 5;
 
  int labelsVerticalMargin =
    stylePtr->layoutSpacing(
      QSizePolicy::Label,
      QSizePolicy::Label,
      Qt::Vertical,
      &option,
      option.widget
    );
  if ( labelsVerticalMargin == -1 )
    labelsVerticalMargin = 2;
 
  // Obtain/reformat some data.
 
  EbTimecode length =
    index.data(
      sps::spsGUI::PlaylistModel::CustomRole::ITEM_LENGTH
    ).value< EbTimecode >();
  QString lengthString = length.getTimecodeString();
 
 
  // Measure components which are not flexible.
 
  // Note that the measuring includes a space between header and data. This way
  // margin is scaled well with the font that is used rather than fixed to given
  // pixels value.
 
  nameHeaderLabel =
    stylePtr->itemTextRect(
      option.fontMetrics,
      QRect(),
      Qt::AlignLeft | Qt::AlignBottom,
      ( option.state & QStyle::State_Enabled ) != 0,
      tr( "Name:" ) + " "
    );
 
  nameDataLabel =
    stylePtr->itemTextRect(
      option.fontMetrics,
      QRect(),
      Qt::AlignLeft | Qt::AlignBottom,
      ( option.state & QStyle::State_Enabled ) != 0,
      option.text
    );
 
  lengthHeaderLabel =
    stylePtr->itemTextRect(
      option.fontMetrics,
      QRect(),
      Qt::AlignLeft | Qt::AlignBottom,
      ( option.state & QStyle::State_Enabled ) != 0,
      tr( "Length:" ) + " "
    );
 
  lengthDataLabel =
    stylePtr->itemTextRect(
      option.fontMetrics,
      QRect(),
      Qt::AlignLeft | Qt::AlignBottom,
      ( option.state & QStyle::State_Enabled ) != 0,
      lengthString
    );
 
  // it will be changed after demo:)
  videoEffectImageRect = QRect( 0, 0, 30, 20 );
 
  // Now start laying out the components with labels.
 
  nameHeaderLabel.translate(
    -nameHeaderLabel.left(),
    -nameHeaderLabel.top()
      + option.rect.top()
      + padding.top()
  );
  nameDataLabel.translate(
    -nameDataLabel.left(),
    -nameDataLabel.top()
      + option.rect.top()
      + padding.top()
  );
 
  Q_ASSERT( nameHeaderLabel.height() == nameDataLabel.height() );
  int nameJoinedLabelHeight =
    ( std::max )( nameHeaderLabel.height(), nameDataLabel.height() );
 
  lengthHeaderLabel.translate(
    -lengthHeaderLabel.left(),
    -lengthHeaderLabel.top()
      + option.rect.top()
      + padding.top()
      + nameJoinedLabelHeight
      + labelsVerticalMargin
  );
  lengthDataLabel.translate(
    -lengthDataLabel.left(),
    -lengthDataLabel.top()
      + option.rect.top()
      + padding.top()
      + nameJoinedLabelHeight
      + labelsVerticalMargin
  );
  videoEffectImageRect.translate(
    -videoEffectImageRect.left(),
    -videoEffectImageRect.top()
      + option.rect.top()
      + padding.top()
      + nameJoinedLabelHeight
      + labelsVerticalMargin
      + labelsVerticalMargin
  );
 
  Q_ASSERT( lengthHeaderLabel.height() == lengthDataLabel.height() );
  int lengthJoinedLabelHeight =
    ( std::max )( lengthHeaderLabel.height(), lengthDataLabel.height() );
 
  int headersWidth =
    ( std::max )( nameHeaderLabel.width(), lengthHeaderLabel.width() );
 
  nameHeaderLabel.translate(
    option.rect.left()
      + padding.left(),
    0
  );
  nameDataLabel.translate(
    option.rect.left()
      + padding.left()
      + headersWidth,
    0
  );
 
  lengthHeaderLabel.translate(
    option.rect.left()
      + padding.left(),
    0
  );
  lengthDataLabel.translate(
    option.rect.left()
      + padding.left()
      + headersWidth,
    0
  );
  videoEffectImageRect.translate(
    option.rect.left()
      + padding.left()
      + headersWidth
      + lengthDataLabel.width(),
    0
  );
 
  int labelsWidth =
    headersWidth
      + ( std::max )( nameDataLabel.width(), lengthDataLabel.width() );
  int labelsHeight =
    nameJoinedLabelHeight
      + labelsVerticalMargin
      + lengthJoinedLabelHeight;
 
 
  // Now that the labels are laid out we have to include the thumbnail.
 
  int thumbnailHeight =
    (std::max )( minimumThumbnailHeight, labelsHeight );
  auto thumbnailWidthExact = thumbnailAspectRatio * thumbnailHeight;
  int thumbnailWidth = boost::rational_cast< int >( thumbnailWidthExact );
 
  Q_ASSERT( thumbnailHeight >= labelsHeight );
  int itemContentsHeight = thumbnailHeight;
 
  int itemContentsWidth =
    thumbnailWidth
      + thumbnailLabelsMargin
      + labelsWidth;
 
 
  thumbnail.setCoords(
    option.rect.left()
      + padding.left(),
    option.rect.top()
      + padding.top(),
    option.rect.left()
      + padding.left()
      + thumbnailWidth,
    option.rect.top()
      + padding.top()
      + itemContentsHeight
  );
 
  // Offset labels by the thumbnail width (plus margin).
 
  nameHeaderLabel.translate(
    thumbnailWidth + thumbnailLabelsMargin,
    0
  );
  nameDataLabel.translate(
    thumbnailWidth + thumbnailLabelsMargin,
    0
  );
  lengthHeaderLabel.translate(
    thumbnailWidth + thumbnailLabelsMargin,
    0
  );
  lengthDataLabel.translate(
    thumbnailWidth + thumbnailLabelsMargin,
    0
  );
 
  videoEffectImageRect.translate(
    thumbnailWidth + thumbnailLabelsMargin,
    0
  );
 
  // Finally item size itself.
 
  item.setCoords(
    option.rect.left(),
    option.rect.top(),
    option.rect.left()
      + padding.left()
      + itemContentsWidth
      + padding.right(),
    option.rect.top()
      + padding.top()
      + itemContentsHeight
      + padding.bottom()
  );
}

---

As can be seen obviously it is neither trivial nor 3-6 lines...

And now consider that user requests a change and wants the "header labels" ("Name:" and "Length:") to be aligned to right rather than to left in its column. It can be done. But it requires a programmer who understands the code. While conceptually it should be just "changing an enum value" and I believe UI Designer would allow such change much easier. But since I have QTreeView I cannot design my items in UI Designer...

I have a crazy idea. :D

What if you add to your delegate pointer to a shadow QWidget.
Then you'll do all the custom layout in designer, and add resulting QWidget to your delegate.
Then in paint event you apply necessary tweaks to the widget, and then render it using QWidget::render ()!

Quote:

---

Originally Posted by Adam Badura

It does not. The star sample skips the most important (for me) part: the thumbnail and stacked labels on the right of it.

---

Ups, I call that selective perception... Right.

Quote:

---

Now the crucial part of the code

---

Ok, you have a tree view: 1st column the image -> handled by the standard item delegate. 2nd column the labels. To simplify that and to extend it easily use loops e.g.

Code:

---

QList<QPair<QString,QString> > labels; // a pair has "label:value"
// max width of label description:
int maxDescWidth = 0;
foreach(QPair<QString,QString> l, labels)
    maxDescWidth = qMax(maxDescWidth, fontmetrics.width(l.first));
 
// then drawing
int height = fontmetrics.height() + padding;
for(int i = /*...*/) {
    painter->drawText(QRect(0,i*height, maxDescWidth, height), Qt::AlignRight, labels.at(i).first);
    // same for the text.
}

---

That's just a quick mockup but I guess you see where it is going. This way you don't need a function calculating each bounding rect.

Quote:

---

Originally Posted by Lykurg

This way you don't need a function calculating each bounding rect.

---

(Just a quick note, I will think about both above solutions a bit more later.)

I need to know the size for QStyledItemDelegate::sizeHint as well...

Also I considered using columns. At least to separate the thumbnail and labels issues. But I found it to complex and risky in face of the two modes I described earlier. And thus I considered making those two separate QTreeView in the first place. I will think about it as well.

Quote:

---

Originally Posted by Adam Badura

I need to know the size for QStyledItemDelegate::sizeHint as well...

---

Yes, but only the height and width, not every exact position of the elements. Also I guess the count of labels is known and static, so you can cache that information. So it would be easy:

Code:

---

int countOfLabels = 3; // or whatever
int h = 0;
h = countOfLabels * (fontmetrics.height() + padding)
h = qMax(h, heightOfThumbnail);

---

Quote:

---

Also I considered using columns.

---

Also that doesn't change much of my example:

Code:

---

QList<QPair<QString,QString> > labels; // a pair has "label:value"
// Draw the pixmap
int currentX = pixmap.width() + padding /* +... */;
 
// then drawing
int height = fontmetrics.height() + padding;
for(int i = /*...*/) {
    painter->drawText(QRect(currentX, i*height, maxDescWidth, height), Qt::AlignRight, labels.at(i).first);
    // same for the text.
}

---

The solution with a QWidget is possible but then you could simply use Q***Widget. And also the delegate functions are called quite a lot so it isn't with good performance.

Quote:

---

Originally Posted by Adam Badura

Over 10 days passed. You were here. And yet you did not provide those trivial 3-6 lines...

---

Ouch, sorry, I forgot about it.

This should work (not tested but you should get the picture...)

Code:

---

void paint ( QPainter * painter, const QStyleOptionViewItem & option, const QModelIndex & index ) const {
  QPixmap px = index.data(Qt::DecorationRole).toPixmap();
  painter->drawPixmap(option.rect.topLeft()+QPoint(1,1), px);
  painter->drawText(option.rect.adjusted(px.width+1+5, 1, -1, -1), Qt::AlighLeft|Qt::AlignTop, QString("%1\n%2\n%3").arg(index.data(Qt::DisplayRole).toString()).arg(index.data(...).toString()).arg(index.data(...).toString()));
}

---

Similar for sizeHint(), just use QFontMetrics::boundingRect() instead of painting.

Edit: Next time, instead of assuming my bad will, just send me a PM with a reminder or bump the thread.

Quote:

---

Originally Posted by wysota

This should work (not tested but you should get the picture...)

Code:

---

void paint ( QPainter * painter, const QStyleOptionViewItem & option, const QModelIndex & index ) const {
  QPixmap px = index.data(Qt::DecorationRole).toPixmap();
  painter->drawPixmap(option.rect.topLeft()+QPoint(1,1), px);
  painter->drawText(option.rect.adjusted(px.width+1+5, 1, -1, -1), Qt::AlighLeft|Qt::AlignTop, QString("%1\n%2\n%3").arg(index.data(Qt::DisplayRole).toString()).arg(index.data(...).toString()).arg(index.data(...).toString()));
}

---

---

Only that this does not do what I wanted. The 3 text fields consist of two columns which have to be aligned properly. I did mention that and it was visible on the image of current state I posted in the mean time.

Although I do like the idea of drawing multiline text. I wasn't aware this is possible. Some simplification at least.

Quote:

---

Originally Posted by Lykurg

The solution with a QWidget is possible but then you could simply use Q***Widget. And also the delegate functions are called quite a lot so it isn't with good performance.

---

Why do you think this is bad? I am actually very attracted to this idea as it would allow me to do what I indeed wanted: use UI for designing the items. Also this might ease styling. After all the QWidget drawing/layout would do the same thing that I would do "by hand".

Quote:

---

Originally Posted by Adam Badura

Only that this does not do what I wanted. The 3 text fields consist of two columns which have to be aligned properly. I did mention that and it was visible on the image of current state I posted in the mean time.

---

I don't see any "two columns" on the image you posted (http://stackoverflow.com/questions/4...tyled-listview). Could you elaborate what you mean? I can see a thumbnail and four rows of single word strings.

Quote:

---

Why do you think this is bad?

---

This is really bad, trust me :) Try having more than 10 rows with cell widgets and notice how performance drops. If you need anything complex I would really consider using QtQuick for the view.

Anyway I will think about some mechanism for simplified delegate layout. I think for static cases such as yours this is possible. It's much more complicated if you want to do animations (e.g. like in QProgressBar on modern Windows). I used to tutor a bachelor thesis that aimed to work around this problem.

If you don't hear back from me in a couple of days, send me a PM or bump the thread.

Quote:

---

Originally Posted by wysota

I don't see any "two columns" on the image you posted (http://stackoverflow.com/questions/4...tyled-listview). Could you elaborate what you mean? I can see a thumbnail and four rows of single word strings.

---

See second paragraph of my first post. Then second paragraph of the post in which I asked you for sample. Then the post in which I provided my current code and a snapshot of what it does. The Windows Media Player example was just that. An example of similar layout which I found quickly. I didn't know any better example and didn't want to spend to much time looking for one.

Quote:

---

Originally Posted by wysota

This is really bad, trust me :) Try having more than 10 rows with cell widgets and notice how performance drops. If you need anything complex I would really consider using QtQuick for the view.

---

I didn't want to have QWidget for every item. (This way I could use QTreeWidget instead...) I wanted to have just one object hidden behind the scenes which I would feed with data (either directly or by sharing model and changing "active index") to than size/paint through it. Would that be bad to? Why? What more would it do (as compared to ordinary delegate) that it would influence performance so much?

I cannot switch to QtQuick. I would like but the decision is not up to me.

Quote:

---

Originally Posted by Adam Badura

See second paragraph of my first post. Then second paragraph of the post in which I asked you for sample. Then the post in which I provided my current code and a snapshot of what it does. The Windows Media Player example was just that. An example of similar layout which I found quickly. I didn't know any better example and didn't want to spend to much time looking for one.

---

Your delegate is still pretty simple.

Quote:

---

I didn't want to have QWidget for every item. (This way I could use QTreeWidget instead...)

---

I don't understand how using QTreeWidget influences anything here.

Quote:

---

I wanted to have just one object hidden behind the scenes which I would feed with data (either directly or by sharing model and changing "active index") to than size/paint through it. Would that be bad to? Why?

---

You would have to modify most properties of that object during every paint which is quite heavy.

Quote:

---

I cannot switch to QtQuick.

---

Not even for this single view?

Just to let you know, I already have a preliminary working solution. It's not pixel-perfect yet, but it already gives acceptable results.

Attachment 8769

Code:

---

DelegateLayout *l = new DelegateLayoutFrame(Qt::Horizontal);
l->setSpacing(10);
l->setMargins(QMargins(5,5,5,5));
DelegatePixmap *p1 = new DelegatePixmap(l);
DelegateText *t1 = new DelegateText(l);
t1->setRole(Qt::UserRole+1);
t1->setMargins(QMargins(5,5,5,5));
t1->setMinimumSize(QSize(QFontMetrics(f).width("MMMM"), 1));
 
DelegateLayout *headersl = new DelegateLayout(Qt::Vertical, l);
DelegateStaticText *h1 = new DelegateStaticText(headersl);
h1->setText("Name:");
DelegateStaticText *h2 = new DelegateStaticText(headersl);
h2->setText("Created:");
 
DelegateLayout *valuesl = new DelegateLayout(Qt::Vertical, l);
DelegateText *t2 = new DelegateText(valuesl);
t2->setRole(Qt::DisplayRole);
t2->setForegroundColor(Qt::darkGreen);
DelegateText *t3 = new DelegateText(valuesl);
t3->setRole(Qt::UserRole);
QListView v;
 
LayoutDelegate *ld = new LayoutDelegate(&v);
ld->setLayout(l);
v.setItemDelegate(ld);
v.setModel(&model);
 
v.show();

---