Commit f1fbb54a authored by Michael Pyne's avatar Michael Pyne
Browse files

slider: Simplify slider handling to fix seeking and clazy warning.

JuK playback/seeking has been sort of busted in some ways for all of
KF5. While trying to fix a few clazy warnings about sliderReleased being
overloaded to a parent Qt class, I realized that most of the code we
have for slider handling is needless, especially with Qt5 (I'm assuming
it made more sense in the Qt3 timeframe it descended from).

Rather than fighting Qt, just use the slider classes like they are meant
to be used. I keep the custom paint handler for now but that may go
later as well.

The hard part is keeping the PlayerManager (which is controlling the
position of the slider when the user is not) from fighting with the user
or overriding the user's inputs. I'm not sure it's perfect but it's much
better in my testing than it was before... although MPRIS is still
buggy.
parent 7d98cf3d
Pipeline #51895 passed with stage
in 11 minutes and 14 seconds
......@@ -2,6 +2,7 @@
* Copyright (c) 2003-2009 Mark Kretschmann <kretschmann@kde.org>
* Copyright (c) 2005 Gabor Lehel <illissius@gmail.com>
* Copyright (c) 2008 Dan Meltzer <parallelgrapefruit@gmail.com>
* Copyright (c) 2021 Michael Pyne <mpyne@kde.org>
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
......@@ -32,10 +33,6 @@
Slider::Slider( Qt::Orientation orientation, uint max, QWidget *parent )
: QSlider( orientation, parent )
, m_sliding( false )
, m_outside( false )
, m_prevValue( 0 )
, m_needsResize( true )
{
setMouseTracking( true );
setRange( 0, max );
......@@ -90,98 +87,6 @@ Slider::wheelEvent( QWheelEvent *e )
nval = qMin(nval, maximum());
QSlider::setValue( nval );
emit sliderReleased( value() );
}
void
Slider::mouseMoveEvent( QMouseEvent *e )
{
if ( m_sliding )
{
//feels better, but using set value of 20 is bad of course
QRectF rect( -20, -20, width()+40, height()+40 );
if ( orientation() == Qt::Horizontal && !rect.contains( e->pos() ) )
{
if ( !m_outside )
{
QSlider::setValue( m_prevValue );
//if mouse released outside of slider, emit sliderMoved to previous value
emit sliderMoved( m_prevValue );
}
m_outside = true;
}
else
{
m_outside = false;
slideEvent( e );
emit sliderMoved( value() );
}
}
else
QSlider::mouseMoveEvent( e );
}
void
Slider::slideEvent( QMouseEvent *e )
{
QRectF knob;
if ( maximum() > minimum() )
knob = sliderHandleRect( rect(), ((qreal)value()) / ( maximum() - minimum() ) );
int position;
int span;
if( orientation() == Qt::Horizontal )
{
position = e->pos().x() - knob.width() / 2;
span = width() - knob.width();
}
else
{
position = e->pos().y() - knob.height() / 2;
span = height() - knob.height();
}
const bool inverse = ( orientation() == Qt::Horizontal ) ?
( invertedAppearance() != (layoutDirection() == Qt::RightToLeft) ) :
( !invertedAppearance() );
const int val = QStyle::sliderValueFromPosition( minimum(), maximum(), position, span, inverse );
QSlider::setValue( val );
}
void
Slider::mousePressEvent( QMouseEvent *e )
{
m_sliding = true;
m_prevValue = value();
QRectF knob;
if ( maximum() > minimum() )
knob = sliderHandleRect( rect(), ((qreal)value()) / ( maximum() - minimum() ) );
if ( !knob.contains( e->pos() ) )
mouseMoveEvent( e );
}
void
Slider::mouseReleaseEvent( QMouseEvent* )
{
if( !m_outside && value() != m_prevValue )
emit sliderReleased( value() );
m_sliding = false;
m_outside = false;
}
void
Slider::setValue( int newValue )
{
//don't adjust the slider while the user is dragging it!
if ( !m_sliding || m_outside )
QSlider::setValue( newValue );
else
m_prevValue = newValue;
}
void Slider::paintCustomSlider( QPainter *p )
......@@ -191,7 +96,7 @@ void Slider::paintCustomSlider( QPainter *p )
percent = ((qreal)value()) / ( maximum() - minimum() );
QStyleOptionSlider opt;
initStyleOption( &opt );
if ( m_sliding ||
if ( this->isSliderDown() ||
( underMouse() && sliderHandleRect( rect(), percent ).contains( mapFromGlobal(QCursor::pos()) ) ) )
{
opt.activeSubControls |= QStyle::SC_SliderHandle;
......@@ -211,21 +116,8 @@ VolumeSlider::VolumeSlider( uint max, QWidget *parent, bool customStyle )
setFocusPolicy( Qt::NoFocus );
setInvertedAppearance( false );
setInvertedControls( false );
connect( this, SIGNAL(sliderMoved(int)),
this, SLOT(emitVolumeChanged(int)) );
connect( this, SIGNAL(sliderReleased(int)),
this, SLOT(emitVolumeChanged(int)) );
}
void
VolumeSlider::mousePressEvent( QMouseEvent *e )
{
if( e->button() != Qt::RightButton )
{
Slider::mousePressEvent( e );
slideEvent( e );
}
connect(this, &QAbstractSlider::valueChanged, this, &VolumeSlider::emitVolumeChanged);
}
void
......@@ -246,8 +138,7 @@ VolumeSlider::contextMenuEvent( QContextMenuEvent *e )
const int n = a->data().toInt();
if( n >= 0 )
{
QSlider::setValue( n );
emit volumeChanged( float( n ) / float( maximum() ) );
this->setValue( n );
}
}
}
......@@ -257,9 +148,7 @@ VolumeSlider::wheelEvent( QWheelEvent *e )
{
static const int volumeSensitivity = 30;
const uint step = e->angleDelta().y() / volumeSensitivity;
QSlider::setValue( QSlider::value() + step );
emit volumeChanged( float( value() ) / float( maximum() ) );
this->setValue( this->value() + step );
}
void
......@@ -273,7 +162,7 @@ VolumeSlider::paintEvent( QPaintEvent *event )
return;
}
QSlider::paintEvent( event );
Slider::paintEvent( event );
}
void
......@@ -295,12 +184,6 @@ TimeSlider::TimeSlider( QWidget *parent )
setFocusPolicy( Qt::NoFocus );
}
void
TimeSlider::setSliderValue( int value )
{
Slider::setValue( value );
}
void
TimeSlider::paintEvent( QPaintEvent *pe )
{
......@@ -308,7 +191,6 @@ TimeSlider::paintEvent( QPaintEvent *pe )
p.setClipRegion( pe->region() );
paintCustomSlider( &p );
p.end();
}
void TimeSlider::sliderChange( SliderChange change )
......@@ -330,9 +212,3 @@ void TimeSlider::sliderChange( SliderChange change )
else
Slider::sliderChange( change ); // calls update()
}
void TimeSlider::mousePressEvent( QMouseEvent *event )
{
// We should probably eat this event if we're not able to seek
Slider::mousePressEvent( event );
}
......@@ -2,6 +2,7 @@
* Copyright (c) 2003-2009 Mark Kretschmann <kretschmann@kde.org>
* Copyright (c) 2005 Gabor Lehel <illissius@gmail.com>
* Copyright (c) 2008 Dan Meltzer <parallelgrapefruit@gmail.com>
* Copyright (c) 2021 Michael Pyne <mpyne@kde.org>
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
......@@ -34,25 +35,12 @@ class Slider : public QSlider
public:
explicit Slider( Qt::Orientation, uint max = 0, QWidget* parent = 0 );
virtual void setValue( int );
signals:
//we emit this when the user has specifically changed the slider
//so connect to it if valueChanged() is too generic
//Qt also emits valueChanged( int )
void sliderReleased( int );
protected:
virtual void wheelEvent( QWheelEvent* ) override;
virtual void mouseMoveEvent( QMouseEvent* ) override;
virtual void mouseReleaseEvent( QMouseEvent* ) override;
virtual void mousePressEvent( QMouseEvent* ) override;
virtual void slideEvent( QMouseEvent* );
QRectF sliderHandleRect( const QRectF &slider, qreal percent ) const;
void paintCustomSlider( QPainter *p );
bool m_sliding;
bool m_usingCustomStyle;
static const int s_borderWidth = 6;
......@@ -62,9 +50,6 @@ protected:
static const int s_sliderInsertY = 5;
private:
bool m_outside;
int m_prevValue;
bool m_needsResize;
QPixmap m_topLeft;
QPixmap m_topRight;
QPixmap m_top;
......@@ -89,7 +74,6 @@ class VolumeSlider : public Slider
protected:
virtual void paintEvent( QPaintEvent* ) override;
virtual void mousePressEvent( QMouseEvent* ) override;
virtual void contextMenuEvent( QContextMenuEvent* ) override;
signals:
......@@ -108,10 +92,8 @@ class TimeSlider : public Slider
public:
explicit TimeSlider( QWidget *parent );
void setSliderValue( int value );
protected:
virtual void paintEvent( QPaintEvent* ) override;
virtual void mousePressEvent( QMouseEvent* ) override;
virtual void sliderChange( SliderChange change ) override;
private:
......
/**
* Copyright (C) 2002-2004 Scott Wheeler <wheeler@kde.org>
* Copyright (C) 2021 Michael Pyne <mpyne@kde.org>
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
......@@ -40,21 +41,49 @@ TrackPositionAction::TrackPositionAction(const QString &text, QObject *parent) :
QWidget *TrackPositionAction::createWidget(QWidget *parent)
{
Slider *slider = new TimeSlider(parent);
TimeSlider *slider = new TimeSlider(parent);
slider->setObjectName(QLatin1String("timeSlider"));
PlayerManager *player = JuK::JuKInstance()->playerManager();
// When user starts dragging slider we may have tick events in event loop
// that will set the position back so stop updating until the user is done.
connect(slider, &TimeSlider::sliderPressed, this, [player, slider]() {
QObject::disconnect(player, &PlayerManager::tick, slider, nullptr);
});
connect(slider, &TimeSlider::sliderReleased, this, [player, slider]() {
const auto newValue = slider->value();
player->seek(newValue);
slider->setValue(newValue);
connect(player, &PlayerManager::tick, slider, &TimeSlider::setValue);
});
// TODO only connect the tick signal when actually visible
connect(player, &PlayerManager::tick, slider, &Slider::setValue);
connect(player, &PlayerManager::tick, slider, &TimeSlider::setValue);
connect(slider, &TimeSlider::actionTriggered, this, [player, slider](int action) {
if(action == QAbstractSlider::SliderMove) {
return;
}
// Set the new position before the PlayerManager overwrites it
// again
player->seek(slider->sliderPosition());
});
connect(player, &PlayerManager::seekableChanged, slider, &Slider::setEnabled);
connect(player, &PlayerManager::seekableChanged, slider, [slider](bool seekable) {
static const QString noSeekMsg =
i18n("Seeking is not supported in this file with your audio settings.");
slider->setToolTip(seekable ? QString() : noSeekMsg);
});
connect(player, &PlayerManager::totalTimeChanged, slider, &Slider::setMaximum);
connect(slider, &Slider::sliderReleased, player, &PlayerManager::seek);
connect(player, &PlayerManager::totalTimeChanged, slider, [slider](int newLength) {
slider->setMaximum(newLength);
slider->setPageStep(newLength / 10);
slider->setSingleStep(qMin(5000, newLength / 20));
});
return slider;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment