[vlmc-devel] [PATCH] Refactored some qt4 connects to qt5 connects, added some FIXME's about invalid connections (not portable to qt5)

Hugo Beauzée-Luyssen hugo at beauzee.fr
Mon Mar 14 21:13:00 CET 2016


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


More information about the Vlmc-devel mailing list