Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

view: Implement set-position and set-geometry #449

Closed
wants to merge 1 commit into from

Conversation

ThreeFx
Copy link
Contributor

@ThreeFx ThreeFx commented Oct 9, 2021

The command set-position x y sets the current view's coordinates to be
(x, y) relative to the current output. The view will always remain
within the bounds of the current output. Any parts of the view that end
up outside of the current output are silently truncated.

The command set-geometry w h sets the geometry of a view to be w
wide and h high. Any parts of the view that end up outside of the
current output's bounding box are silently truncated.

View truncation is applied as shown below:

+-------------+              +-------------+
|   output    |              |   output    |
|      +------+--+  ------>  |      +------+
|      | view |  |           |      | view |
+------+------+  |           +------+------+
       +---------+

In addition to the above commands, this refactors the move and resize functions to use the setPosition and setGeometry commands. This way there is only one true way of setting position and geometry of windows.

@ThreeFx
Copy link
Contributor Author

ThreeFx commented Oct 9, 2021

This closes #368 btw :)

Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the above commands, this refactors the move and resize functions to use the setPosition and setGeometry commands. This way there is only one true way of setting position and geometry of windows.

This is very nice. There's still one place left with duplicate logic however, see the .resize branch in Cursor.processMotion(). I think that could be replaced with the new View.resize() as well, though maybe it's better to hold off on that refactor for now to avoid conflicts with #399.

On a higher level, I'm curious what you use these new commands for in your workflow. With the current restriction of functioning only on the currently focused view I'm not sure how I would use them...

river/View.zig Outdated Show resolved Hide resolved
river/View.zig Outdated Show resolved Hide resolved
@ThreeFx
Copy link
Contributor Author

ThreeFx commented Oct 11, 2021

It's nice for positioning some windows as part of a larger setup (e.g. a terminal running find <zig files> | entr zig build). Other than that: The issue told me to do it :P

@ifreund
Copy link
Member

ifreund commented Oct 12, 2021

It's nice for positioning some windows as part of a larger setup (e.g. a terminal running find <zig files> | entr zig build).

hmm, could you elaborate a bit more on this? From a code standpoint this PR seems solid, I'm just not yet convinced that this feature is widely useful due to the restriction to always targeting the focused view.

@ThreeFx
Copy link
Contributor Author

ThreeFx commented Oct 13, 2021

hmm, could you elaborate a bit more on this?

During my Master's thesis, I used something like this to set up my initial workspace(s) every morning:

riverctl spawn "alacritty --with --some --extra --args"
sleep 0.2
riverctl move right 100

riverctl spawn "other stuff"
sleep 0.2
riverctl snap up

Having set-position and set-geometry makes this process a bit nicer.

The command `set-position x y` sets the current view's coordinates to be
(x, y) relative to the current output. The view will always remain
within the bounds of the current output. Any parts of the view that end
up outside of the current output are silently truncated.

The command `set-geometry w h` sets the geometry of a view to be `w`
wide and `h` high. Any parts of the view that end up outside of the
current output's bounding box are silently truncated.

View truncation is applied as shown below:

    +-------------+              +-------------+
    |   output    |              |   output    |
    |      +------+--+  ------>  |      +------+
    |      | view |  |           |      | view |
    +------+------+  |           +------+------+
           +---------+
@ifreund
Copy link
Member

ifreund commented Oct 20, 2021

Hit a panic when attempting to move a view past the edge of the output in a nested session:

debug(cursor): enter move cursor mode
thread 26410 panic: attempt to cast negative value to unsigned integer
/home/ifreund/projects/river/river/View.zig:559:31: 0x29bd89 in View.setPositionAndGeometry (river)
    self.pending.box.height = @intCast(u32, y_end - y_start);
                              ^
/home/ifreund/projects/river/river/View.zig:504:32: 0x276985 in View.setPosition (river)
    self.setPositionAndGeometry(x, y, self.pending.box.width, self.pending.box.height);
                               ^
/home/ifreund/projects/river/river/View.zig:438:21: 0x24a871 in View.move (river)
    self.setPosition(self.pending.box.x + dx, self.pending.box.y + dy);
                    ^
/home/ifreund/projects/river/river/Cursor.zig:754:22: 0x233757 in Cursor.processMotion (river)
            view.move(@floatToInt(i32, delta_x), @floatToInt(i32, delta_y));
                     ^
/home/ifreund/projects/river/river/Cursor.zig:382:23: 0x232f81 in Cursor.handleMotionAbsolute (river)
    self.processMotion(event.device, event.time_msec, dx, dy, dx, dy);
                      ^
/home/ifreund/projects/river/zig-cache/zig-wayland/wayland_server_core.zig:492:25: 0x230a40 in .wayland.wayland_server_core.struct:490:17.wrapper (river)
                        @call(.{ .modifier = .always_inline }, notify, .{ listener, @intToPtr(T, @ptrToInt(data)) });
                        ^
../util/signal.c:29:3: 0x7f9e85123f9b in wlr_signal_emit_safe (../util/signal.c)
../types/wlr_cursor.c:430:2: 0x7f9e850f9396 in handle_pointer_motion_absolute (../types/wlr_cursor.c)
../util/signal.c:29:3: 0x7f9e85123f9b in wlr_signal_emit_safe (../util/signal.c)
../backend/wayland/seat.c:104:2: 0x7f9e850d9ac2 in pointer_handle_motion (../backend/wayland/seat.c)
???:?:?: 0x7f9e84ce9e6c in ??? (???)
Panicked during a panic. Aborting.

@ifreund
Copy link
Member

ifreund commented Oct 20, 2021

I've come around on having this feature in river. It's not really that complex and lets us unify some code paths. We should also be able to get rid of the separate resize handling in Cursor.processMotion() eventually though that could be done in a follow up PR.

@Danacus
Copy link
Contributor

Danacus commented Nov 25, 2021

To add to what @ThreeFx already said, this feature could be useful for floating terminals. For example I use a floating terminal with an mpc client that can toggled with a key binding.

scratch_tag=$((1 << 20 ))
mpc_tag=$((1 << 21 ))

riverctl map normal Mod4 W toggle-focused-tags ${scratch_tag}
riverctl map normal Mod4+Shift M toggle-focused-tags ${mpc_tag}

riverctl set-focused-tags ${scratch_tag}

riverctl spawn "alacritty"
sleep 0.1
riverctl toggle-float
riverctl resize horizontal 200
riverctl resize vertical 200

riverctl set-focused-tags ${mpc_tag}

riverctl spawn "alacritty -e mpc-rs"
sleep 0.1
riverctl toggle-float
riverctl resize horizontal -100
riverctl resize vertical -200
riverctl snap down

riverctl set-focused-tags 1

set-position and set-geometry would give more control compared to snap and resize.

@justinesmithies
Copy link

I've come around on having this feature in river. It's not really that complex and lets us unify some code paths. We should also be able to get rid of the separate resize handling in Cursor.processMotion() eventually though that could be done in a follow up PR.

Is this feature still planned for River as I'd find it extremely useful ?

@ifreund
Copy link
Member

ifreund commented Jul 3, 2023

Is this feature still planned for River as I'd find it extremely useful ?

I'm not opposed to it, though I don't have plans to implement it myself. In any case this PR is unfortunately quite out of date now.

I'd recommend anyone interested on working on this to wait until I resolve the issues described in #828 to avoid conflicts and save time for both of us.

@ifreund ifreund closed this Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants