[DONE] RFT: bl-aerosnap

Unia · 2016-09-01 11:09:38

Hello all,

I've been in the process of rewriting bl-aerosnap to use XLib and EWMH as opposed to subprocesses calling out to xdotool and wmctrl to do the work. Using XLib and EWMH directly is a cleaner and faster solution, which I hope improves performance, reliability and maintainability. In the process, I have also added --top and --bottom flags, so that you can also snap your windows to the top and bottom halfs of your monitor.

All of this work changed the code of bl-aerosnap quite significantly. Whilst (for me at least), it never quite worked reliably, I do want to make sure that my changes do what I intended them to do. Hence this thread, asking for your testing and feedback. I believe this is the first time such a thread is made, so bear with me on the formatting and structure (I suppose we're testing two things at once here; bl-aerosnap and RFT threads!)

SETUP:
You'll have to clone my GitHub repository, so you'll have to have git installed. You'll also need python and python-xlib (which you'll likely already have installed if running a BunsenLabs system).

$ sudo apt-get install git python python-xlib

To get the new bl-aerosnap script:

$ git clone https://github.com/Unia/bunsen-utilities.git
$ cd bunsen-utilities
$ git checkout bl-aerosnap-rewrite
# Copy the first script found below, using your favorite text editor
$ $EDITOR bl-aerosnap-test-1.sh
# Copy the second script found below, using your favorite text editor. Edit this script as is indicated below, or skip it and run the test manually (see below).
$ $EDITOR bl-aerosnap-test-2.sh
$ chmod +x bl-aerosnap-test-?.sh
$ ./bl-aerosnap-test-1.sh
# Make note of the output, provide feedback to me
$ ./bl-aerosnap-test-2.sh
# Make note of the output, provide feedback to me

TEST 1: Window snapping
This test is to confirm the window snapping of bl-aerosnap, which works as follows:

usage: bl-aerosnap [-h] [-l] [-r] [-t] [-b]

optional arguments:
  -h, --help    show this help message and exit
  -l, --left    attempt to snap the active window to the left of the screen
  -r, --right   attempt to snap the active window to the right of the screen
  -t, --top     attempt to snap the active window to the top of the screen
  -b, --bottom  attempt to snap the active window to the bottom of the screen

When the user supplies one of the directional arguments (left, right, top and bottom) the active window (in this case likely the terminal window used to call bl-aerosnap) should snap towards that direction. When calling bl-aerosnap again (with the same positional argument!), bl-aerosnap should restore the window to its original place. I have written a quick script to do this for me in an automated manner, which snaps and restores the active window to each direction ten times, comparing the positions. If an anomaly is found, the script halts.

#!/usr/bin/env sh

# Remove old history, if any
rm "/tmp/bl-aerosnap-$(id -u)"

# Test regular snap/restore, by snapping and restoring the active window 10
# times per snapping direction. If the debug prints are not equal, the test is
# stopped and exists with an exit status of 1.
for i in "left" "right" "top" "bottom"; do
    for j in $(seq 1 10); do
        FROM="$(python bin/bl-aerosnap --$i | grep "from" | cut -c23- -)"
        TO="$(python bin/bl-aerosnap --$i | grep "Restoring" | cut -c15- -)"
        if [ "$FROM" != "$TO" ]; then
            printf "Geometries do not match:\nFrom: %s\nTo: %s\n" "$FROM" "$TO" 1>&2
            exit 1
        fi
    done
done

TEST 2: changing your mind
Imagine the following scenario: the user snaps the active window to the left, but now wants it snapped to the right instead. In this case, several things should happen:

1. The user should be able to directly snap to the right, without having to restore the window first.
2. The original position (the one prior to snapping to the left) should be remembered to restore to.

This test tests if this happens, by first snapping to the left and then to the right. If it works for these two directions and this combination, it works for all directions and combinations. This script is not very clean and is highly setup dependent (line ""$RIGHT" != "0 0 681 713""; for me this is a 1366x768 laptop screen, with a single tint2 panel in the bottom with a height of 30 pixels), so you'll likely have to adapt it a bit. If this is too difficult for you, manually snapping and restoring and comparing the numbers will do just fine too! Suggestions on making the test script setup-agnostic are also appreciated.

#!/usr/bin/env sh

# Remove old history, if any
rm "/tmp/bl-aerosnap-$(id -u)"

# This piece of code tests whether the above happens.
ORIG="$(python bin/bl-aerosnap --left | grep "from" | cut -c23- -)"
RIGHT="$(python bin/bl-aerosnap --right | grep "from" | cut -c23- -)"
if [ "$RIGHT" != "0 0 681 713" ]; then
    printf "Not snapping from the left" 1>&2
    exit 1
fi
TO="$(python bin/bl-aerosnap --right | grep "Restoring" | cut -c15- -)"
if [ "$ORIG" != "$TO" ]; then
    printf "Geometries do not match:\nFrom: %s\nTo: %s\n" "$ORIG" "$TO" 1>&2
    exit 1
fi

TEST 3: for the experienced Python programmers among us
There is one serious issue left, namely that windows that originally have a negative position will not be able to be restored. Help with this issue would be appreciated, as it looks like it is quite a deep issue. A traceback of this issue is found in my PR: https://github.com/BunsenLabs/bunsen-utilities/pull/54. Please have a look and see if you can help resolve this issue. If you can, please contact me either through email, through a PM, or by replying on the GitHub PR.

Conclusion:

Please provide the output of the tests, in combination with details about your setup (amount of panels, panel heights, window manager, desktop environment, amount of monitors, et cetera). When something goes wrong, please be as specific as possible in describing what went wrong. I would appreciate it if you could use the following format in your replies:

[b]TEST 1:[/b] PASS/FAIL
<if fail: details, what went wrong>
[b]TEST 2:[/b] PASS/FAIL
<if fail: details, what went wrong>
[b]REMARKS:[/b] <any optional remarks>

Thank you for taking the time to help us make BunsenLabs better!

damo · 2016-09-01 11:46:44

Immediate feedback, without too much investigation....

Dual 1920x1080 monitors, Openbox, Tint2 panels on each monitor (bottom, 40px high)

TEST 1:
No output

Window moves a few px with every reload

damo@hydrogen bunsen-utilities $ xwininfo | grep geometry
  -geometry 102x56+2044+64
damo@hydrogen bunsen-utilities $ ./bl-aerosnap-test-1.sh 
damo@hydrogen bunsen-utilities $ xwininfo | grep geometry
  -geometry 102x56+2084--38

FAIL?

TEST 2:
With no script edits, the output is:

damo@hydrogen bunsen-utilities $ ./bl-aerosnap-test-2.sh 
Not snapping from the left

PASS/FAIL?

REMARKS:
The only command switches which work are the long form "--left" and "--right"

damo@hydrogen bunsen-utilities $ ./bin/bl-aerosnap -l
bl-aerosnap: usage:
  --help     show this message and exit
  --left     attempt to snap active window to left of screen
  --right    attempt to snap active window to right of screen

This isn't surprising because I only see those in the script! ie no bottom, top, or short forms for any of them.

The snaps on dual monitors are far left and far right of the full desktop, not the monitor the window starts on. This would be a show-stopper for me

The window moves are 4x faster than my winsnap bash script that I use, using xdotool and friends!

Unia · 2016-09-01 11:56:47

I did a derp, forgot to instruct you to check out the branch that actually contains my fixes

OP updated. For brevity, after cloning the git repository, run

git checkout bl-aerosnap-rewrite

damo wrote:

TEST 2:
With no script edits, the output is...

That's not gonna work, as I said in the OP that script is highly setup dependent. Comparing mine with yours:

1. 1366x768, single monitor, one panel, 30 pixels high
2. 1920x1080, double monitor, one panel (on each), 40 pixels high

Of course that's not going to work What you can do (if you don't want to edit the script) is just run the test manually:

1. Snap to the left;
2. Snap to the right - did the window snap to the right immediately? Yes: go to step 3, No: tell me.
3. Snap to the right to restore the window - did the window restore to where it was prior to it being snapped to the left? Yes: it works, tell me. No: it doesn't work, tell me.

damo · 2016-09-01 12:17:14

master/bl-aerosnap
Snapping and restoring to left/right edges of desktop is snappy and accurate

bl-aerosnap-rewrite/bl-aerosnap

damo@hydrogen bunsen-utilities $ ./bin/bl-aerosnap --left
Xlib.protocol.request.QueryExtension
Traceback (most recent call last):
  File "./bl-aerosnap", line 298, in <module>
    main()
  File "./bl-aerosnap", line 279, in main
    load_history(history)
  File "./bl-aerosnap", line 166, in load_history
    WINDOWS = eval(hist_file.read())
  File "<string>", line 0
    
    ^
SyntaxError: unexpected EOF while parsing

Is this a path problem for the hist_file? Or where I am running the script from?

$ pwd
/home/damo/git-repos/unia/bunsen-utilities

Unia · 2016-09-01 17:28:10

The format for the history file has changed, so when you run the new version after the old without removing the history file in between things go wrong.

Which is why I provided the scripts that remove the history file just in case.

damo · 2016-09-01 19:52:13

Trying it in a VM, with a single monitor setup. The root window appears as 1920x1052 in the VM.

Left/right snapping and restoring is good.
Top snapping and restoring is good.

Bottom snapping doesn't go all the way, leaving a gap. This happens whether Tint2 is at the top or bottom.
The window values (with xwininfo, terminal window; top 30px Tint2) for Initial position, Bottom position and Restore position.....

Corners:  +96+49  --1+49  --1-1  +961-1

Corners:  +0+531  --0+531  --0-44  +0-44      <--- 44px from bottom of screen

Corners:  +96+49  --1+49  --1-1  +961-1

If you need I can repeat with full xwinifo output, and screenshots?

There is a small issue with terminal emulator windows having small gaps around them, or overlapping snapped windows slightly. This is nothing to do with the script, but how the WM gets the wrong values reported to it. I hd the same problem with my script, and started to write test functions for the different terms and terminals - some are OK, some aren't. In the end I lived with the 4px error from the misbehaving windows!

Unia · 2016-09-01 21:32:37

Most terminal windows do not support arbitrary sizes, rather they resize by character columns. If you give me the details if your terminal, we can figure out if we can work around this (some terminals provide ways to resize by pixels, like termite which I use).

damo · 2016-09-01 21:57:58

I was using default BL Terminator. IIRC xfce4-terminal also misbehaved, but gnome-terminal was OK

ohnonot · 2016-09-02 06:37:32

single monitor, 1680x1050 resolution, openbox, single tint2 panel on bottom 30px high, running archlinux:

TEST1: no output, all seems to be working normally (quick snapping/restoring, last position equals original position)

TEST2: "Not snapping from the left" - both with original values, later edited to "$RIGHT" != "0 0 830 990", adjusted values a few pixels more/less, always the same output.

looks good to me.
i'd love to see this working on archlinux. hopefully the testing is helpful.
i'll look into the 3rd test later, haven't found it yet.

Unia · 2016-09-02 10:03:04

damo wrote:

I was using default BL Terminator. IIRC xfce4-terminal also misbehaved, but gnome-terminal was OK

My apologies, I should have thought of adding that information to the OP. Can I assume that both test 1 and test 2 now passed?

ohnonot wrote:

TEST1: no output, all seems to be working normally (quick snapping/restoring, last position equals original position)

No output means it worked!

ohnonot wrote:

TEST2: "Not snapping from the left" - both with original values, later edited to "$RIGHT" != "0 0 830 990", adjusted values a few pixels more/less, always the same output.

What behavior did you notice? If the window first snapped to the left, then to the right and then back to its original position before being snapped to the left, it works.

If you (or anyone else) wants to fix their test 2 script: your coordinate string doesn't seem right to me. It reads as "x y width height", so when your window is snapped to the left it appears at coordinates 0,0 (for a single monitor this is correct) with a width of 830 and a height of 990.

You say you have a monitor with a 1680x1050 resolution with a single 30px high panel on the bottom. This leaves 1680x1020 pixels to snap windows to. When snapping to the left or the right, your window should thus have a width of (1680/2) - <width of your window manager's decorations on the left and the right>. Assuming these are 1 pixel on both sides, you should thus have a width of (1680/2) - 2 = 838 pixels. The height works the same; assuming a 10 pixel titlebar and a 3 pixel bottom decoration, the window's height should be 1050 - 13 = 1037. You can find these dimensions by running "xprop | grep _NET_FRAME_EXTENTS" and selecting your terminal window. The values read left, right, top, bottom.

ohnonot wrote:

i'd love to see this working on archlinux. hopefully the testing is helpful.

I develop it on Arch Linux Your testing has most certainly proven helpful; I think I can assume everything worked correctly.

damo · 2016-09-02 10:10:23

Unia wrote:

...Can I assume that both test 1 and test 2 now passed?...

The tests were OK. There was just the issue with windows not snapping to the bottom of the screen.

BTW my Openbox margins are all set to '0'. Are you leaving a 1px margin with the snapped windows? @corenominal set up OB like that so that the r-click menu was still available by clicking on a screen edge.

Unia · 2016-09-02 10:16:49

damo wrote:

There was just the issue with windows not snapping to the bottom of the screen.

Can you elaborate on this, and possibly include a screenshot?

damo wrote:

BTW my Openbox margins are all set to '0'. Are you leaving a 1px margin with the snapped windows? @corenominal set up OB like that so that the r-click menu was still available by clicking on a screen edge.

Margins are set using struts, which are taken into account by the window manager when I ask for the work area of the desktop. Indeed, when I set a top margin the window is neatly placed against it with the correct height. However, windows that are now snapped to the bottom receive an incorrect y coordinate (whilst still getting the correct height). The same happens with vertical margins. I'll add this to my list of issues to resolve -- thanks for the tip!

damo · 2016-09-02 10:54:43

This is what I see, using a default BL in Virtualbox (screen dims reported as 1920x1052)

Note the 44px gap when bottom snapping with Tint2 at the top. Is the script duplicating panel values in the wrong place?

Initial positions

Snap left

Snap bottom

Snap bottom, 2 windows (NB terminal is smaller)

Snap top

Snap top, 2 windows (NB terminal is smaller)

Tint2 at bottom, 2 windows, snap bottom and top (NB terminal is smaller)

Tint2 at bottom, 2 windows, snap left and right

Unia · 2016-09-02 11:01:02

Got it... back to work!

Unia · 2016-09-02 11:55:24

^ The above issue has been resolved in the latest commit. @damo, could you please pull my changes and try again?

xaos52 · 2016-09-02 12:09:36

@Unia,

Here is the test results on my setup:
Both tests run without output, so:

TEST 1: PASS
TEST 2: PASS
REMARKS:

My setup:
An external screen 1920x1080+0+0 to the left of my laptop screen 1366x768+1920+0 using xinerama:

Due to the differences in screen sizes, I get following results:

Screen before snap - all windows on the external monitor.

After areosnap --left - window is on 1 monitor, but with half of the combined width.

After aerosnap --right - window spans 2 monitors with half of the combined width.

Arandr

xrandr ouput

(bl-aerosnap-rewrite U:1 ?:2 ✗) /h/m/t/t/U/bunsen-utilities xrandr
Screen 0: minimum 320 x 200, current 3286 x 1080, maximum 8192 x 8192
LVDS1 connected 1366x768+1920+0 (normal left inverted right x axis y axis) 344mm x 194mm
   1366x768      60.00*+  40.00  
   1360x768      59.80    59.96  
   1024x768      60.00  
   800x600       60.32    56.25  
   640x480       59.94  
VGA1 disconnected (normal left inverted right x axis y axis)
HDMI1 connected primary 1920x1080+0+0 (normal left inverted right x axis y axis) 531mm x 299mm
   1920x1080     60.00*+  50.00    59.94  
   1920x1080i    60.00    50.00    59.94  
   1680x1050     59.88  
   1440x900      59.90  
   1280x960      60.00  
   1280x800      59.91  
   1152x864      75.00  
   1280x720      60.00    50.00    59.94  
   1440x576i     50.00  
   1024x768      70.07    60.00  
   1440x480i     60.00    59.94  
   800x600       60.32    56.25  
   720x576       50.00  
   720x480       60.00    59.94  
   640x480       66.67    60.00    59.94  
   720x400       70.08  
DP1 disconnected (normal left inverted right x axis y axis)
(bl-aerosnap-rewrite U:1 ?:2 ✗) /h/m/t/t/U/bunsen-utilities

As you can see from above shots, your program uses the combined width and height of both monitors to determine its scrren dimensions.

In my case I would prefer the snapping to occur with the dimensions of 'the active monitor', on the active monitor only.

People with 2 monitors of the same size might prefer the program to work as it is now.

So it seems like the program needs an option to select which of the two 'program modes' is preferred by the user.

Conc the python problem with negative coordinates, I am in the process of debugging this.

damo · 2016-09-02 13:15:51

Unia wrote:

^ The above issue has been resolved in the latest commit. @damo, could you please pull my changes and try again?

Well done - that fix seems to be snapping properly

Unia · 2016-09-02 15:58:55

@xaos52, thanks for your testing. You confirmed my suspision that multimonitor support was not complete. Tomorrow I have access again to my own second monitor, so I'll have the ability to fix it then.

ohnonot · 2016-09-03 05:32:11

still single monitor 1680x1050, archlinux, openbox, tint2 at bottom 30px:

re TEST2:

Unia wrote:

What behavior did you notice? If the window first snapped to the left, then to the right and then back to its original position before being snapped to the left, it works.

i did another pull, but the behavior is exactly the same as yesterday: it says "not snapping to the left", it quickly snaps to the left, then to the right, and does not return to its original position.

If you (or anyone else) wants to fix their test 2 script: your coordinate string doesn't seem right to me. It reads as "x y width height", so when your window is snapped to the left it appears at coordinates 0,0 (for a single monitor this is correct) with a width of 830 and a height of 990.
You say you have a monitor with a 1680x1050 resolution with a single 30px high panel on the bottom. This leaves 1680x1020 pixels to snap windows to. When snapping to the left or the right, your window should thus have a width of (1680/2) - <width of your window manager's decorations on the left and the right>. Assuming these are 1 pixel on both sides, you should thus have a width of (1680/2) - 2 = 838 pixels. The height works the same; assuming a 10 pixel titlebar and a 3 pixel bottom decoration, the window's height should be 1050 - 13 = 1037. You can find these dimensions by running "xprop | grep _NET_FRAME_EXTENTS" and selecting your terminal window. The values read left, right, top, bottom.

ok, but does it matter if the values leave plenty of space? does it have to be pixel-precise? my titlebar is higher than 10px + borders, it is not possible to have a precise value when changing openbox themes.

anyhow, thanks, this is cool!

btw, i don't like the 1px margin so much, is this configurable?

[ ps, not so important: testing (on arch, and providing dash is installed) is quicker & smoother if you replace the headers in the test scripts from "#!/usr/bin/env sh" to "#!/usr/bin/env dash" ]

xaos52 · 2016-09-03 09:43:01

@Unia,

Conc the crash with negative offsets:

The error occurs when the library converts the x coordinate to an unsigned int, realizes that it loses information, and crashes.
This may be intended behaviour, to protect the user from the window-manager placing a window in a position where it may not be reachable in any way by the user.

Just an 'educated guess', really.

EDIT:

See source code for libxcb-ewmh2:

/**
 * _NET_MOVERESIZE_WINDOW
 */

/* x, y, width, height may be equal to -1 */
xcb_void_cookie_t
xcb_ewmh_request_moveresize_window(xcb_ewmh_connection_t *ewmh, int screen_nbr,
                                   xcb_window_t moveresize_window,
                                   xcb_gravity_t gravity,
                                   xcb_ewmh_client_source_type_t source_indication,
                                   xcb_ewmh_moveresize_window_opt_flags_t flags,
                                   uint32_t x, uint32_t y,
                                   uint32_t width, uint32_t height)

x, y, width, height are unsigned ints.

#1 2016-09-01 11:09:38

[DONE] RFT: bl-aerosnap

#2 2016-09-01 11:46:44

Re: [DONE] RFT: bl-aerosnap

#3 2016-09-01 11:56:47

Re: [DONE] RFT: bl-aerosnap

#4 2016-09-01 12:17:14

Re: [DONE] RFT: bl-aerosnap

#5 2016-09-01 17:28:10

Re: [DONE] RFT: bl-aerosnap

#6 2016-09-01 19:52:13

Re: [DONE] RFT: bl-aerosnap

#7 2016-09-01 21:32:37

Re: [DONE] RFT: bl-aerosnap

#8 2016-09-01 21:57:58

Re: [DONE] RFT: bl-aerosnap

#9 2016-09-02 06:37:32

Re: [DONE] RFT: bl-aerosnap

#10 2016-09-02 10:03:04

Re: [DONE] RFT: bl-aerosnap

#11 2016-09-02 10:10:23

Re: [DONE] RFT: bl-aerosnap

#12 2016-09-02 10:16:49

Re: [DONE] RFT: bl-aerosnap

#13 2016-09-02 10:54:43

Re: [DONE] RFT: bl-aerosnap

#14 2016-09-02 11:01:02

Re: [DONE] RFT: bl-aerosnap

#15 2016-09-02 11:55:24

Re: [DONE] RFT: bl-aerosnap

#16 2016-09-02 12:09:36

Re: [DONE] RFT: bl-aerosnap

#17 2016-09-02 13:15:51

Re: [DONE] RFT: bl-aerosnap

#18 2016-09-02 15:58:55

Re: [DONE] RFT: bl-aerosnap

#19 2016-09-03 05:32:11

Re: [DONE] RFT: bl-aerosnap

#20 2016-09-03 09:43:01

Re: [DONE] RFT: bl-aerosnap

Board footer