If an unexpected error was returned from a Wayland API during rendering (e.g.
from wl_display_flush), the code did set input.sym = XKB_KEY_Escape, so that
the next call to poll_key would return BM_KEY_ESCAPE and bemenu would quit.
However, this has been broken since #135, because input.key_pending was not
set, so the "fake" XKB_KEY_Escape is just ignored, bemenu doesn't quit, but
instead, it enters an infinite loop and keeps a CPU core at 100% usage.
The "quick fix" would be to just set input.key_pending wherever input.sym was
set to XKB_KEY_Escape. However, to make error handling less error-prone,
decouple it from input handling and add an error flag to (bm_menu_)render.
If the pointer suddenly enters a window but moves no further, we may only get
a POINTER_EVENT_ENTER but no further POINTER_EVENT_MOTION events, so we also
need to handle this event to properly update the selected item.
The issue can be reproduced by moving the mouse programatically, e.g. on Sway:
swaymsg seat - cursor set 200 200
After the changes of the previous commit, the event loop flow on Wayland is:
1. Render windows if window->render_pending is set
2. Wait for events [NOTE: this includes the frame callback]
3. Schedule render if menu->dirty is set
4. Handle events (return from render) and repeat
This can still miss renders since the menu->dirty flag is set in step (4),
but the menu->dirty flag is checked in step (3). So if the event loop only
does a single iteration (quite unusual as most user actions cause multiple
events), we can get stuck on step (2) for a while.
In order to avoid this problem, this changes the event loop order to:
1. Schedule render if menu->dirty is set
2. Wait for events [NOTE: this includes the frame callback]
3. Render windows if window->render_pending is set
4. Handle events (return from render) and repeat
Script (for Sway) to reproduce the issue / verify the fix:
#!/usr/bin/env sh
mousesety() { swaymsg seat - cursor set 200 "$1" >/dev/null; sleep 0.2; }
{ while true; do mousesety 200; mousesety 300; mousesety 400; done } &
trap 'kill $!' EXIT
export BEMENU_BACKEND=wayland BEMENU_OPTS='--list 40 --hb #0000FF'
yes | head -30 | bemenu
Fixes: #274
Fixes: #275
Despite the fix in the previous commit (3d7e47c4e6), the following command:
{ echo one; echo two; } | BEMENU_BACKEND=wayland bemenu --grab
Will likely still show the 'Loading...' text after all items are available.
A related problem (also on Wayland) is that when pressing two keys (for the
filter) almost simultaneously, sometimes only one of the keys will be rendered.
The other key will only be shown on the next render. For example:
- Filter shows "s"
- User presses the "o" and "n" keys simultaneously
- Filter shows "so" ["n" appears to have been lost]
- User presses the "g" key
- Filter shows "song" [now the "n" has been rendered]
Both problems have the same root cause: If two events that cause a render happen
"close enough" to each other, often the second event will not cause a render.
As far as I can tell, the cause is that the "dirty && render_pending" render
check should not check the dirty flag at all, just "render_pending".
With "dirty && render_pending", if two events happen in close succession:
- On the first event, generally dirty==true, render_pending==true, a render
will happen, the frame callback will be set, and both flags will be cleared.
- For the second event, generally dirty==true and render_pending==false.
dirty will be unset and nothing will happen.
- When the frame triggers, render_pending is set, but no render will happen
because dirty==false
With just "render_pending" (the change in this commit):
- On the first event, generally dirty==true, render_pending==false, the frame
callback will be set, and dirty will be cleared.
- For the second event, generally dirty==true and render_pending==false.
dirty will be unset and nothing will happen.
- When the frame triggers, render_pending is set, then a render will be done.
Currently, on the Wayland backend, the following command:
{ echo one; sleep 1; echo two; } | BEMENU_BACKEND=wayland bemenu --grab
Will keep showing the 'Loading...' text even after all items are available.
The items will only be shown after some input, e.g. a key press, happens.
This was a regression introduced by 5a095705d2
(versions 0.6.5+). A dirty flag was added to avoid unnecessary redraws,
however, this flag is not reset between the renders before/after the items are
loaded, so the bm_menu_render call after the items are loaded is ignored
(on Wayland, which is the only renderer that currently uses the dirty flag).
Fix the issue by setting the dirty flag when the filter changes, so it will be
reset when changing from "Loading..." to empty and cause a redraw.
We add a dirty flag on the menu to track if the menu actually need a
redraw. With it, we will not redraw if the touch is hold on the same
entry by example.
It works on Wayland and X11 and acts as a complement to margin. Exact
behavior is as follows:
- If width factor is 0, width minus margin is used.
- If width multiplied by factor is greater than width minus margin,
width minus is used. (so margin may be used to make sure that bemenu
is at least N pixels away from the view border)
- Otherwise width multiplied by factor is used.
I think it's fine to disable warnings about floating point numbers
comparision. We don't do any arithmetics on them anyway, so we can't
suffer from inaccuracy.
`ncuses-6.3` added printf-style function attributes and now makes
it easier to catch cases when user input is used in palce of format
string when built with CFLAGS=-Werror=format-security:
lib/renderers/curses/curses.c:234:9:
error: format not a string literal and no format arguments [-Werror=format-security]
234 | mvprintw(0, 0, menu->title);
| ^~~~~~~~
Let's wrap all the missing places with "%s" format.
This caused issue when using the -b (bottom) and -l (line) arguments on
scaled outputs.
When using a scaled output, the set_size use a wrong value as height.
We generated a scalled buffer so we used a scale x too high size.
We just have to divide the scaling to use a good size.
This seems to be broken at least on some terminals and the high bit
toggle corrupts input. In addition there was printf for the esc/alt key
detection that can break the UI.
Neovim seems to have noncompatible way of detecting alt, so I think
neovim should be looked for proper way for handling this.
Previously, any seat without a keyboard could destroy our selected
keyboard. Now, select by seat instead and only destroy the keyboard if
it vanishes from that seat. This isn't actually multi-seat support, but
at least it will allow bemenu to accept input.
This adds an alias 'focused' for selecting the current monitor, which
becomes the default on x11 and wayland. The previous wayland default of
displaying on all outputs moves under '-2' or 'all'.
ref: https://github.com/Cloudef/bemenu/issues/102#issuecomment-604562234
Signed-off-by: Robert Günzler <r@gnzler.io>
With dmenu, monitor indices start at 0 and a value of -1 (the default)
is used to spawn dmenu on the current monitor. While bemenu strives to
be compatible with dmenu, bemenu monitor indices previously started at 1
and a value of 0 (the default) was used to spawn on the current monitor.
This commit aligns the behaviour of bemenu's x11 backend with dmenu. For
this purposes, the affected code in the x11 backend is synced with the
current dmenu implementation. While doing so the monitor type has also
been switched from a uint32_t to a int32_t.
Joan Bruguera
Stacy Harper
Daniel Lublin
Barbaross
Julian Orth
Jari Vetoniemi
Maxim Karasev
Andrei E
lunacb
Sergei Trofimovich
rei de vries
Ben Brown
Tuyen Pham
Bill Doyle
Robert Günzler
Jari Vetoniemi
Sören Tempel
Harley Swick