Bug 240463 - Tray icon causes moderate Xorg and plasma-desktop cpu usage
Summary: Tray icon causes moderate Xorg and plasma-desktop cpu usage
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Notifications (show other bugs)
Version: 2.3.1
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-02 14:21 UTC by niburu1
Modified: 2010-07-08 13:13 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.2


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description niburu1 2010-06-02 14:21:23 UTC
Version:           2.3.0 (using Devel) 
OS:                Linux

When the tray icon setting is enabled Xorg consumes 10-15% of the cpu and plasma-desktop consumes roughly 6-10%. I'm running an Intel Core Solo 1.06Ghz ULV CPU. This seems to me to be higher than one would expect.

Reproducible: Always

Steps to Reproduce:
Enable tray icon setting

Actual Results:  
CPU usage goes up about 20% (due to Xorg and plasma-desktop)
Comment 1 Mikko C. 2010-06-02 15:11:28 UTC
How do you "disable" the tray icon?
Does CPU usage decrease when amarok is not playing?
Comment 2 niburu1 2010-06-02 16:58:52 UTC
To disable the tray icon go to "System" -> "Configure Amarok" and uncheck "Show tray icon".

CPU usage goes up *only* when Amarok is playing local files. It does not happen when streaming audio or when a local file is paused or stopped.
Comment 3 Myriam Schweingruber 2010-06-03 00:18:02 UTC
Can somebody test this with Amarok 2.3.1, please?
Comment 4 Kevin Funk 2010-06-03 00:57:17 UTC
Tried both with 2.3.0 and 2.3.1, can't reproduce. Tried playing local tracks. My CPU frequency is at 2x800MHz, so nearly the same.
* Plasma: 0% usage
* X: 1-3% usage

Sure its not caused by other settings (some window minimized or not)?
Comment 5 niburu1 2010-06-03 01:07:26 UTC
I'm running a nearly fresh install of Kubuntu 10.04 with unsupported updates enabled and backports. On a clean boot the problem is present, so it has nothing to do with minimized windows or such. What I would like to know is: is it my system or Kubuntu's packaging?
Comment 6 niburu1 2010-06-03 01:11:34 UTC
I should mention that the problem persists in Amarok 2.3.1 and that I've noticed the tray icon does not show progress as it should, which might be linked to the problem somehow.
Comment 7 Mikko C. 2010-06-03 07:48:49 UTC
what phonon backend do you use? Xine or gstreamer? Does changing it fix the issue?
Comment 8 niburu1 2010-06-03 14:02:34 UTC
I'm using the Xine backend and unsure of how to change it to gstreamer. I'll see if I can and report back if successful.
Comment 9 Myriam Schweingruber 2010-06-03 16:34:23 UTC
(In reply to comment #8)
> I'm using the Xine backend and unsure of how to change it to gstreamer. I'll
> see if I can and report back if successful.

You need to install gstreamer and the gstreamer codecs, then you can change it in the System Settings -> Multimedia -> Backend tab
Comment 10 Kevin Funk 2010-07-08 12:13:52 UTC
commit 26104cd35fd50222c354f3afc9fce6bba093c05f
Author: Kevin Funk <krf@electrostorm.net>
Date:   Thu Jul 8 12:13:00 2010 +0200

    Some TrayIcon changes:
    * Remove track progress effect (this is because KSNI has some bogus
    implementation of the setIconByPixmap() function causing the overlay
    icon being wrong sized
    * Fix overlay icon size
    * Tooltip album cover is now updated if changed in Amarok
    * Cleanup
    CCBUG: 231539
    CCBUG: 232578
    CCBUG: 232312
    BUG: 233506
    BUG: 240463

diff --git a/src/TrayIcon.cpp b/src/TrayIcon.cpp
index fb97483..4bd46e4 100644
--- a/src/TrayIcon.cpp
+++ b/src/TrayIcon.cpp
@@ -52,7 +52,6 @@
 Amarok::TrayIcon::TrayIcon( QObject *parent )
         : KStatusNotifierItem( parent )
         , Engine::EngineObserver( The::engineController() )
-        , m_trackLength( 0 )
         , m_separator( 0 )
 {
     DEBUG_BLOCK
@@ -80,14 +79,8 @@ Amarok::TrayIcon::TrayIcon( QObject *parent )
 
     PERF_LOG( "Adding system tray icon" );
 
-    m_baseIcon = KIconLoader::global()->loadIcon( "amarok", KIconLoader::Panel );
-    setIconByPixmap( m_baseIcon ); // show icon
-    setOverlayIconByName( QString() );
+    setIconByName( "amarok" );
 
-    m_grayedIcon = m_baseIcon; // copies object
-    KIconEffect::semiTransparent( m_grayedIcon );
-
-    paintIcon();
     setupToolTip();
 
     connect( this, SIGNAL( scrollRequested( int, Qt::Orientation ) ), SLOT( slotScrollRequested(int, Qt::Orientation) ) );
@@ -102,22 +95,13 @@ Amarok::TrayIcon::setupToolTip()
         setToolTipTitle( The::engineController()->prettyNowPlaying() );
 
         QStringList tooltip;
-        // TODO: Use Observer to get notified about changed album art
-        if( m_track->album() )
+        if( m_track->album() && m_track->album()->hasImage() )
         {
             const QString uid = m_track->uidUrl();
             if ( uid != m_toolTipIconUid ) {
                 const QPixmap image = The::svgHandler()->imageWithBorder( m_track->album(), KIconLoader::SizeLarge, 5 );
-                if ( image.isNull() )
-                {
-                    setToolTipIconByName( "amarok" );
-                    m_toolTipIconUid.clear();
-                }
-                else
-                {
-                    setToolTipIconByPixmap( image );
-                    m_toolTipIconUid = uid;
-                }
+                setToolTipIconByPixmap( image );
+                m_toolTipIconUid = uid;
             }
         }
         else
@@ -184,9 +168,10 @@ Amarok::TrayIcon::setupToolTip()
     else
     {
         setToolTipIconByName( "amarok" );
-        m_toolTipIconUid.clear();
         setToolTipTitle( KCmdLineArgs::aboutData()->programName() );
         setToolTipSubTitle( The::engineController()->prettyNowPlaying() );
+
+        m_toolTipIconUid.clear();
     }
 }
 
@@ -206,20 +191,26 @@ Amarok::TrayIcon::engineStateChanged( Phonon::State state, Phonon::State /*oldSt
     switch( state )
     {
         case Phonon::PlayingState:
-            unsubscribeFrom( m_track );
+            if ( m_track )
+            {
+                unsubscribeFrom( m_track );
+                unsubscribeFrom( m_track->album() );
+            }
             m_track = track;
-            m_trackLength = m_track ? m_track->length() : 0;
-            subscribeTo( track );
+            if ( track )
+            {
+                subscribeTo( track );
+                subscribeTo( track->album() );
+            }
 
-            paintIcon( 0 );
+            setOverlayIconByName( "media-playback-start" );
             setupMenu();
             break;
 
         case Phonon::StoppedState:
             m_track = 0;
-            m_trackLength = 0;
 
-            paintIcon();
+            setOverlayIconByName( QString() );
             setupMenu(); // remove custom track actions on stop
             break;
 
@@ -230,6 +221,7 @@ Amarok::TrayIcon::engineStateChanged( Phonon::State state, Phonon::State /*oldSt
         case Phonon::LoadingState:
         case Phonon::ErrorState:
         case Phonon::BufferingState:
+            setOverlayIconByName( QString() );
             break;
     }
 
@@ -240,9 +232,6 @@ void
 Amarok::TrayIcon::engineNewTrackPlaying()
 {
     m_track = The::engineController()->currentTrack();
-    m_trackLength = m_track ? m_track->length() : 0;
-
-    paintIcon( 0 );
 
     setupToolTip();
     setupMenu();
@@ -258,12 +247,12 @@ Amarok::TrayIcon::metadataChanged( Meta::TrackPtr track )
 }
 
 void
-Amarok::TrayIcon::engineTrackPositionChanged( qint64 position, bool userSeek )
+Amarok::TrayIcon::metadataChanged( Meta::AlbumPtr album )
 {
-    Q_UNUSED( userSeek );
+    Q_UNUSED( album )
 
-    if( m_trackLength )
-        paintIcon( position );
+    setupToolTip();
+    setupMenu();
 }
 
 void
@@ -283,55 +272,6 @@ Amarok::TrayIcon::engineMuteStateChanged( bool mute )
 }
 
 void
-Amarok::TrayIcon::paletteChange( const QPalette & op )
-{
-    Q_UNUSED( op );
-
-    paintIcon();
-}
-
-void
-Amarok::TrayIcon::paintIcon( qint64 trackPosition )
-{
-    static qint64 oldMergePos = -1;
-
-    // trackPosition < 0 means reset
-    if( trackPosition < 0 )
-    {
-        oldMergePos = -1;
-        setIconByPixmap( m_baseIcon );
-        setOverlayIconByName( QString() );
-        return;
-    }
-
-    // check if we are playing a stream
-    if( !m_trackLength )
-    {
-        m_icon = m_baseIcon;
-        setIconByPixmap( m_icon );
-        setOverlayIconByName( "media-playback-start" );
-        return;
-    }
-
-    const qint64 mergePos = ( float( trackPosition ) / m_trackLength ) * m_icon.height();
-
-    // return if pixmap would stay the same
-    if( oldMergePos == mergePos )
-        return;
-
-    // draw m_baseIcon on top of the gray version
-    m_icon = m_grayedIcon; // copies object
-    QPainter p( &m_icon );
-    p.drawPixmap( 0, 0, m_baseIcon, 0, 0, 0, m_icon.height() - mergePos );
-    p.end();
-
-    oldMergePos = mergePos;
-
-    setIconByPixmap( m_icon );
-    setOverlayIconByName( "media-playback-start" );
-}
-
-void
 Amarok::TrayIcon::setupMenu()
 {
     foreach( QAction* action, m_extraActions )
diff --git a/src/TrayIcon.h b/src/TrayIcon.h
index c2602df..d590b5f 100644
--- a/src/TrayIcon.h
+++ b/src/TrayIcon.h
@@ -49,16 +49,13 @@ protected:
     // reimplemented from engineobserver
     virtual void engineStateChanged( Phonon::State state, Phonon::State oldState = Phonon::StoppedState );
     virtual void engineNewTrackPlaying();
-    virtual void engineTrackPositionChanged( qint64 position, bool /*userSeek*/ );
     virtual void engineVolumeChanged( int percent );
     virtual void engineMuteStateChanged( bool mute );
 
-    //Reimplemented from Meta::Observer
+    // reimplemented from Meta::Observer
     using Observer::metadataChanged;
     virtual void metadataChanged( Meta::TrackPtr track );
-
-    // get notified of 'highlight' color change
-    virtual void paletteChange( const QPalette & oldPalette );
+    virtual void metadataChanged( Meta::AlbumPtr album );
 
 private slots:
     void slotActivated();
@@ -68,13 +65,9 @@ private:
     void setupMenu();
     void setupToolTip();
 
-    void paintIcon( qint64 trackPosition = -1 );
-
     Meta::TrackPtr m_track;
-    qint64 m_trackLength;
     QString m_toolTipIconUid;
 
-    QPixmap m_baseIcon, m_grayedIcon, m_icon;
     SmartPointerList<QAction> m_extraActions;
     QPointer<QAction> m_separator;
 };
Comment 11 niburu1 2010-07-08 12:25:14 UTC
That was relatively fast. I'll be glad to use the tray icon again.