If any error is encountered during execution of the first subcommand of
a freshly created bar configuration, parsing apparently is to be aborted
and the current bar config is freed. The pointer to that memory is left
dangling though, leading to a use-after-free on successive bar
subcommands. This quite reliably ends in a crash like so:
sway -c reproducer.config
00:00:00.083 [sway/config.c:865] Error on line 2 'foo bar': Unknown/invalid command 'foo' (s)
free(): double free detected in tcache 2
00:00:00.608 [swaynag/swaynag.c:451] failed to register with the wayland display
Aborted (core dumped)
Minimal reproducer config:
bar {
foo bar
position top
}
Other messages:
malloc(): unaligned fastbin chunk detected
double free or corruption (fasttop)
The invalid command has to be the first for a newly created bar config.
Removing the command or switching order so it's not the first one masks
the problem.
Prevent this from occuring by resetting the pointer to NULL after
freeing the memory.
Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
the original behavior set fullscreen for all descendents of a container,
which causes issues when firefox is one of those children because it
sends its own set_fullscreen request in response to being fullscreened.
Can be useful to make sure a bugfix is included. In the future maybe
the wlroots version string could include a commit hash when built
from source, too.
When a tiling container is floated, the focus stack needs to be
appropraitely modified to return the container to its original
position in the tree upon floating disable, like i3.
Logic that obtains current DPMS state is put inside the handler.
sway_output from which the current DPMS state will be obtained is selected by the following logic:
* For '-' and '--' the focused output is used;
* For '*' error "Cannot apply toggle to all outputs" is reported;
* For everything else all_output_by_name_or_id() is used.
Fixes#5929.
When an application inhibited idle, a view pointer was stored and a
destroy listener was registered to the wlr inhibitor. As the wlr
inhibitor lives longer than the view, this lead to a dangling view
pointer between view unmap and inhibitor destroy.
Store a pointer to the wlr inhibitor instead of to the view, and look up
the view when needed, which may at any point be NULL. This also allows
for an inhibitor to remain functional if a surface is re-mapped.
container_at checks if the position provided matches the currently
focused container with view_container_at as a fast path.
view_container_at checks using the main container geometry, which
includes the titlebar and border area. If a tabbed container is focused,
then positions over unfocused tabs are incorrectly reported as belonging
to the focused container, breaking focus on click.
Add view_container_content_at for use in the focused container fast path
which only tests container content area, and fall back to full workspace
scans for border and titlebar areas.
Closes: https://github.com/swaywm/sway/issues/6074
container_at would maintain the current focus as long as a position was
over one of the container view's surfaces. If an oversized surface was
being clipped, this lead to weird focus behavior.
Instead, use view_container_at for this test, which intersects the
container box before looking at surfaces.
If a surface is associated with a sway container, we limit the
destination box to the container dimensions.
Floating views and popups are exempt from this clipping.
Previously, the special case handling of scratchpad and unmark commands
was (probably accidentally) limited to criteria directly handled in the
execute_command function. This would exclude: 1. for_window criteria, as
these are handled externally for views and 2. and mouse bindings which
select target the node currently under the mouse cursor.
As a concrete example `for_window [app_id="foobar"] move scratchpad,
scratchpad show` would show (or hide due to the toggling functionality)
another window from the scratchpad, instead of showing the window with
app_id "foobar".
This commit replaces the "using_criteria" flag with "node_overridden"
with the more general notion of signifying that the node (and
container/workspace) in the current command handler context of the sway
config is not defined by the currently focused node, but instead
overridden by other means, i.e., criteria or mouse position.
When issuing a focus command on a specific container, users expect to
proceed it even if is hidden by a fullscreen window.
This matches the behavior of i3.
In e0a94bee8d, it was believed that if the
container is being rendered, it must have an output.
This turned out not to be the case. When rendering a container, all its
children are rendered, even if the children is positioned off screen and
thus not having any output. This is the cause of the crash in #6061.
This commit introduces a null-check, which fixes#6061.
Before this commit, when an output had its scale dynamically changed,
Sway would not load a cursor theme with the new scale. This results
in stale cursor images when moving the cursor into an area controlled
by the compositor, like the background or resize areas.
To reproduce:
- Using IPC, set an output scale to a value that isn't currently used
- Move the cursor into a compositor-controlled area
- The cursor will not change
As of 66343839b1, sway now uses a
libdrm header. Add this dependency to the build system so headers from
it can be used on systems where pkg-config is required to find them.
On server request, we need to send configure events to inform the client
of the new intended size. If the client changes size itself, sending a
configure event will only cause problems.
Use transaction_commit_dirty_client to distinguish between the two
transaction causes.
Currently, various floating-point expressions involving
the coordinates of borders, titlebars and content surfaces
are directly assigned to integers, and so they are rounded
towards zero.
This results in off-by-one distances between these elements
when the signs of their coordinates differ.
Fixed by wrapping these expressions with a call to
floor before the assignment.
view_child_init was calling view_init_subsurfaces, which did not set the
parent attribute for the subchildren. This lead to the subchildren
acting as standalone children. If the parent was an xdg_popup, this
would make the subchild unaware of the popup position.
Introduce view_child_init_subsurfaces for view_child_init to use
instead.
Closes: https://github.com/swaywm/sway/issues/6038
The subchildren lose their parent association at this point, so they
will not be able to see that the parent is unmapped.
Instead, just set the subchildren to be unmapped directly.
When a container straddles multiple outputs, the title bar is only rendered
at the scale of the "effective" output. If the title bar straddles onto
another output with a different scale factor, it was drawn at the wrong size.
In this commit, we take into consideration the scale the title was rendered
at and scale it accordingly so that it appears at the right size on the other
outputs.
This fixes#6054.
To reproduce:
- Open a floating window and a popup that hangs over the bottom or right
- Move the window in the direction of the popup overhang
- The previous position of the popup is damaged, not the new one
Pending state is currently inlined directly in the container struct,
while the current state is in a state struct. A side-effect of this is
that it is not immediately obvious that pending double-buffered state is
accessed, nor is it obvious what state is double-buffered.
Instead, use the state struct for both current and pending.
Every seat_set_focus* should be followed by a transaction_commit_dirty.
In cases where the focus change is followed by a seatop_begin* this is
not needed, as transaction_commit_dirty is then called by the
seatop_begin* function.
Fixes#6034
The transaction system contains a necessary optimization where a popped
transaction is combined with later, similar transactions. This breaks
the chronological order of states, and can lead to desynchronized
geometries.
To fix this, we replace the queue with only 2 transactions: current and
pending. If a pending transaction exists, it is updated with new state
instead of creating additional transactions.
As we never have more than a single waiting transaction, we no longer
need the queue optimization that is causing problems.
Closes: https://github.com/swaywm/sway/issues/6012
Try to better mimic JSON node structure produced by i3 which might be
relied on by already existing tools. In particular having "type" right
after "id" is quite handy for streaming high-performance JSON parsers
such as simdjson (which are handy for maintaining responsiveness on
resource constrained systems).
refer ab2a22a78b/src/ipc.c (L338)
Transactions currently wait for all configures to be acked, regardless
fo what they were sent to. This includes views that are hidden in tabbed
or stacked containers. If these views do not ack the configure in
response to a single frame callback, they can cause transaction
timeouts.
Check if a container is hidden before registering the configure serial
and saving any view buffers.
Closes: https://github.com/swaywm/sway/issues/6023
Before this commit, there would be cases where focus changes from one
window to another, the new window activates text_input, then the old
window sends a deactivate request, making text_input unfocused
completely.
There is no need to check for transactions at the end of every user
input, as the vast majority of input will not issue transactions. This
implementation can also hide where changes are made without an
appropriate transaction commit, as a future unrelated input would issue
the commit instead.
Instead, commit transactions in places where changes are made or are
likely to be made.
xdg_shell and xwayland handled geometry changes differently despite
needing mostly identical behavior. The xwayland implementation has been
changed to match that of xdg_shell.
The size of a tiled container cannot change in response to new buffer
sizes, so there is no need to commit a new transaction. Instead, simply
recenter the view with the new geometry, leaving the full transaction
flow for floating containers.
We need to use surface_x and surface_y when rendering and damaging saved
buffers as these compensate for views that have been centered due to
being smaller than their container.
Add them to the surface positions on the saved buffer so we have the
values from the time the buffer was saved.
Sway records pid, workspace, and output for every new process. However, if the
output gets disabled and the workspace disappears, the workspace is still
re-created on the disabled output. This commit adds a check for the enabled
flag, so that NULL will be passed to workspace_create() in this case.
wlr_output_configuration_head_v1_create normally fills out the head
"enabled" field to match the wlr_output state. We overwrite this to also
set the head as enabled if it is only turned off with DPMS.
However, in some cases we may not have a mode for this display, in which
case setting it as enabled will lead to a segfault later on. Therefore,
enabled conditional on the presence of a mode.
For certain applications (e.g. JetBrains) the parent window controls
input. We need to adhere to the ICCCM input focus specification to
properly handle these cases.
Relates to swaywm/wlroots#2604
Instead of calling wlr_xdg_surface_for_each_popup and then
wlr_surface_for_each_surface, use the new for_each_popup_surface helper
introduced in [1] that does it in one go.
[1]: https://github.com/swaywm/wlroots/pull/2609
workspace_squash is container_flatten in the reverse
direction. Instead of eliminating redundant splits that are
parents of the target container, it eliminates pairs of
redundant H/V splits that are children of the workspace.
Splits are redundant if a con and its grandchild have the
same layout, and the immediate child has the opposite split.
For example, layouts are transformed like:
H[V[H[app1 app2]] app3] -> H[app1 app2 app3]
i3 uses this operation to simplify the tree after moving
heavily nested containers to a higher level in the tree via
an orthogonal move.
This changes the move command to better match i3
behavior after the layout changes.
workspace_rejigger handled the case where containers would
escape their workspace in an orthogonal move by changing
the layout to accomodate them, but this case is now handled
within the loop.
In i3, the workspace_layout command does not affect the
workspace layout. Instead, new workspace level containers
are wrapped in the desired layout and the workspace layout
always defaults to the output orientation.
Some comparisons of current Sway versus i3 behavior:
1) T[T[T[app]]] + move left
* Sway: T[app]
* i3: T[T[app]]
2) H[V[H[V[app]]]] + move left
* Sway: H[app]
* i3: H[V[app]]
After this commit, Sway behavior matches i3. The intermediate states
are now:
T[T[T[app]]] -> T[T[app T[]]] -> T[T[app]]
H[V[H[V[app]]]] -> H[V[app H[V[]]]] -> H[V[app]]
In i3 the layout command on a workspace affects the workspace layout
only on empty workspaces. Otherwise children are placed in a new
container with the desired layout to preserve the workspace layout.
Instead of letting wlroots print messages to stdout, route debugging
messages into Sway's logging functions. This allows a more consistent
output (e.g. if Sway or wlroots changes its output style, they don't get
out-of-sync).
I also added a [wlr] prefix to wlroots messages, not yet sure it's a
good thing.
For each following combinations of criteria & command below, the command would
crash sway without the fix.
It's particular about the __focused__ criteria, where the view matches part of
the criteria but not the focused app, leading to a failure when calling
`strcmp` with NULL.
"xterm" is a non-wayland app (X11) and "kitty" is. Both are terminals.
# "class" is specific to X11
# The view is X11 (xterm) leading to the criteria checking for the
# focused app's class, leading to a crash
for_window [class="__focused__"] floating enable
exec kitty -e xterm
# Similarly, crash as the focused app (xterm) has no app_id when the view has one
for_window [app_id="__focused__"] floating enable
exec xterm -e kitty
# If the view has a title but not the focused app: NULL title will crash criteria checking
for_window [title="__focused__"] floating enable
exec xterm -title "" -e xterm
Currently, when sway sends a configure with some geometry and the
client responds with a different geometry in a commit that acks that
configure, sway ignores the new size. Sway applies the surface
geometry it had requested to the container, not what was actually
committed, in the following transaction.
This change allows any client commit to change its surface geometry,
even if it is a response to a configure event.
The keyboard group's effective keyboard layout was never being changed
due to a condition that incorrectly preventing it from being performed.
The IPC event that follows the change was correctly being prevented.
To query whether a container is sticky, checking `con->is_sticky` is
insufficient. `container_is_floating_or_child` must also return true;
this led to a lot of repetition.
This commit introduces `container_is_sticky[_or_child]` functions, and
switches all stickiness checks to use them. (Including ones where the
container is already known to be floating, for consistency.)
Previously, `find_edge` on a single fullscreen view would occasionally
return an edge rather than `WLR_EDGE_NONE`. This would trigger entry
into `seatop_resize_tiling`, which doesn't have meaning for a fullscreen
view.
The result was that the fullscreen container hitbox was considered to be
that of where it'd be if it were tiling, so most clicks would not go
through.
Fixes#5792.
When scrolling on a tabbed/stacked container, i3 focuses its
inactive-focused focused child. Sway does the same, but then resets the
focus to whatever was focused previously.
Ref e5992eed16/src/click.c (L207-L219)
The function evacuate_sticky() was changed in commit 32788a93 to be used
by workspace_for_each_container() to make the code more readable. But I
overlooked that it is not safe to use workspace_for_each_container() to
remove container from a workspace. This commit restores the previous
implementation for evacuate_sticky().
Currently, when a floating container with a view is split and
children are added to it, the new views are rendered as tiled,
while the first view stays in floating style.
Here this is addressed by setting the view to tiled as soon
as the container is split, by duplicating the "view part" of
the logic in container_set_floating(..., false). Since the new
container of the view is no longer considered floating, it
makes sense to set the view to tiling at this point.
The view would have to be set back to floating if it was possible
to "unsplit" the container.
Sticky floating containers on an otherwise empty workspace can only be
evacuated if the new output has an active workspace. The noop output may
not have one and in that case we have to move the whole workspace to the
new output.
Currently, in view_autoconfigure, the only condition for show_border
is !view_is_only_visible. view_is_only_visible does not cross the
boundary between the workspace's tiling and floating lists and does not
differentiate between them.
The result is, that in a workspace with zero or more tiling containers
and a single floating container, the floating container will lose its
borders as soon as it is split, provided that a only one view is visible
within the floating container.
Fixed by adjusting the condition for show_borders.
A "resize shrink width 1px" will cause grow_x to be 0 while grow_width is -1,
incorrectly rejecting the command even though the resize is not a noop. Fix
this by checking width/height instead of x/y.
Sway maintains a list of pending transactions, and tries to merge
consecutive transactions applying to the same views into one. Given
a pending transactions list on views {A, B, C} of:
A -> A' -> A'' -> B -> B' -> B''
Sway will collapse the transactions into just A'' -> B''. This works
fine when doing things like resizing views by their border. However,
when interactively resizing layouts like H[V[A B] C], we end up with
pending transaction lists like:
A -> B -> C -> A' -> B' -> C' -> A'' -> B'' -> C''
Previously, Sway would not be able to simplify this transaction list,
and execute many more transactions than would be necessary (the final
state is determined by {A'', B'', C''}).
After this commit, the transaction list gets simplified to A'' -> B'' ->
C'', resolving performance problems (that were particularly noticeable
with high-refresh-rate mice).
Fixes#5736.
Xwayland views are aware of their coordinates, so validating transaction
completions should take into account the reported coordinates of the
view. Prior to this commit they didn't, and matching dimensions would
suffice to validate the transaction.
Also introduced `transaction_notify_view_ready_immediately` to support
the fix from d0f7e0f without jumping through hoops to figure out the
geometry of an `xdg_shell` view.
Sway logical coordinates are doubles, but they get truncated to integers
when sent to Xwayland through `xcb_configure_window`. X11 apps will not
respond to duplicate configure requests (from their truncated point of
view) and cause transactions to time out.
Fixes#5035.
Prior to this commit, having a layout like T[app1 V[app2]], focusing
app2, and then doing `move left` would result in T[app2 app1]. Now, the
resulting layout is T[app1 app2], which matches i3 behavior.
`container_flatten` updates `container->parent`, meaning that the
existing check would never be true.
i3 shows indicators for the workspace-level pseudo-split, but Sway does
not, as of b977c02. This commit replaces the floating container check
with a call to `container_is_floating`, which has some more robust
checks in place.
Fixes#5699.
We can't arm the timer during cursor creation since the config may not
be ready yet. Instead arm the timer while applying the input
configuration, by this time the configuration has been parsed and we can
arm the hide timer.
Fixes#5686
According to the wayland docs, wayland timers are disarmed on creation.
This leads to the cursor not being hidden if there is no activity after
creation, since the timer is armed on activity, but not at creation.
Arm the timer after creation to ensure the cursor is hidden even if
there is no cursor activity after creation.
Fixes#5684
Reset the event source after unhiding the cursor, to ensure that the
timeout starts after showing the cursor. Also remove the open coded
variant in seat_consider_warp_to_focus().
Fixes#5679
My primary issue was IntelliJ IDEA's code suggestion pop-up not returning focus
to the active editing window.
I have spent some time looking at the changes of @Xyene (#5398) and
@RyanDwyer (#2103). I think my proposed change maintains the status
quo for the most part whilst fixing my focus issue.
I have verified that @Xyene's fix for IntelliJ sub-menus still works.
I have done basic testing which consists of:
- Chrome
- IntelliJ IDEA 2020.2.1
- VSCode
- Alacritty
It seems to hold up. I at least didn't see any obvious errors.
Relates to #3007
This changes it so all libinput config options are set on any device
that supports it. Previously, only a subset of libinput config options
were being considered depending on the input type. Instead of trying to
guess which properties the device may support, attempt to set any
configured property regardless of the device type. All of the functions
already have early returns in them for when the device does not actually
support the property. This brings the configuration side inline with
describe_libinput_device for the IPC side. This change was prompted
by a tablet tool showing the calibration matrix property in the IPC
message, but not being able to actually change it since that property
was only being considered for the touch input type.
Instead of listening to both transform and scale events, we can listen
to the commit event and use the new wlr_output_event_commit struct to
decide what to do.
This de-duplicates some of the work we were doing twice when an output
was re-configured.
Depends on [1].
[1]: https://github.com/swaywm/wlroots/pull/2315
If the environment variable is not defined, getenv returns NULL.
Passing a NULL pointer to the "%s" format specifier is undefined
behavior. Even if some implementations output "(null)", an empty
string is nicer.
`!*rgba` tests if the first byte of rgba isn't `'\0'`.
`hex_to_rgba_hex` returns NULL if `parse_color` fails. There's a null
pointer dereference in that case. The intended behavior is `!rgba`.
The pointer `data` is cast to a more strictly aligned pointer type. To
prevent issues, the `data32` buffer is removed and its occurrences are
replaced with an offset from the `data` buffer.
If the mouse/cursor/pointer is near the edge of an output when a "move
position to pointer" command is run, then the floating container will be
constrained to fit inside the bounds of the output as much as possible.
This behavior matches what i3 does in this scenario. I also think it is
a better user experience.
Relates to #4906
The logic for the bounds check follows the implementation in i3: 7330778223/src/floating.c (L536)