Bug 133804

Summary: the kicker clock applet updates too frequently
Product: [Unmaintained] kicker Reporter: Luciano Montanaro <mikelima>
Component: generalAssignee: Aaron J. Seigo <aseigo>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: Change update frequency when the seconds display is off.

Description Luciano Montanaro 2006-09-09 12:27:51 UTC
Version:            (using KDE KDE 3.5.4)
Installed from:    Compiled From Sources

Prompted by reading
http://impul.se/~kling/blog/?p=9

I checked kicker for possible timer load reduction.

Kicker seem to have a number of timers running, but one easy optimization would be to update the clock at a lower rate (say every 30s) when the user does not require the seconds display.
Comment 1 Luciano Montanaro 2006-09-09 12:30:32 UTC
Created attachment 17677 [details]
Change update frequency when the seconds display is off.

This is a patch for the KDE 3.5 branch implementing the slow update.
Comment 2 Luciano Montanaro 2006-09-11 11:33:11 UTC
I tried porting the patch to KDE4, but I can't find the clock applet anymore.
Comment 3 Aaron J. Seigo 2006-09-11 12:17:38 UTC
the clock applet doesn't exist in kde4.

that said, i too looked at timer reduction in kicker and except for the clock it's silent if not being interacted with as far as i could tell after a few fixes.

so, the clock ... the proper fix, imho, is to figure out how long it is until the next minute and set a time for that many seconds hence. then check every 60 seconds after that. otherwise the clock can be up to 30s off if it's just checked every 30s. should be an easy addition to your patch. let's see....
Comment 4 Luciano Montanaro 2006-09-11 12:35:57 UTC
Is the timer reduction a recent development? Because I have 3-4 gettimeofday() calls per second with 3.5.4 kicker. I use the kasbar instead of the normal taskbar, though, so maybe the problem is there. 

As for the proper fix, yes, I have thought about syncing to the minute too, but I wondered if it was really needed. It should easy enough, with a one-shot timer used to start the real timer.

What happened to the clock, by the way? Do you plan to provide a simpler one by  default, or have you folded somewhere else? 
Comment 5 Aaron J. Seigo 2006-09-11 13:11:07 UTC
SVN commit 583021 by aseigo:

silent night. kde night,
all is calm, all is bright
now the clock updates less often
to our batteries this is a godsend
<chorus, verse, verse, chorus>

thx to Luciano Montanaro for starting on this fix

BUG:133804


 M  +45 -2     clock.cpp  
 M  +2 -0      clock.h  


--- branches/KDE/3.5/kdebase/kicker/applets/clock/clock.cpp #583020:583021
@@ -903,8 +903,6 @@
     showZone(zone->zoneIndex());
     slotUpdate();
 
-    _timer->start(500);
-
     if (kapp->authorizeKAction("kicker_rmb"))
     {
         menu = new KPopupMenu();
@@ -1140,6 +1138,8 @@
 // DCOP interface
 void ClockApplet::reconfigure()
 {
+    _timer->stop();
+
     // ugly workaround for FuzzyClock: sometimes FuzzyClock
     // hasn't finished drawing when getting deleted, so we
     // ask FuzzyClock to delete itself appropriately
@@ -1155,15 +1155,21 @@
 
     QColor globalBgroundColor = KApplication::palette().active().background();
     QColor bgroundColor;
+    int shortInterval = 500;
+    int updateInterval = 0;
     switch (_prefs->type())
     {
         case Prefs::EnumType::Plain:
             _clock = new PlainClock(this, _prefs, this);
             bgroundColor = _prefs->plainBackgroundColor();
+            if (_prefs->plainShowSeconds())
+                updateInterval = shortInterval;
             break;
         case Prefs::EnumType::Analog:
             _clock = new AnalogClock(this, _prefs, this);
             bgroundColor = _prefs->analogBackgroundColor();
+            if (_prefs->analogShowSeconds())
+                updateInterval = shortInterval;
             break;
         case Prefs::EnumType::Fuzzy:
             _clock = new FuzzyClock(this, _prefs, this);
@@ -1173,9 +1179,27 @@
         default:
             _clock = new DigitalClock(this, _prefs, this);
             bgroundColor = _prefs->digitalBackgroundColor();
+            if (_prefs->digitalShowSeconds() || _prefs->digitalBlink())
+                updateInterval = shortInterval;
             break;
     }
 
+    m_updateOnTheMinute = updateInterval != shortInterval;
+    if (m_updateOnTheMinute)
+    {
+        connect(_timer, SIGNAL(timeout()), this, SLOT(setTimerTo60()));
+        updateInterval = ((60 - clockGetTime().second()) * 1000) + 500;
+    }
+    else
+    {
+        // in case we reconfigure to show seconds but setTimerTo60 is going to be called
+        // we need to make sure to disconnect this so we don't end up updating only once
+        // a minute ;)
+        disconnect(_timer, SIGNAL(timeout()), this, SLOT(setTimerTo60()));
+    }
+
+    _timer->start(updateInterval);
+
     // See if the clock wants to show the day of week.
     // use same font/color as for date
     showDayOfWeek = _clock->showDayOfWeek();
@@ -1235,6 +1259,13 @@
     emit(updateLayout());
 }
 
+void ClockApplet::setTimerTo60()
+{
+//    kdDebug() << "setTimerTo60" << endl;
+    disconnect(_timer, SIGNAL(timeout()), this, SLOT(setTimerTo60()));
+    _timer->changeInterval(60000);
+}
+
 void ClockApplet::setBackground()
 {
     bool toreset = true;
@@ -1326,6 +1357,18 @@
         updateDateLabel();
     }
 
+    if (m_updateOnTheMinute)
+    {
+        // catch drift so we're never more than a few s out
+        int seconds = clockGetTime().second();
+//        kdDebug() << "checking for drift: " << seconds << endl;
+
+        if (seconds > 2)
+        {
+            connect(_timer, SIGNAL(timeout()), this, SLOT(setTimerTo60()));
+            _timer->changeInterval(((60 - seconds) * 1000) + 500);
+        }
+    }
     _clock->updateClock();
     KickerTip::Client::updateKickerTip();
 }
--- branches/KDE/3.5/kdebase/kicker/applets/clock/clock.h #583020:583021
@@ -297,6 +297,7 @@
 	void aboutToShowContextMenu();
 	void fixupLayout();
 	void globalPaletteChange();
+	void setTimerTo60();
 
 protected:
 	void toggleCalendar();
@@ -332,6 +333,7 @@
 	Zone *zone;
 	bool showDate;
 	bool showDayOfWeek;
+	bool m_updateOnTheMinute;
 	QStringList _remotezonelist;
 	KPopupMenu* menu;
 	ClockAppletToolTip m_tooltip;
Comment 6 Aaron J. Seigo 2006-09-11 13:19:43 UTC
> Is the timer reduction a recent development?

sometime in the last few weeks

> Because I have 3-4 gettimeofday() calls per second with 3.5.4 kicker. I use 
> the kasbar instead of the normal taskbar, though, so maybe the problem is
> there. 

could be. i tested mostly just the default setup.

also note that some styles are REALLY evil. in fact, i stopped using polyester after doing this work because it starts timers and never stops them. i was wondering where these gettimeofday()s were coming from while strace'ing kicker and noticed they started as soon as a combobox widget was shown. i eventually tracked it down to polyester.

so try with plastik as your style if you aren't using that and if the gettimeofday()s persist, try removing things from your kicker (e.g. kasbar) to see what causes them.
 
> As for the proper fix, yes, I have thought about syncing to the minute too, 
> but I wondered if it was really needed.

it would look fairly bad IMHO if the clock on the panel was out of sync with time shown elsewhere.

> What happened to the clock, by the way? Do you plan to provide a simpler one
> by  default, or have you folded somewhere else? 

the clock in kde3 is crap. the code is not great and the design is poor. it served the purpose, but we can and need to do much better from a look 'n feel perspective. in kde4 we'll use SVGs for the graphics for easier and more flexible theming, not to mention simpler code.