From 19c4bfcf4f422b09da9c98059a95b9c1a3780b1a Mon Sep 17 00:00:00 2001 From: Jonathan Marten Date: Thu, 1 Oct 2020 15:16:51 +0100 Subject: [PATCH] Use the configured Volume Step value for slider page steps This makes all operations which move the volume in steps - hot keys, mouse wheel over the system tray icon, and slider page steps - use the same step value. Improve the interface between the KMix application and the Volume class, by adding Volume::setVolumeStep() to set the value. There is no need for a getter function since a value can be obtained from Volume::volumeStep(). Eliminates some public static variables that looked like constants. Add an explanatory tool tip to the Volume Step configuration spin box. --- apps/kmix.cpp | 13 +++--------- core/volume.cpp | 49 +++++++++++++++++++++++++++++++-------------- core/volume.h | 6 +++--- gui/kmixprefdlg.cpp | 15 +++++++++++--- gui/mdwslider.cpp | 20 ++++++++++++------ 5 files changed, 66 insertions(+), 37 deletions(-) diff --git a/apps/kmix.cpp b/apps/kmix.cpp index 4ace0ba5..4c6aed81 100644 --- a/apps/kmix.cpp +++ b/apps/kmix.cpp @@ -510,16 +510,9 @@ void KMixWindow::loadBaseConfig() QString mixerIgnoreExpression = config.readEntry("MixerIgnoreExpression", "Modem"); MixerToolBox::setMixerIgnoreExpression(mixerIgnoreExpression); - // --- Advanced options, without GUI: START ------------------------------------- - QString volumePercentageStepString = config.readEntry("VolumePercentageStep"); - if (!volumePercentageStepString.isNull()) - { - float volumePercentageStep = volumePercentageStepString.toFloat(); - if (volumePercentageStep > 0 && volumePercentageStep <= 100) - Volume::VOLUME_STEP_DIVISOR = (100 / volumePercentageStep); - } - - // --- Advanced options, without GUI: END ------------------------------------- + // The global volume step setting. + const int volumePercentageStep = config.readEntry("VolumePercentageStep", -1); + if (volumePercentageStep>0) Volume::setVolumeStep(volumePercentageStep); // The following log is very helpful in bug reports. Please keep it. m_backendFilter = config.readEntry<>("Backends", QList()); diff --git a/core/volume.cpp b/core/volume.cpp index 5815df2e..afdac7aa 100644 --- a/core/volume.cpp +++ b/core/volume.cpp @@ -27,8 +27,19 @@ #include -float Volume::VOLUME_STEP_DIVISOR = 20; -float Volume::VOLUME_PAGESTEP_DIVISOR = 10; + +// This value is the number of steps to take the volume from minimum to maximum. +// It applies to changing the volume by hot key, or by the mouse wheel over the +// system tray icon. For consistency, it is also used as the page step for +// volume sliders (clicking in the blank area on each side of the slider, or +// the PgUp/PgDown keys when the slider has focus). The slider single step +// (dragging the slider, or using the arrow keys) is not changed from the default +// of 1, which gives one hardware volume step. +// +// The default set here is 5% which means 20 steps. It can be changed by the +// "Volume Step" option in the KMix settings. + +static float s_volumeStepDivisor = (100.0/5); static Volume::ChannelMask channelMask[] = @@ -164,27 +175,35 @@ QMap Volume::getVolumes() const } /** - * Returns the absolute change to do one "step" for this volume. This is similar to a page step in a slider, - * namely a fixed percentage of the range. - * One step is the percentage given by 100/VOLUME_STEP_DIVISOR. The - * default VOLUME_STEP_DIVISOR is 20, so default change is 5% of the volumeSpan(). + * Returns the absolute change to do one "step" for this volume. * - * This method guarantees a minimum absolute change of 1, zero is never returned. + * This is similar to a page step in a slider, namely a fixed percentage + * of the range. + * One step is the percentage given by 100/VOLUME_STEP_DIVISOR. + * The default divisor is 20, so the default change is 5% of the volumeSpan(). * - * It is NOT verified, that such a volume change would actually be possible. You might hit the upper or lower bounds - * of the volume range. + * This method guarantees a minimum absolute change of 1, zero is never returned. * + * It is NOT verified, that such a volume change would actually be possible. + * You might hit the upper or lower bounds of the volume range. * - * @param decrease true, if you want a volume step that decreases the volume by one page step + * @param decrease @c true if you want a volume step that decreases the volume by one page step * @return The volume step. It will be negative if you have used decrease==true - * */ long Volume::volumeStep(bool decrease) const { - long inc = volumeSpan() / Volume::VOLUME_STEP_DIVISOR; - if ( inc == 0 ) inc = 1; - if ( decrease ) inc *= -1; - return inc; + long inc = qRound(volumeSpan()/s_volumeStepDivisor); + if (inc==0) inc = 1; + if (decrease) inc = -inc; + return (inc); +} + + +/* static */ void Volume::setVolumeStep(int percent) +{ + if (percent<=0 || percent>=100) return; + s_volumeStepDivisor = 100.0/percent; + qDebug() << "percent" << percent << "-> divisor" << s_volumeStepDivisor; } diff --git a/core/volume.h b/core/volume.h index c20f1e75..631a755e 100644 --- a/core/volume.h +++ b/core/volume.h @@ -161,13 +161,13 @@ public: QMap getVolumesWhenActive() const; long volumeStep(bool decrease) const; - static float VOLUME_STEP_DIVISOR; // The divisor for defining volume control steps (for mouse-wheel, DBUS and Normal step for Sliders ) - static float VOLUME_PAGESTEP_DIVISOR; // The divisor for defining volume control steps (page-step for sliders) + // Sets the value from the GUI configuration. This affects all volume + // increment or decrement operations. + static void setVolumeStep(int percent); protected: long _chmask; QMap _volumesL; -// QMap _volumesMuted; long _minVolume; long _maxVolume; diff --git a/gui/kmixprefdlg.cpp b/gui/kmixprefdlg.cpp index 28399057..b17768fa 100644 --- a/gui/kmixprefdlg.cpp +++ b/gui/kmixprefdlg.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -231,6 +232,14 @@ void KMixPrefDlg::createGeneralTab() // Register SpinBox for KConfig m_volumeStep->setObjectName("kcfg_VolumePercentageStep"); + m_volumeStep->setToolTip(xi18nc("@info:tooltip", + "Set the volume step as a percentage of the volume range." + "This affects changing the volume via hot keys, " + "with the mouse wheel over the system tray icon, " + "or moving sliders by a page step." + "%1 must be restarted for this change to take effect.", + QGuiApplication::applicationDisplayName())); + horizontalGrid->addWidget(new QLabel(i18n("Volume step:"), m_generalTab), 0, 0, Qt::AlignLeft); horizontalGrid->addWidget(m_volumeStep, 0, 1, Qt::AlignLeft); horizontalGrid->addItem(new QSpacerItem(1 ,1 , QSizePolicy::Expanding), 0, 2); @@ -240,7 +249,8 @@ void KMixPrefDlg::createGeneralTab() // Volume Step and Overdrive Warning volumeOverdriveWarning = new KMessageWidget( - i18n("KMix must be restarted for the Volume Step and Overdrive settings to take effect."), grp); + i18n("%1 must be restarted for the Volume Step and Overdrive settings to take effect.", + QGuiApplication::applicationDisplayName()), grp); volumeOverdriveWarning->setIcon(QIcon::fromTheme("dialog-information")); volumeOverdriveWarning->setMessageType(KMessageWidget::Information); volumeOverdriveWarning->setCloseButtonVisible(false); @@ -340,8 +350,7 @@ void KMixPrefDlg::addWidgetToLayout(QWidget* widget, QBoxLayout* layout, int spa */ void KMixPrefDlg::updateWidgets() { - if (dialogConfig.data.debugConfig) - qCDebug(KMIX_LOG) << ""; + if (dialogConfig.data.debugConfig) qCDebug(KMIX_LOG); bool toplevelHorizontal = dialogConfig.data.getToplevelOrientation() == Qt::Horizontal; _rbHorizontal->setChecked(toplevelHorizontal); _rbVertical->setChecked(!toplevelHorizontal); diff --git a/gui/mdwslider.cpp b/gui/mdwslider.cpp index 6e9be0ef..6ef977b2 100644 --- a/gui/mdwslider.cpp +++ b/gui/mdwslider.cpp @@ -605,8 +605,8 @@ void MDWSlider::addSliders( QBoxLayout *volLayout, char type, Volume& vol, QList& ref_sliders, const QString &tooltipText) { const int minSliderSize = fontMetrics().height() * 10; - long minvol = vol.minVolume(); - long maxvol = vol.maxVolume(); + const long minvol = vol.minVolume(); + const long maxvol = vol.maxVolume(); const QMap vols = vol.getVolumes(); for (const VolumeChannel &vc : vols) @@ -623,10 +623,18 @@ void MDWSlider::addSliders( QBoxLayout *volLayout, char type, Volume& vol, VolumeSlider *slider = new VolumeSlider(orientation(), this); slider->setMinimum(minvol); slider->setMaximum(maxvol); - slider->setPageStep(maxvol / Volume::VOLUME_PAGESTEP_DIVISOR); - slider->setValue( vol.getVolume( vc.chid ) ); - volumeValues.push_back( vol.getVolume( vc.chid ) ); - + + // Set the slider page step to be the same as the configured volume step. + // If that volume step is the minimum possible value (1), then set it + // to 2 so that it is bigger than the single step. + int pageStep = vol.volumeStep(false); + if (pageStep==1) pageStep = 2; + slider->setPageStep(pageStep); + // Don't show too many tick marks if the page step is small. + if (pageStep<10) slider->setTickInterval(qRound((maxvol-minvol)/10.0)); + + slider->setValue(vol.getVolume(vc.chid)); + volumeValues.push_back(vol.getVolume(vc.chid)); if (orientation()==Qt::Vertical) slider->setMinimumHeight(minSliderSize); else slider->setMinimumWidth(minSliderSize); -- GitLab