the lag / skipping of the clock was not caused by faulty timer code
on fluxbox's side but by the behavior and inner workings of time().
since this is fixed now (913244789f) we can now rollback ec7fe513c8
and detect strftime-formats which need intervals of seconds or minutes.
minor: the small change to FbTk::Timer::setTimeout() reduces one
start() / stop() cycle for a running timer.
std::set<Key, Comp> stores Key only if Comp(Key) yields a unique result (My
mistake: I was under the impression Comp is only used for the ordering). This
prevents FbTk::Timers with equal end-times from actually being started.
Escpecially in situation with multiple ClockTools this lead to stopped timers
(see bug #3600694).
Kudos to Adam Majer for enlightening discussions.
Users expect time switches to happen upon system clock times. Calculating the
timeout for the next refresh of the shown time via the monotonic clock is
wrong: The monotonic clock yields values based upon some arbitrary point in
time which might be off a little bit to the system clock, a 'full' minute of
the monotonic clock might be in the midst of a system clock minute.
* Calling Timer::setTimeout() from within Timer::start() might lead to ugly
behavior (as experienced in bugs #3590078, #3600143, etc; see commit
4d307dcd10, fixes the problem partially).
* Stop a timer first, then call the handler (via Timer::fireTimeout()). A
given handler might call Timer::start() again, which (re)adds the Timer
to the control list .. the following Timer::stop() would remove it again.
* Use 'm_start' as indicator if timer is running.
* Move the (now quite short) code of ::addTimer / ::removeTimer
into the Timer::start() and Timer::stop() functions.
With commit 541c8c4 we switched from an (manually) ordered list to a
std::set<> to handle the active timers. The code which checks for overdue
timers now traverses and modifies the std::set<> in place. This might
lead to an infinite loop. Examples of such bad behavior are "flickering of
the tooltip" (bug #3590078) or crashes (bug #3600143) or just insanely high
cpu load when autoraising windows or submenus.
We now make a copy of the std::set<> traverse this instead of the original.
Setting a new timeout on a running FbTk::Timer must remove it from the timerlist,
otherwise the list is not ordered correctly anymore. So, we stop the running
FbTk::Timer, set the new timeout and restart it.
gettimeofday() is subject to be changed on daylight-saving or to ntp-related
(think leap-seconds). even worse, it is subject to be changed BACK in time. this
is hard to fix correctly (see commit 45726d3016 and bug #3560509). it is
irrelevant for timers to know the nano-seconds since the epoch anyways.
Found with cppcheck:
"Prefix ++/-- operators should be preferred for non-primitive
types. Pre-increment/decrement can be more efficient than
post-increment/decrement. Post-increment/decrement usually
involves keeping a copy of the previous value around and adds
a little extra code."
the previous version of operator*() made no sense. E.g., it violated the invariant
(*ptr).foo <=> ptr->foo. The dereferencing operator now returns a reference to the pointed-to
object, rather than a pointer to it.
I also added a bool conversion operator, which can be used in testing the NULL-ness of the
pointer. Anyone wondering if that could be done in a simpler way is encouraged to read
<http://www.artima.com/cppsource/safebool.html>.
And, finally, I removed the mutable flag from the m_data member, since it does not need it.
