Rework Crossover EQ plugin GUI (#7781)

* Switches to SVG assets
* Fixes layout issues
* Refactors redundant code

---------

Co-authored-by: Tres Finocchiaro <tres.finocchiaro@gmail.com>

data/themes/default/fader_knob.svg

@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" width="23" height="30">
+  <defs>
+    <linearGradient id="a">
+      <stop offset="0" stop-color="#1c1f24"/>
+      <stop offset="1" stop-color="#2c3138"/>
+    </linearGradient>
+    <linearGradient id="b">
+      <stop offset="0" stop-color="#08a342"/>
+      <stop offset="1" stop-color="#0bd556"/>
+    </linearGradient>
+    <linearGradient xlink:href="#a" id="c" x1="16" x2="16" y1="27.5" y2="4.5" gradientTransform="matrix(.52168 0 0 1.04337 3.15242 -2.19516)" gradientUnits="userSpaceOnUse"/>
+    <linearGradient xlink:href="#b" id="d" x1="4" x2="16" y1="8" y2="8" gradientTransform="translate(4.16667 11.33333) scale(.45833)" gradientUnits="userSpaceOnUse" spreadMethod="reflect"/>
+  </defs>
+  <rect width="12" height="24" x="5.5" y="2.5" fill="url(#c)" stroke="#000" stroke-linecap="round" rx="2.065" ry="2.065"/>
+  <path stroke="url(#d)" stroke-width="2" d="M6 15h11"/>
+  <path fill="#fff" d="M7.56445 3C6.6888 3 6 3.6888 6 4.56445v1C6 4.6888 6.6888 4 7.56445 4h7.8711C16.3112 4 17 4.6888 17 5.56445v-1C17 3.6888 16.3112 3 15.43555 3h-7.8711z" opacity=".1"/>
+  <path fill="#000" d="M7.56445 26C6.6888 26 6 25.3112 6 24.43555v-1C6 24.3112 6.6888 25 7.56445 25h7.8711C16.3112 25 17 24.3112 17 23.43555v1C17 25.3112 16.3112 26 15.43555 26Z" opacity=".3"/>
+</svg>

include/Fader.h

@@ -171,6 +171,7 @@ private:
 	QElapsedTimer m_lastPeakTimer_R;
 
 	QPixmap m_knob {embed::getIconPixmap("fader_knob")};
+	QSize m_knobSize;
 
 	/**
 	 * @brief Stores the offset to the knob center when the user drags the fader knob

include/embed.h

@@ -51,6 +51,24 @@ auto LMMS_EXPORT getIconPixmap(std::string_view name,
 	int width = -1, int height = -1, const char* const* xpm = nullptr) -> QPixmap;
 auto LMMS_EXPORT getText(std::string_view name) -> QString;
 
+/**
+ * @brief Temporary shim for QPixmap::deviceIndependentSize.
+ * @param pixmap The pixmap to get the size of.
+ * @return The device-independent size of the pixmap.
+ */
+#if QT_VERSION >= QT_VERSION_CHECK(6, 2, 0)
+[[deprecated("Use QPixmap::deviceIndependentSize() instead; See "
+             "https://doc.qt.io/qt-6/qpixmap.html#deviceIndependentSize")]]
+#endif
+inline auto logicalSize(const QPixmap &pixmap) noexcept
+{
+#if QT_VERSION >= QT_VERSION_CHECK(6, 2, 0)
+	return pixmap.deviceIndependentSize().toSize();
+#else
+	return pixmap.isNull() ? QSize() : pixmap.size() / pixmap.devicePixelRatio();
+#endif
+}
+
 } // namespace embed
 
 class PixmapLoader

plugins/CrossoverEQ/CMakeLists.txt

@@ -1,3 +1,3 @@
 INCLUDE(BuildPlugin)
 
-BUILD_PLUGIN(crossovereq CrossoverEQ.cpp CrossoverEQControls.cpp CrossoverEQControlDialog.cpp MOCFILES CrossoverEQControls.h CrossoverEQControlDialog.h EMBEDDED_RESOURCES artwork.png fader_bg.png fader_empty.png fader_knob2.png logo.png)
+BUILD_PLUGIN(crossovereq CrossoverEQ.cpp CrossoverEQControls.cpp CrossoverEQControlDialog.cpp MOCFILES CrossoverEQControls.h CrossoverEQControlDialog.h EMBEDDED_RESOURCES artwork.svg logo.svg)

plugins/CrossoverEQ/CrossoverEQControlDialog.cpp

@@ -31,90 +31,89 @@
 #include "LedCheckBox.h"
 #include "Knob.h"
 #include "Fader.h"
+#include "PixmapButton.h"
 
 #include <QPixmap>
+#include <QHBoxLayout>
+#include <QVBoxLayout>
 
 
 namespace lmms::gui
 {
 
 
-CrossoverEQControlDialog::CrossoverEQControlDialog( CrossoverEQControls * controls ) :
-	EffectControlDialog( controls )
+CrossoverEQControlDialog::CrossoverEQControlDialog(CrossoverEQControls *controls) :
+	EffectControlDialog(controls)
 {
-	setAutoFillBackground( true );
+	setAutoFillBackground(true);
 	QPalette pal;
-	pal.setBrush( backgroundRole(),	PLUGIN_NAME::getIconPixmap( "artwork" ) );
-	setPalette( pal );
-	setFixedSize( 167, 178 );
+	pal.setBrush(backgroundRole(), PLUGIN_NAME::getIconPixmap("artwork"));
+	setPalette(pal);
+	setFixedSize(167, 218);
+	auto layout = new QVBoxLayout(this);
+
+	auto knobsLayout = new QHBoxLayout();
+	layout->addLayout(knobsLayout);
+
+	const auto makeKnob = [this, knobsLayout](
+		FloatModel *model,
+		const QString &label,
+		const QString &txt_before
+	) {
+		auto k = new Knob(KnobType::Bright26, this);
+		k->setModel(model);
+		k->setLabel(label);
+		k->setHintText(txt_before, "Hz");
+		knobsLayout->addWidget(k, 0, Qt::AlignHCenter);
+	};
+
+	makeKnob(&controls->m_xover12, "1/2", tr("Band 1/2 crossover"));
+	makeKnob(&controls->m_xover23, "2/3", tr("Band 2/3 crossover"));
+	makeKnob(&controls->m_xover34, "3/4", tr("Band 3/4 crossover"));
 	
-	// knobs
-	auto xover12 = new Knob(KnobType::Bright26, this);
-	xover12->move( 29, 11 );
-	xover12->setModel( & controls->m_xover12 );
-	xover12->setLabel( "1/2" );
-	xover12->setHintText( tr( "Band 1/2 crossover:" ), " Hz" );
+	auto bandsLayout = new QGridLayout();
+	bandsLayout->setContentsMargins(4, 10, 4, 5);
+	layout->addLayout(bandsLayout);
 
-	auto xover23 = new Knob(KnobType::Bright26, this);
-	xover23->move( 69, 11 );
-	xover23->setModel( & controls->m_xover23 );
-	xover23->setLabel( "2/3" );
-	xover23->setHintText( tr( "Band 2/3 crossover:" ), " Hz" );
+	const auto makeFader = [this, bandsLayout](
+		FloatModel *model,
+		const QString &label,
+		int column
+	) {
+		auto f = new Fader(model, label, this, false);
+		f->setHintText(label, "dBFS");
+		f->setDisplayConversion(false);
+		f->setRenderUnityLine(false);
+		bandsLayout->addWidget(f, 0, column, Qt::AlignHCenter);
+	};
 
-	auto xover34 = new Knob(KnobType::Bright26, this);
-	xover34->move( 109, 11 );
-	xover34->setModel( & controls->m_xover34 );
-	xover34->setLabel( "3/4" );
-	xover34->setHintText( tr( "Band 3/4 crossover:" ), " Hz" );
-	
-	QPixmap const fader_knob(PLUGIN_NAME::getIconPixmap("fader_knob2"));
-	
-	// faders
-	auto gain1 = new Fader(&controls->m_gain1, tr("Band 1 gain"), this, fader_knob, false);
-	gain1->move( 7, 56 );
-	gain1->setDisplayConversion( false );
-	gain1->setHintText( tr( "Band 1 gain:" ), " dBFS" );
-	gain1->setRenderUnityLine(false);
+	makeFader(&controls->m_gain1, tr("Band 1 gain"), 0);
+	makeFader(&controls->m_gain2, tr("Band 2 gain"), 1);
+	makeFader(&controls->m_gain3, tr("Band 3 gain"), 2);
+	makeFader(&controls->m_gain4, tr("Band 4 gain"), 3);
 
-	auto gain2 = new Fader(&controls->m_gain2, tr("Band 2 gain"), this, fader_knob, false);
-	gain2->move( 47, 56 );
-	gain2->setDisplayConversion( false );
-	gain2->setHintText( tr( "Band 2 gain:" ), " dBFS" );
-	gain2->setRenderUnityLine(false);
+	const auto muteOn = embed::getIconPixmap("mute_active");
+	const auto muteOff = embed::getIconPixmap("mute_inactive");
 
-	auto gain3 = new Fader(&controls->m_gain3, tr("Band 3 gain"), this, fader_knob, false);
-	gain3->move( 87, 56 );
-	gain3->setDisplayConversion( false );
-	gain3->setHintText( tr( "Band 3 gain:" ), " dBFS" );
-	gain3->setRenderUnityLine(false);
+	const auto makeMuteBtn = [this, bandsLayout, muteOn, muteOff](
+		BoolModel *model,
+		const QString &label,
+		int column
+	) {
+		auto b = new PixmapButton(this, label);
+		b->setActiveGraphic(muteOff);
+		b->setInactiveGraphic(muteOn);
+		b->setCheckable(true);
+		b->setModel(model);
+		b->setToolTip(label);
+		bandsLayout->addWidget(b, 1, column, Qt::AlignCenter);
+	};
 
-	auto gain4 = new Fader(&controls->m_gain4, tr("Band 4 gain"), this, fader_knob, false);
-	gain4->move( 127, 56 );
-	gain4->setDisplayConversion( false );
-	gain4->setHintText( tr( "Band 4 gain:" ), " dBFS" );
-	gain4->setRenderUnityLine(false);
-	
-	// leds
-	auto mute1 = new LedCheckBox("", this, tr("Band 1 mute"), LedCheckBox::LedColor::Green);
-	mute1->move( 15, 154 );
-	mute1->setModel( & controls->m_mute1 );
-	mute1->setToolTip(tr("Mute band 1"));
-
-	auto mute2 = new LedCheckBox("", this, tr("Band 2 mute"), LedCheckBox::LedColor::Green);
-	mute2->move( 55, 154 );
-	mute2->setModel( & controls->m_mute2 );
-	mute2->setToolTip(tr("Mute band 2"));
-
-	auto mute3 = new LedCheckBox("", this, tr("Band 3 mute"), LedCheckBox::LedColor::Green);
-	mute3->move( 95, 154 );
-	mute3->setModel( & controls->m_mute3 );
-	mute3->setToolTip(tr("Mute band 3"));
-
-	auto mute4 = new LedCheckBox("", this, tr("Band 4 mute"), LedCheckBox::LedColor::Green);
-	mute4->move( 135, 154 );
-	mute4->setModel( & controls->m_mute4 );
-	mute4->setToolTip(tr("Mute band 4"));
+	makeMuteBtn(&controls->m_mute1, tr("Mute band 1"), 0);
+	makeMuteBtn(&controls->m_mute2, tr("Mute band 2"), 1);
+	makeMuteBtn(&controls->m_mute3, tr("Mute band 3"), 2);
+	makeMuteBtn(&controls->m_mute4, tr("Mute band 4"), 3);
 }
 
 
-} // namespace lmms::gui
+} // namespace lmms::gui

plugins/CrossoverEQ/CrossoverEQControls.cpp

@@ -121,4 +121,4 @@ void CrossoverEQControls::sampleRateChanged()
 }
 
 
-} // namespace lmms
+} // namespace lmms

plugins/CrossoverEQ/artwork.svg

@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" width="167" height="218">
+  <defs>
+    <linearGradient id="a">
+      <stop offset="0" stop-color="#08090c"/>
+      <stop offset="1" stop-color="#101116"/>
+    </linearGradient>
+    <linearGradient id="b">
+      <stop offset="0" stop-color="#1a1d21"/>
+      <stop offset="1" stop-color="#2d333a"/>
+    </linearGradient>
+    <linearGradient xlink:href="#a" id="c" x1="0" x2="0" y1="110" y2="0" gradientTransform="scale(1.67 1.98183)" gradientUnits="userSpaceOnUse"/>
+    <linearGradient xlink:href="#b" id="d" x1="-29.25007" x2="-29.25007" y1="118" y2="10" gradientTransform="matrix(4 0 0 1.46296 122.50031 39.87036)" gradientUnits="userSpaceOnUse"/>
+  </defs>
+  <path fill="url(#c)" d="M0 0h167v218.00001H0Z"/>
+  <rect width="156" height="158" x="5.5" y="54.5" fill="url(#d)" stroke="#000" rx="2.065" ry="2.065"/>
+  <path fill="#fff" d="M7.56445 55C6.6888 55 6 55.6888 6 56.56445v1C6 56.6888 6.6888 56 7.56445 56h151.8711C160.3112 56 161 56.6888 161 57.56445v-1C161 55.6888 160.3112 55 159.43555 55Z" opacity=".1"/>
+  <path d="M7.56445 212.00045C6.6888 212.00045 6 211.31165 6 210.436v-1c0 .87565.6888 1.56445 1.56445 1.56445h151.8711c.87565 0 1.56445-.6888 1.56445-1.56445v1c0 .87565-.6888 1.56445-1.56445 1.56445z" opacity=".3"/>
+</svg>

plugins/CrossoverEQ/logo.svg

@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" width="48" height="48">
+  <path fill="#fff" d="M7.86719 2C3.95608 2 2 3.95608 2 7.86719V40.1328C2 44.04392 3.95608 46 7.86719 46H40.1328C44.04392 46 46 44.04392 46 40.13281V7.8672C46 3.95608 44.04392 2 40.13281 2H7.8672zM24 9l15 8.4375V35.25l-5.625 2.8125L27.75 35.25v-6.5625l5.625-2.8125V20.25L24 15.5625 14.625 20.25v5.625l5.625 2.8125V35.25l-5.625 2.8125L9 35.25V17.4375L24 9z"/>
+</svg>

src/gui/widgets/Fader.cpp

@@ -73,6 +73,7 @@ SimpleTextFloat* Fader::s_textFloat = nullptr;
 Fader::Fader(FloatModel* model, const QString& name, QWidget* parent, bool modelIsLinear) :
 	QWidget(parent),
 	FloatModelView(model, this),
+	m_knobSize(embed::logicalSize(m_knob)),
 	m_modelIsLinear(modelIsLinear)
 {
 	if (s_textFloat == nullptr)
@@ -198,7 +199,7 @@ void Fader::mousePressEvent(QMouseEvent* mouseEvent)
 
 		const int localY = mouseEvent->y();
 		const auto knobLowerPosY = calculateKnobPosYFromModel();
-		const auto knobUpperPosY = knobLowerPosY - m_knob.height();
+		const auto knobUpperPosY = knobLowerPosY - m_knobSize.height();
 
 		const auto clickedOnKnob = localY >= knobUpperPosY && localY <= knobLowerPosY;
 
@@ -207,7 +208,7 @@ void Fader::mousePressEvent(QMouseEvent* mouseEvent)
 			// If the users clicked on the knob we want to compensate for the offset to the center line
 			// of the knob when dealing with mouse move events.
 			// This will make it feel like the users have grabbed the knob where they clicked.
-			const auto knobCenterPos = knobLowerPosY - (m_knob.height() / 2);
+			const auto knobCenterPos = knobLowerPosY - (m_knobSize.height() / 2);
 			m_knobCenterOffset = localY - knobCenterPos;
 
 			// In this case we also will not call setVolumeByLocalPixelValue, i.e. we do not make any immediate
@@ -361,9 +362,9 @@ int Fader::calculateKnobPosYFromModel() const
 			const auto scaledRatio = computeScaledRatio(actualDb);
 
 			// This returns results between:
-			// * m_knob.height()  for a ratio of 1
+			// * m_knobSize.height()  for a ratio of 1
 			// * height()         for a ratio of 0
-			return height() - (height() - m_knob.height()) * scaledRatio;
+			return height() - (height() - m_knobSize.height()) * scaledRatio;
 		}
 	}
 	else
@@ -375,9 +376,9 @@ int Fader::calculateKnobPosYFromModel() const
 		auto const ratio = (clampedValue - minV) / (maxV - minV);
 
 		// This returns results between:
-		// * m_knob.height()  for a ratio of 1
+		// * m_knobSize.height()  for a ratio of 1
 		// * height()         for a ratio of 0
-		return height() - (height() - m_knob.height()) * ratio;
+		return height() - (height() - m_knobSize.height()) * ratio;
 	}
 }
 
@@ -391,11 +392,11 @@ void Fader::setVolumeByLocalPixelValue(int y)
 
 	// The y parameter gives us where the mouse click went.
 	// Assume that the middle of the fader should go there.
-	int const lowerFaderKnob = y + (m_knob.height() / 2);
+	int const lowerFaderKnob = y + (m_knobSize.height() / 2);
 
 	// In some cases we need the clamped lower position of the fader knob so we can ensure
 	// that we only set allowed values in the model range.
-	int const clampedLowerFaderKnob = std::clamp(lowerFaderKnob, m_knob.height(), height());
+	int const clampedLowerFaderKnob = std::clamp(lowerFaderKnob, m_knobSize.height(), height());
 
 	if (modelIsLinear())
 	{
@@ -411,7 +412,7 @@ void Fader::setVolumeByLocalPixelValue(int y)
 
 			// First map the lower knob position to [0, 1] so that we can apply some curve mapping, e.g.
 			// square, cube, etc.
-			LinearMap<float> knobMap(float(m_knob.height()), 1., float(height()), 0.);
+			LinearMap<float> knobMap(float(m_knobSize.height()), 1., float(height()), 0.);
 
 			// Apply the inverse of what is done in calculateKnobPosYFromModel
 			auto const knobPos = std::pow(knobMap.map(clampedLowerFaderKnob), 1./c_dBScalingExponent);
@@ -433,7 +434,7 @@ void Fader::setVolumeByLocalPixelValue(int y)
 	}
 	else
 	{
-		LinearMap<float> valueMap(float(m_knob.height()), model()->maxValue(), float(height()), model()->minValue());
+		LinearMap<float> valueMap(float(m_knobSize.height()), model()->maxValue(), float(height()), model()->minValue());
 
 		model()->setValue(valueMap.map(clampedLowerFaderKnob));
 	}
@@ -553,7 +554,7 @@ void Fader::paintEvent(QPaintEvent* ev)
 	}
 
 	// Draw the knob
-	painter.drawPixmap((width() - m_knob.width()) / 2, calculateKnobPosYFromModel() - m_knob.height(), m_knob);
+	painter.drawPixmap((width() - m_knobSize.width()) / 2, calculateKnobPosYFromModel() - m_knobSize.height(), m_knob);
 }
 
 void Fader::paintLevels(QPaintEvent* ev, QPainter& painter, bool linear)
@@ -727,7 +728,7 @@ void Fader::paintFaderTicks(QPainter& painter)
 	for (float i = startValue; i >= c_faderMinDb; i-= stepSize)
 	{
 		const auto scaledRatio = computeScaledRatio(i);
-		const auto maxHeight = height() - (height() - m_knob.height()) * scaledRatio - (m_knob.height() / 2);
+		const auto maxHeight = height() - (height() - m_knobSize.height()) * scaledRatio - (m_knobSize.height() / 2);
 
 		if (approximatelyEqual(i, 0.))
 		{

src/gui/widgets/PixmapButton.cpp

@@ -102,17 +102,13 @@ void PixmapButton::mouseDoubleClickEvent( QMouseEvent * _me )
 }
 
 
-
-
-void PixmapButton::setActiveGraphic( const QPixmap & _pm )
+void PixmapButton::setActiveGraphic(const QPixmap &pm)
 {
-	m_activePixmap = _pm;
-	resize(m_activePixmap.size() / m_activePixmap.devicePixelRatio());
+	m_activePixmap = pm;
+	resize(embed::logicalSize(m_activePixmap));
 }
 
 
-
-
 void PixmapButton::setInactiveGraphic( const QPixmap & _pm, bool _update )
 {
 	m_inactivePixmap = _pm;
@@ -129,9 +125,8 @@ QSize PixmapButton::sizeHint() const
 
 QSize PixmapButton::minimumSizeHint() const
 {
-	const auto activeSize = m_activePixmap.size() / m_activePixmap.devicePixelRatio();
-	const auto inactiveSize = m_inactivePixmap.size() / m_inactivePixmap.devicePixelRatio();
-	return activeSize.expandedTo(inactiveSize);
+	return embed::logicalSize(m_activePixmap)
+		.expandedTo(embed::logicalSize(m_inactivePixmap));
 }
 
 bool PixmapButton::isActive() const