Use trackcolor for whole row

[Holzhaus]

The track color column is easy to overlook (especially if it's next to the cover art). Hence, I thought it might be cool paint the whole row in a dark shade of track color (similar to what Traktor does: https://www.youtube.com/watch?v=PV4VI-FtaLg)

This even allows using track colors on small screens where the color column takes too much space:

[uklotzde]

These kind of features are the reason why we urgently need #2538 to move the generic, reusable part of the code out of BaseSqlTableModel.

[uklotzde]

For tracks without an assigned track color this technique could also be applied for the (background) color extracted from the cover image in #2524. I will try how it looks like.

...Two different color sources might be confusing and adding a configuration option to switch the row color between track color, cover art color, or both might be overkill. Nevertheless we could hard-code the logic and only use track color by default.

[Holzhaus]

Done.

[ronso0]

This is a nice extension of the track color feature, thanks!
I can't comment on the code, but I'm building this right now to check how it works in skins and with color schemes.

[ronso0]

This works well for our dark skins.
Though the track colors should be stronger for brighter skins IMO. We don't have any yet (I plan to create a daylight scheme for LateNight) but for those it would be nice if the color opacity could be adjusted in the <Library> node, with a decimal value in <TrackColorRowOpacity>0.40</TrackColorRowOpacity> for example.

[Holzhaus]

This works well for our dark skins.
Though the track colors should be stronger for brighter skins IMO. We don't have any yet (I plan to create a daylight scheme for LateNight) but for those it would be nice if the color opacity could be adjusted in the <Library> node, with a decimal value in <TrackColorRowOpacity>0.40</TrackColorRowOpacity> for example.

I don't see a straightforward way to pass this value from WLibrary to BaseSqlTableModel. This would be nice to have, but we don't have any daylight skin right now, so let's keep this PR small. We can just add this later on if it becomes necessary. .

[uklotzde]

This works well for our dark skins.
Though the track colors should be stronger for brighter skins IMO. We don't have any yet (I plan to create a daylight scheme for LateNight) but for those it would be nice if the color opacity could be adjusted in the <Library> node, with a decimal value in <TrackColorRowOpacity>0.40</TrackColorRowOpacity> for example.

I don't see a straightforward way to pass this value from WLibrary to BaseSqlTableModel. This would be nice to have, but we don't have any daylight skin right now, so let's keep this PR small. We can just add this later on if it becomes necessary. .

I have an idea how to do this. But first let's get this PR merged and BaseTrackTableModel ready.

[ronso0]

This works well for our dark skins.
Though the track colors should be stronger for brighter skins IMO. We don't have any yet (I plan to create a daylight scheme for LateNight) but for those it would be nice if the color opacity could be adjusted in the <Library> node, with a decimal value in <TrackColorRowOpacity>0.40</TrackColorRowOpacity> for example.

I don't see a straightforward way to pass this value from WLibrary to BaseSqlTableModel. This would be nice to have, but we don't have any daylight skin right now, so let's keep this PR small. We can just add this later on if it becomes necessary. .

It will also e helpful for darker skins. But yeah, let's merge this and add it later.

[ronso0]

An idea for further improvement:
paint a selected table row with the track color and pick a contrasting color for the text, like it is done for the hotcue markers in the overview.

[ronso0]

GUI part lgtm.

[Holzhaus]

Another idea for further improvement in the future: Instead of painting the row background, selected rows should have a border around them (just like when hovering the prelisten controls). That we we could show the track color as row background even for selected rows.

[uklotzde]

Debug assertion is still not fixed yet:

DEBUG ASSERT: "bgValue.canConvert<QBrush>()" in function static void TableItemDelegate::paintItemBackground(QPainter*, const QStyleOptionViewItem&, const QModelIndex&) at /tmp/mixxx/src/library/tableitemdelegate.cpp:67

[uklotzde]

Deselecting the color does not trigger a repaint of the row. Only after selecting a different row the color is updated.

[Holzhaus]

Deselecting the color does not trigger a repaint of the row. Only after selecting a different row the color is updated.

What do you mean be deselecting the color?

[ronso0]

Deselecting the color does not trigger a repaint of the row. Only after selecting a different row the color is updated.

What do you mean be deselecting the color?

True, didn't notice it due to the color column being out oof view: if you select the top-left default color the row doesn't get repainted immediately but after another rows is selected.
For 'un-coloring' multiple rows at once this works instantly though.

[Holzhaus]

if you select the top-left default color the row doesn't get repainted immediately but after another rows is selected.

I think Track::setColor does not cause the dataChanged to be fired in all circumstances. I don't know a clean way to handle this tbh because I'm not sure how track changes propagate to basesqltablemodel. In any case I think we can merge this as-is and handle that bug in a separate PR, because it's also present in master.

[ronso0]

Seems I didn't check carefully enough earlier: in Shade I noticed that the style of WLibrary QPushButton:checked overrides WColorPicker QPushButton[noColor="true"] from the default stylesheet ( #2470 )
In the other skins it's black as intended because Library feature buttons are addressed differently.

Unfortunately I did not yet have the time to restore my local git folder, so I can't supply a PR for Shade right now.
I'm working on a patch, though, that paints the default color black and also applies the button style from #CueColorPicker QPushButton. Let me know if you need that before merge or if apply it afterwards.

[uklotzde]

dataChanged

I tested master after the discovering the bug to do a cross check before reporting it. Everything worked as expected. Now I tested master again and it didn't work. This is strange. But the bug was definitely not caused by by this PR!

[uklotzde]

@ronso0 Does that mean that the bug is caused by the skins? Probably yes, because it only occurs when selecting no color. When selecting an actual color table cells are updated immediately. This should be fixed in a separate PR.

@Holzhaus Let's revert or better drop the last commit and then everything is ready for merge.

[ronso0]

Does that mean that the bug is caused by the skins?

@uklotzde We're talking about different issues:
1 default color button in WColorPicker is not black only in Shade > fixable, but tricky
2 repaint of the row and color column does not always happen immediately in all skins with 3e9678f , could not yet test the last commit so I have no expertise on that

[Holzhaus]

@uklotzde

Let's revert or better drop the last commit and then everything is ready for merge.

Done.

[uklotzde]

@Holzhaus Thanks. Let's not introduce any workarounds before we know the actual cause. I noticed that sometimes if I select no color it is not picked. A moment later everything works as expected. Strange. The issue might also be related to the color picker.

[uklotzde]

LGTM. We don't need to wait on CI builds again.