[vlmc-devel] [PATCH] Refactored some qt4 connects to qt5 connects, added some FIXME's about invalid connections (not portable to qt5)
Paweł Wegner
pawel.wegner95 at gmail.com
Mon Mar 14 21:29:15 CET 2016
Hi,
It has to be ::Clip because Clip would correspond to Commands::Clip in that
scope and that would result in a compilation error.
The temporary variable was needed because in the following connect if i had
written simply &Transcoder::progress, then compiler would find it ambiguous
because Transcoder::progress
can have either int or float as a parameter.
Should i submit a patch which changes void (NotificationZone::*f)(float) to
auto?
2016-03-14 21:13 GMT+01:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
> Hi,
>
> On 03/12/2016 01:30 PM, Paweł Wegner wrote:
> > ---
> > src/Commands/Commands.cpp | 11 +++++++----
> > src/Commands/KeyboardShortcutHelper.cpp | 4 ++--
> > src/Library/Library.cpp | 3 +++
> > src/Main/Core.cpp | 6 +++---
> > src/Main/main.cpp | 1 +
> > src/Media/Clip.cpp | 4 ++--
> > src/Media/Transcoder.cpp | 13 +++++++------
> > 7 files changed, 25 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/Commands/Commands.cpp b/src/Commands/Commands.cpp
> > index 9351107..bfa291f 100644
> > --- a/src/Commands/Commands.cpp
> > +++ b/src/Commands/Commands.cpp
> > @@ -43,6 +43,9 @@ Commands::Generic::Generic() :
> > {
> > //This is connected using a direct connection to ensure the view
> can be refreshed
> > //just after the signal has been emited.
> > +
> > + //FIXME: there is no signal retranslateRequired in QUndoStack class
> > + // <3 qt4 connects
> > connect( Core::getInstance()->undoStack(), SIGNAL(
> retranslateRequired() ),
> > this, SLOT( retranslate() ), Qt::DirectConnection );
> > }
> > @@ -80,7 +83,7 @@ Commands::Clip::Add::Add( ClipHelper* ch,
> TrackWorkflow* tw, qint64 pos ) :
> > m_trackWorkflow( tw ),
> > m_pos( pos )
> > {
> > - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT(
> invalidate() ) );
> > + connect( ch->clip(), &::Clip::destroyed, this, &Add::invalidate );
>
> Why do you prepend by a scope resolution operator? Why not simply
> Clip::destroyed?
> Same question goes for below
>
> > retranslate();
> > }
> >
> > @@ -158,7 +161,7 @@ Commands::Clip::Move::internalUndo()
> > Commands::Clip::Remove::Remove( ClipHelper* ch, TrackWorkflow* tw ) :
> > m_clipHelper( ch ), m_trackWorkflow( tw )
> > {
> > - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT(
> invalidate() ) );
> > + connect( ch->clip(), &::Clip::destroyed, this, &Remove::invalidate
> );
> > retranslate();
> > m_pos = tw->getClipPosition( ch->uuid() );
> > }
> > @@ -189,7 +192,7 @@ Commands::Clip::Resize::Resize( TrackWorkflow* tw,
> ClipHelper* ch, qint64 newBeg
> > m_newEnd( newEnd ),
> > m_newPos( newPos )
> > {
> > - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT(
> invalidate() ) );
> > + connect( ch->clip(), &::Clip::destroyed, this, &Resize::invalidate
> );
> > m_oldBegin = ch->begin();
> > m_oldEnd = ch->end();
> > m_oldPos = tw->getClipPosition( ch->uuid() );
> > @@ -230,7 +233,7 @@ Commands::Clip::Split::Split( TrackWorkflow *tw,
> ClipHelper *toSplit,
> > m_newClipPos( newClipPos ),
> > m_newClipBegin( newClipBegin )
> > {
> > - connect( toSplit->clip(), SIGNAL( destroyed() ), this, SLOT(
> invalidate() ) );
> > + connect( toSplit->clip(), &::Clip::destroyed, this,
> &Split::invalidate );
> > m_newClip = new ClipHelper( toSplit->clip(), newClipBegin,
> toSplit->end() );
> > m_oldEnd = toSplit->end();
> > retranslate();
> > diff --git a/src/Commands/KeyboardShortcutHelper.cpp
> b/src/Commands/KeyboardShortcutHelper.cpp
> > index 58a834c..eb76bb4 100644
> > --- a/src/Commands/KeyboardShortcutHelper.cpp
> > +++ b/src/Commands/KeyboardShortcutHelper.cpp
> > @@ -33,7 +33,7 @@ KeyboardShortcutHelper::KeyboardShortcutHelper( const
> QString& name, QWidget* pa
> > {
> > SettingValue* setting = Core::getInstance()->settings()->value(
> name );
> > setKey( QKeySequence( setting->get().toString() ) );
> > - connect( setting, SIGNAL( changed( QVariant ) ), this, SLOT(
> shortcutUpdated( const QVariant& ) ) );
> > + connect( setting, &SettingValue::changed, this,
> &KeyboardShortcutHelper::shortcutUpdated );
> > }
> >
> > KeyboardShortcutHelper::KeyboardShortcutHelper( const QString& name,
> QAction *action,
> > @@ -44,7 +44,7 @@ KeyboardShortcutHelper::KeyboardShortcutHelper( const
> QString& name, QAction *ac
> > {
> > SettingValue* setting = Core::getInstance()->settings()->value(
> name );
> > action->setShortcut( setting->get().toString() );
> > - connect( setting, SIGNAL( changed( QVariant ) ), this, SLOT(
> shortcutUpdated( const QVariant& ) ) );
> > + connect( setting, &SettingValue::changed, this,
> &KeyboardShortcutHelper::shortcutUpdated );
> > }
> >
> > void
> > diff --git a/src/Library/Library.cpp b/src/Library/Library.cpp
> > index 5e102a3..9685280 100644
> > --- a/src/Library/Library.cpp
> > +++ b/src/Library/Library.cpp
> > @@ -114,6 +114,9 @@ Library::mediaLoaded( const Media* media )
> > {
> > if ( media != NULL )
> > {
> > + //FIXME: metaDataComputed in Media class has no arguments, this
> shouldn't be working;
> > + //replacing this with disconnect( media,
> &Media::metaDataComputed, this, &Library::mediaLoaded );
> > + //produces static_assert error
> > disconnect( media, SIGNAL( metaDataComputed( const Media* ) ),
> > this, SLOT( mediaLoaded( const Media* ) ) );
> > }
> > diff --git a/src/Main/Core.cpp b/src/Main/Core.cpp
> > index a9fcf0b..7145478 100644
> > --- a/src/Main/Core.cpp
> > +++ b/src/Main/Core.cpp
> > @@ -55,9 +55,9 @@ Core::Core()
> > m_library = new Library( m_workspace );
> > m_currentProject = new Project( m_settings );
> >
> > - connect( m_undoStack, SIGNAL( cleanChanged( bool ) ),
> m_currentProject, SLOT( cleanChanged( bool ) ) );
> > - connect( m_currentProject, SIGNAL( projectSaved() ), m_undoStack,
> SLOT( setClean() ) );
> > - connect( m_library, SIGNAL( cleanStateChanged( bool ) ),
> m_currentProject, SLOT( libraryCleanChanged( bool ) ) );
> > + connect( m_undoStack, &QUndoStack::cleanChanged, m_currentProject,
> &Project::cleanChanged );
> > + connect( m_currentProject, &Project::projectSaved, m_undoStack,
> &QUndoStack::setClean );
> > + connect( m_library, &Library::cleanStateChanged, m_currentProject,
> &Project::libraryCleanChanged );
> > connect( m_currentProject, SIGNAL( projectLoaded( QString, QString
> ) ),
> > m_recentProjects, SLOT( projectLoaded( QString, QString )
> ) );
> > }
> > diff --git a/src/Main/main.cpp b/src/Main/main.cpp
> > index f2e9b49..6003d7b 100644
> > --- a/src/Main/main.cpp
> > +++ b/src/Main/main.cpp
> > @@ -193,6 +193,7 @@ VLMCCoremain( int argc, char **argv )
> > ConsoleRenderer renderer;
> > ProjectManager *pm = Core::getInstance()->currentProject();
> >
> > + //FIXME: only signal projectLoaded( const QString& ) in
> ProjectManager
> > QCoreApplication::connect( pm, SIGNAL( projectLoaded() ),
> &renderer, SLOT( startRender() ) );
> > pm->loadProject( app.arguments()[1] );
> > #endif
> > diff --git a/src/Media/Clip.cpp b/src/Media/Clip.cpp
> > index 2c4a491..94535b3 100644
> > --- a/src/Media/Clip.cpp
> > +++ b/src/Media/Clip.cpp
> > @@ -47,8 +47,8 @@ Clip::Clip( Media *media, qint64 begin /*= 0*/, qint64
> end /*= -1*/, const QStri
> > m_childs = new MediaContainer( this );
> > m_rootClip = media->baseClip();
> > computeLength();
> > - connect( media, SIGNAL( metaDataComputed() ),
> > - this, SLOT( mediaMetadataUpdated() ) );
> > + connect( media, &Media::metaDataComputed,
> > + this, &Clip::mediaMetadataUpdated );
> > }
> >
> > Clip::Clip( Clip *parent, qint64 begin /*= -1*/, qint64 end /*= -1*/,
> > diff --git a/src/Media/Transcoder.cpp b/src/Media/Transcoder.cpp
> > index 1357329..7640d31 100644
> > --- a/src/Media/Transcoder.cpp
> > +++ b/src/Media/Transcoder.cpp
> > @@ -36,10 +36,11 @@ Transcoder::Transcoder( Media* media )
> > : m_media( media )
> > , m_renderer( NULL )
> > {
> > - connect( this, SIGNAL( notify( QString ) ),
> > - NotificationZone::getInstance(), SLOT( notify( QString ) )
> );
> > - connect( this, SIGNAL( progress( float ) ),
> > - NotificationZone::getInstance(), SLOT( progressUpdated(
> float ) ) );
> > + connect( this, &Transcoder::notify,
> > + NotificationZone::getInstance(), &NotificationZone::notify
> );
> > + void (NotificationZone::*f)(float) =
> &NotificationZone::progressUpdated;
>
> Why the temporary variable? Also, feel free to use "auto" ;)
>
> > + connect( this, &Transcoder::progress,
> > + NotificationZone::getInstance(), f );
> > m_eventWatcher = new RendererEventWatcher;
> > }
> >
> > @@ -62,8 +63,8 @@ Transcoder::transcodeToPs()
> > m_destinationFile = outputDir + '/' +
> m_media->fileInfo()->baseName() + ".ps";
> > m_renderer->setOutputFile( qPrintable( m_destinationFile ) );
> > m_renderer->setName( qPrintable( QString( "Transcoder " ) +
> m_media->fileInfo()->baseName() ) );
> > - connect( m_eventWatcher, SIGNAL( positionChanged( float ) ), this,
> SIGNAL( progress( float ) ) );
> > - connect( m_eventWatcher, SIGNAL( endReached() ), this, SLOT(
> transcodeFinished() ) );
> > + connect( m_eventWatcher, &RendererEventWatcher::positionChanged,
> this, &Transcoder::progress );
> > + connect( m_eventWatcher, &RendererEventWatcher::endReached, this,
> &Transcoder::transcodeFinished );
> > emit notify( "Transcoding " +
> m_media->fileInfo()->absoluteFilePath() + " to " + m_destinationFile );
> > m_renderer->start();
> > }
> >
>
> Except for those remarks, looks good to me!
>
> Regards,
>
> --
> Hugo Beauzée-Luyssen
> www.beauzee.fr
> _______________________________________________
> Vlmc-devel mailing list
> Vlmc-devel at videolan.org
> https://mailman.videolan.org/listinfo/vlmc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlmc-devel/attachments/20160314/e18beba2/attachment-0001.html>
More information about the Vlmc-devel
mailing list