Note: I expect only modern compilers will hit the source of fluxbox. It
seems futile to guard stdlib headers while demanding <algorithm> etc. This
should trim down the noise in the source quite a bit.
Every time either the Slit menu or the Toolbar menu were added to the
root menu, the whole root menu was (re)created from scratch. Now we
create and remove only the menus needed.
Side effect: the position of these menus is now at the end of the root
menu and not somewhere in between. If users complaint, I' ll think about
it.
This commit is a preparation step for some menu cleanup ahead. To make it
easier to understand which types of MenuItems are added where and when, I
converted the overloaded FbTk::Menu::insert() functions into explicit ones
(Menu::insertSubmenu(), Menu::insertItem(), etc. ). This makes it possible
to just grep for 'insertSubmenu()'.
Side effect this commit: it trims down the very verbose creation of menu items
in regards to how the labels are created.
Minor: FbTk::Menu::shown and FbTk::Menu::s_focused are moved out of the class.
When fluxbox comes up some of it's drawables span a 1x1 area. Subtracting from
such small numbers bigger ones always lead to massive problems when 'unsigned
int' are involved:
'button_width' is an unsigned int of '1' (this might be caused by another
issue or on purpose, anyway), subtracting -10 or any other number should
result in something < 0 when in reality an integer underflow happen: max_width
is now MAX_INT-something big. This makes fluxbox crash under certain
circumstances.
This fixes bug #1116.
POSIX states that 'd_name' in 'struct dirent' is char[], so it cannot be NULL.
This will result in the compiler complainting about an expression which always
evaluates to true ... for this compiler (clang). But in some implementations
'd_name' is a 'char*' that's why it's better to keep the check for possible
NULL.
While running code analysis tools this issue popped up. As it is written it is
clearly a NOP, but the reason for why it is written that way remains unknown.
I suspect that it was a textfragment introduced by some repeat-function of the
used editor (think '.' in VIM). The code was introduced by commit a932a7a801
and looked like this in the original form:
if(!XQueryTree(FbTk::App::instance()->display(), window().window(),
&root_return, &parent_return,
&children_return, &nchildren_return))
parent_return=parent_return;//return;
Look at the outcommented return statement. I decided to return from that
function instead of the NOP.
the -1 in the FbTk::FbTimer::remainingNext() function was pointless in the
first place anyway: reducing the timeout by just 1 microsecond improves
nothing (in this case). if the timer triggers exactly at a full unit (second)
then it's correct to wait for the full next unit.
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.
This commit fixes primarily a race condition that occurs when xinit(1) shuts
down: by not acting properly fluxbox gets caught in an infinite loop. It
caused bug #1100.
xinit(1) sends a SIGHUP signal to all processes. fluxbox tries to shutdown
itself properly by shutting down workspaces and screens. While doing that, the
Xserver might be gone already. Additionally, fluxbox used to restart() itself
on SIGHUP, which is clearly not the right thing to do when xinit(1) is about
to end the session.
So, fluxbox does this:
* handling SIGHUP now shuts down fluxbox without clearing workspaces and
screens.
* A 2 second alarm() is triggered in Fluxbox::shutdown() as a last resort
* XSetIOErrorHandler() is used to recognize the disconnect from the xserver.
* SIGUSR1 is for restarting fluxbox, SIGUSR2 for reloading the config
* FbTk/SignalHandler.cc/hh is gone; this unused abstraction served currently
no real purpose. Signal handling is now done in main.cc
* Unrelated to the issue itself src/main.cc was trimmed down quite a bit and
the code (responsible for handling the command line interface) was moved to
src/cli*
usually $HOME is set when fluxbox runs. in some rare scenarios (eg., fuzzying
binaries to detect bugs) one could launch fluxbox by using 'env -i' and thus
eliminating $HOME from the environment. to prevent crashes fluxbox uses now
'getpwuid()' when $HOME is not set to detect the home folder.
Detecting very long window titles is done via FbTk::TextUtils::doAlignment().
Instead of removing one char from the title at a time to see if it fits into a
given 'max_width', we now use a binary-search like approach to get faster to
the right value. This massively improves the speed for windows with
(arbitrary) long window titles (see bug #1090, javascript
document.title = new Array(4999).join(".");
leads to massive waiting for fluxbox to detect that this window has a very
long title).
In addition to that Xft returns 'wrapped' shorts ('integer overflows') for
long texts: XGlpyhInfo.xOff is declared as signed short, it's able to hold
~32k pixels. A monospace font with font-size 10 produces an integer
overflow after 3276 chars / glyphs, thus rendering the check
if (text_width < max_width) { /* ... */ }
pointless and leading rendering the whole title. By calculating some kind of
upper limit for a pseudo-wide glyph ("WW") and strictly cutting off the input
string at that limit prevents this issue.
This commit alters XRANDR (X Resize And Rotate) extension dependency,
which is expected to have at least version 1.4. Earlier old versions of
xrandr were supported, at least to some extent.
Do not try to be too smart which compilations need config.h, as most of
them will simply because of the config.h has information about system
capabilities.
For odd 'widths' and 'heigths' the texture would not be filled completely:
Given a 'width' of 5 we would render only 4 instances of x (-2, 1, 0, 1)
instead of the needed 5. This results in a texture which looks a bit cut off
to the bottom right side.
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.
ClientPatterns might be tricky to get right. Instead of fiddling around in
either the keys-file or the apps-file and restarting fluxbox to see if the
changes had any effect / matched the right windows, 'ClientPatternTest' and
the fluxbox-remote should make this easier:
$> fluxbox-remote "clientpatterntest (title=.*vim*)"
This causes fluxbox to store the list of matched windows in the
_FLUXBOX_ACTION_RESULT property onto the rootwindow. This property might
then be read by:
$> xprop -root _FLUXBOX_ACTION_RESULT
or
$> fluxbox-remote result
The format of the list is:
win_id \t title_of_window \n
win_id is '-1' when fluxbox wasn't able to parse the given ClientPattern.
win_id is '0' when there are no windows matching the given ClientPattern.
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.
The old code did not brighten up "pure" colors like "red", "green" and "blue"
at all. The new code use a different precomputed LUT which is based upon
simplified vector math, see the comments in FbTk/ColorLUT.cc
* 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.
Testing bits-per-pixel in the inner loop is suboptimal, especially since
that value does not change. A little helper macro helps to keep the code
readable, also improves the situation for StaticGray and PseudoColor.
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.
'filename' might contain a '~' which has to be expanded to work for
regular system calls. We expanded 'filename' already but then did not use
it. Fixes bug #3576586.
* much more readable and easier to read code
* smaller code
* reduced binary size due to removed big lookup-table for square root
* simple 'optimizations (lookup tables, packing of data), nothing too fancy
The old formula for vertical align text inside FbTk::TextButton ('height/2 +
font_ascent/2 - 1') produced not always good looking results, escpecially
when different fonts are involved (eg, ClockTool and WorkspaceName have
different fonts and font-sizes).
'(height - font_ascent) / 2 - 1' produces better results.
Additional changes:
* added ASCII-Art to document the involved entities when calculating the
baseline
* rewritten tests/testFont.cc to accept multiples texts and multiple
fonts
* removed some internal parts of FbTk::Font from the public interface
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."
if pos is not npos, it will always be less than filename.size().
However, the access later is only safe if there is a character
after pos, which would require pos + 1 to be less than filename.size.
93924af160 might corrupt memory with gcc-4.6.1 when
finishing fluxbox (clicking 'exit', sending it a SIGINT). Allthough the order, in which static / global
objects are initialized is undefined (at least between separate compilation units), the order in
which they are destroyed is well defined: in reverse order of initialization.
this means, that if 'ScreenImlibContextContainer contexts' (of ImageImlib2.cc)
gets initialized AFTER 'ImageImlib2 imlib2_loader' of Image.cc, it gets destroyed before
imlib2_loader. When that happens, ~ImageImlib2() works on a destroyed object.
(That lead to '* glibc detected * fluxbox: corrupted double-linked list: 0x0000000000dd2710 ***'
later on in 'iconv_close')
fluxbox now properly displays windows that require ARGB visuals when
an external compositor is running. This was done by creating the
container window with the correct visual and colormap when needed.
Closes#2874629
The idea is that connecting to a signal doesn't change it's state or the state of the object
owning the signal (even though it needs to add the functor to the list for later reference).
Emitting, on the other hand, is usually done as a result of a state change and therefore remains
non-const.
Additional benefit of this arrangement is that objects can export const references to signals to
allow connecting, while keeping the ability to emit to themselves.
without this it wasn't possible to construct a Slot returning void from functors returning some
real value because the compiler would complain about "return statement with a value in a function
returning void".
Theoretically, this may produce some unexpected type conversions, because static_cast is slightly
stronger than implicit cast, but I judge the risk to be negligable (the alternative would be to
provide explicit specializations for slots returning void - too much typing)
without them, gcc would compare them by converting them to bool first, which is not exactly what
one would expect. Frankly, I'm surprised it even worked without this.
it is too easy too shoot yourself in the foot with it, other smart pointers also don't allow such
assignments. If you do want to assign to a RefCount pointer, use reset().
ps: assignment between two RefCounts remains possible, of course.
