bpm lock icon rendered incorrectly on QT6 (no QML)

[ywwg]

Bug Description

The bpm column does not look correct on QT6:

The custom lock icon we use is not being drawn.

Version

HEAD

OS

Ubuntu 22.04

[ronso0]

Maybe just an error in the SVG? How does it look like in PaleMoon?

/res/skins/LateNight/classic/buttons/btn__lib_bpm_locked_orange.svg is not optimized / cleaned up, the blue PaleMoon version is.
Can you try saving the Classic checkmark as 'Optimized SVG' with default settings (which should remove unused tags and collapse groups)?

[daschuer]

I can confirm the issue with all skins. However only if the row is not selected. In selected case everything looks fine.

[ronso0]

Can you try the explicit !selected selector? LateNight Classic:

#LibraryBPMButton::indicator:!selected:checked,
#LibraryBPMButton::indicator:selected:checked {
  image: url(skin:/classic/buttons/btn__lib_bpm_locked_orange.svg);
}
#LibraryBPMButton::indicator:!selected:unchecked,
#LibraryBPMButton::indicator:selected:unchecked {
  image: url(skin:/classic/buttons/btn__lib_bpm_unlocked_grey.svg);
}

[ywwg]

no, that does not work for me. weird.

[ronso0]

So the WTrackTableView::indicator (played column) rules (applied to all QCheckBox indicators) are not overwritten by the #LibraryBPMButton::indicator rules.

mixxx/src/library/bpmdelegate.cpp

Lines 63 to 94 in d763be6

	void BPMDelegate::paintItem(QPainter* painter,const QStyleOptionViewItem &option,
	const QModelIndex& index) const {
	// NOTE(rryan): Qt has a built-in limitation that we cannot style multiple
	// CheckState indicators in the same QAbstractItemView. The CSS rule
	// QTableView::indicator:checked applies to all columns with a
	// CheckState. This is a big pain if we want to use CheckState roles on two
	// columns (i.e. the played column and the BPM column) with different
	// styling. We typically want a lock icon for the BPM check-state and a
	// check-box for the times-played column and may want more in the future.
	//
	// This workaround creates a hidden QComboBox named LibraryBPMButton. We use
	// the parent QTableView's QStyle with the hidden QComboBox as the source of
	// style rules to draw a CE_ItemViewItem.
	//
	// Here's how you would typically style the LibraryBPMButton:
	// #LibraryBPMButton::indicator:checked {
	// image: url(:/images/library/ic_library_locked.svg);
	// }
	// #LibraryBPMButton::indicator:unchecked {
	// image: url(:/images/library/ic_library_unlocked.svg);
	// }
	QStyleOptionViewItem opt = option;
	initStyleOption(&opt, index);
	if (m_pTableView != nullptr) {
	QStyle* style = m_pTableView->style();
	if (style != nullptr) {
	style->drawControl(QStyle::CE_ItemViewItem, &opt, painter,
	m_pCheckBox);
	}
	}
	}

Can you try some other explicit selector, for example WTrackTableView QCheckBox::indicator?

[ronso0]

Oh, I think I got it (but it's surprising this worked before...):
the LibraryBPMButton rule

mixxx/res/skins/LateNight/style_classic.qss

Lines 2588 to 2595 in d763be6

	/* This is the only way to select the 'Played' checkbox.
	Note that this also selects the BPM lock. */
	WTrackTableView::indicator:checked {
	image: url(skin:/classic/buttons/btn__lib_checkbox_checked.svg);
	}
	WTrackTableView::indicator:unchecked {
	image: url(skin:/classic/buttons/btn__lib_checkbox.svg);
	}

is after the general indicator rule

mixxx/res/skins/LateNight/style_classic.qss

Lines 2113 to 2119 in d763be6

	/* Lock icon at the left */
	#LibraryBPMButton::indicator:checked {
	image: url(skin:/classic/buttons/btn__lib_bpm_locked_orange.svg);
	}
	#LibraryBPMButton::indicator:unchecked {
	image: url(skin:/classic/buttons/btn__lib_bpm_unlocked_grey.svg);
	}

Please try the other way around.

[ywwg]

still no luck. (qt bug?)

I am not sure I am doing this right because even if I comment out all of the indicator:checked / unchecked qss items and anything referencing btn__lib_checkmark_orange, I still see the unselected rows with the orange check.

[ronso0]

So it's probably the default QCheckBox indicator style.
You can try resetting the library text color by disabling

WTrackTableView,
WTrackTableView::indicator,

in style_classic.qss#L1104-L1105 and check if the checkmark color is also reset.

FWIW in res/skins/default.qss we use the QPushButton#LibraryBPMButton:checked selector, so maybe try
QPushButton#LibraryBPMButton:checked and
QPushButton#LibraryBPMButton::indicator:checked in LateNight/style_classic.qss ?

[ywwg]

Would it be possible to work up a draft PR? I am getting a little lost in these files and what changes I should be making to test this

[ronso0]

Try adding this snippet at the bottom of style_classic.qss

QPushButton#LibraryBPMButton::indicator:checked:!selected,
QPushButton#LibraryBPMButton::indicator:checked:selected,
QPushButton#LibraryBPMButton::indicator:checked {
  image: url(skin:/classic/buttons/btn__lib_bpm_locked_orange.svg);
}
QPushButton#LibraryBPMButton::indicator:unchecked:!selected,
QPushButton#LibraryBPMButton::indicator:unchecked:!selected,
QPushButton#LibraryBPMButton::indicator:unchecked {
  image: url(skin:/classic/buttons/btn__lib_bpm_unlocked_grey.svg);
}

If that doesn't fix it I'll take another look and create PR.

[JoergAtGithub]

On Windows the padlock symbol appeared, but off center and smaller than it should be.

[ronso0]

Please post a screenshot.

[ronso0]

@JoergAtGithub Could you test if disabling this part in WLibraryTableView makes a difference?

mixxx/src/widget/wlibrarytableview.cpp

Lines 227 to 232 in 37a71a9

	setStyleSheet(QStringLiteral(
	"WTrackTableView::indicator,"
	"#LibraryBPMButton::indicator {"
	"height: %1px;"
	"width: %1px;}")
	.arg(metrics.height() * 0.7));

[JoergAtGithub]

Please post a screenshot.
Windows 11:

[ronso0]

I didn't build with Qt6 yet so someone else needs to debug this. Above are some hints, maybe also try outline: none.

[ronso0]

affected macOS with qt 6.5.2 #11900

[foss-]

Adding macOS screenshot. When track is highlighted the icons are displayed fine, although a bit too small, Skin is LateNight, PaleMoon: Left checkmark is played state and right column is lock bpm.

[fwcd]

@ronso0 Removing that does indeed fix the issue for me (on macOS), but causes the lock and custom checkmark to render at roughly double the size:

My guess is that the checkmarks still render underneath, but simply aren't visible anymore.

[ronso0]

Could someone who can reproduce try adding
qproperty-icon: none;
edit:
and qproperty-icon: url(""); (just to be sure we don't miss a syntax rule or some Qt quirk)
to both BPM button selectors here

mixxx/res/skins/LateNight/style_palemoon.qss

Lines 2604 to 2609 in 03d0ce7

	#LibraryBPMButton::indicator:checked {
	image: url(skin:../LateNight/palemoon/buttons/btn__lib_bpm_locked_blue.svg);
	}
	#LibraryBPMButton::indicator:unchecked {
	image: url(skin:../LateNight/palemoon/buttons/btn__lib_bpm_unlocked_grey.svg);
	}

[fwcd]

Doesn't seem to change anything, unfortunately

[fwcd]

I've found that setting a border on the item actually fixes the issue:

#LibraryBPMButton::item {
  border: 0px;
}

Similarly, adding

WTrackTableView::item {
  border: 0px;
}

fixes the issue for the play count too:

Funny how border: 0px fixes the issue here and caused UI glitches in other places (#11970)... but it seems to work and I couldn't find any immediately obvious UI regressions.