----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118755/#review60466 -----------------------------------------------------------
src/lib/marble/ParallelTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42200> no need for this #include src/lib/marble/ParallelTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42186> did you mean to write m_delayBeforeTrackStarts = 0? src/lib/marble/ParallelTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42208> please remove (don't change the playback state in seek). Sound currently starts playing in pause mode when you seek. src/lib/marble/ParallelTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42209> please remove (don't change the playback state in seek) src/lib/marble/ParallelTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42210> please remove (don't change the playback state in seek) src/lib/marble/PlaybackSoundCueItem.h <https://git.reviewboard.kde.org/r/118755/#comment42187> #include "config-phonon.h" src/lib/marble/PlaybackSoundCueItem.cpp <https://git.reviewboard.kde.org/r/118755/#comment42188> I'd use #else return 0; #endif here to avoid that compilers/code checkers see unreachable code src/lib/marble/PlaybackSoundCueItem.cpp <https://git.reviewboard.kde.org/r/118755/#comment42199> m_mediaObject->stop(); src/lib/marble/SerialTrack.cpp <https://git.reviewboard.kde.org/r/118755/#comment42193> Will only be the case if m_items is empty. We should test that case (tour with e.g. just sound) also, it might lead to a crash currently. src/lib/marble/TourPlayback.h <https://git.reviewboard.kde.org/r/118755/#comment42204> remove src/lib/marble/TourPlayback.h <https://git.reviewboard.kde.org/r/118755/#comment42205> remove src/lib/marble/TourPlayback.h <https://git.reviewboard.kde.org/r/118755/#comment42196> private src/lib/marble/TourPlayback.h <https://git.reviewboard.kde.org/r/118755/#comment42195> should be private (or remove completely) src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42203> not needed anymore here src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42202> please remove the phonon includes here, not used anymore src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42198> not needed src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42201> unused, remove src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42197> this check is not needed anymore. You can simply do d->m_mainTrack->append( new PlaybackFlyToItem( flyTo ) ); always src/lib/marble/TourPlayback.cpp <https://git.reviewboard.kde.org/r/118755/#comment42194> I'd work directly on d->m_parallelTracks src/lib/marble/TourWidget.cpp <https://git.reviewboard.kde.org/r/118755/#comment42206> the button should not both be checked and switch its icon (that's confusing), just one of both: Variant A) button is not checkable in play mode: show pause icon in pause mode: show play icon Variant B) play-icon, button is checkable in play mode: checked in pause mode: not checked Variant A) is more popular nowadays, so I'd go for that src/lib/marble/TourWidget.cpp <https://git.reviewboard.kde.org/r/118755/#comment42207> did you mean to execute this on actionPlay? - Dennis Nienhüser On June 19, 2014, 3:08 a.m., Sanjiban Bairagya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118755/ > ----------------------------------------------------------- > > (Updated June 19, 2014, 3:08 a.m.) > > > Review request for Marble, Dennis Nienhüser and Torsten Rahn. > > > Repository: marble > > > Description > ------- > > This patch refactors tour playback logic and adds basic interpolation of > tours in Marble > > > Diffs > ----- > > src/lib/marble/ParallelTrack.h PRE-CREATION > src/lib/marble/CMakeLists.txt 4987c90 > src/apps/marble-ui/ControlView.cpp 84ed594 > src/lib/marble/ParallelTrack.cpp PRE-CREATION > src/lib/marble/PlaybackAnimatedUpdateItem.h PRE-CREATION > src/lib/marble/PlaybackAnimatedUpdateItem.cpp PRE-CREATION > src/lib/marble/PlaybackFlyToItem.h PRE-CREATION > src/lib/marble/PlaybackFlyToItem.cpp PRE-CREATION > src/lib/marble/PlaybackItem.h PRE-CREATION > src/lib/marble/PlaybackItem.cpp PRE-CREATION > src/lib/marble/PlaybackSoundCueItem.h PRE-CREATION > src/lib/marble/PlaybackSoundCueItem.cpp PRE-CREATION > src/lib/marble/PlaybackTourControlItem.h PRE-CREATION > src/lib/marble/PlaybackTourControlItem.cpp PRE-CREATION > src/lib/marble/PlaybackWaitItem.h PRE-CREATION > src/lib/marble/PlaybackWaitItem.cpp PRE-CREATION > src/lib/marble/SerialTrack.h PRE-CREATION > src/lib/marble/SerialTrack.cpp PRE-CREATION > src/lib/marble/TourPlayback.h 44a793f > src/lib/marble/TourPlayback.cpp 03ab5f5 > src/lib/marble/TourWidget.h 31d3a59 > src/lib/marble/TourWidget.cpp 95d95b5 > src/lib/marble/TourWidget.ui 7a8a8ce > src/lib/marble/geodata/data/GeoDataAbstractView.h 0f3f13c > src/lib/marble/geodata/data/GeoDataAbstractView.cpp 32867ba > > Diff: https://git.reviewboard.kde.org/r/118755/diff/ > > > Testing > ------- > > > Thanks, > > Sanjiban Bairagya > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
