ssokolow / quicktile

Adds window-tiling hotkeys to any X11 desktop. (An analogue to WinSplit Revolution for people who don't want to use Compiz Grid)
https://ssokolow.com/quicktile/
GNU General Public License v2.0
869 stars 78 forks source link

Use round for pixel numbers calculations #82

Closed avaly closed 7 years ago

avaly commented 7 years ago

Without this for certain COLUMN_COUNT values, I see the following positions:

('positions', {
  'right': [(0.5, 0.0, 0.5, 1), (0.9, 0.0, 0.1, 1), (0.8, 0.0, 0.2, 1), (0.7, 0.0, 0.3, 1), (0.6, 0.0, 0.4, 1), (0.5, 0.0, 0.5, 1), (0.4, 0.0, 0.6, 1), (0.30000000000000004, 0.0, 0.7, 1), (0.19999999999999996, 0.0, 0.8, 1), (0.09999999999999998, 0.0, 0.9, 1)],
  'bottom': [(0.0, 0.5, 1.0, 0.5), (0.45, 0.5, 0.1, 0.5), (0.4, 0.5, 0.2, 0.5), (0.35, 0.5, 0.3, 0.5), (0.3, 0.5, 0.4, 0.5), (0.25, 0.5, 0.5, 0.5), (0.2, 0.5, 0.6, 0.5), (0.15000000000000002, 0.5, 0.7, 0.5), (0.09999999999999998, 0.5, 0.8, 0.5), (0.04999999999999999, 0.5, 0.9, 0.5)],
  'top': [(0.0, 0.0, 1.0, 0.5), (0.45, 0.0, 0.1, 0.5), (0.4, 0.0, 0.2, 0.5), (0.35, 0.0, 0.3, 0.5), (0.3, 0.0, 0.4, 0.5), (0.25, 0.0, 0.5, 0.5), (0.2, 0.0, 0.6, 0.5), (0.15000000000000002, 0.0, 0.7, 0.5), (0.09999999999999998, 0.0, 0.8, 0.5), (0.04999999999999999, 0.0, 0.9, 0.5)],
  'middle': [(0.0, 0.0, 1.0, 1), (0.45, 0.0, 0.1, 1), (0.4, 0.0, 0.2, 1), (0.35, 0.0, 0.3, 1), (0.3, 0.0, 0.4, 1), (0.25, 0.0, 0.5, 1), (0.2, 0.0, 0.6, 1), (0.15000000000000002, 0.0, 0.7, 1), (0.09999999999999998, 0.0, 0.8, 1), (0.04999999999999999, 0.0, 0.9, 1)],
  'bottom-right': [(0.5, 0.5, 0.5, 0.5), (0.9, 0.5, 0.1, 0.5), (0.8, 0.5, 0.2, 0.5), (0.7, 0.5, 0.3, 0.5), (0.6, 0.5, 0.4, 0.5), (0.5, 0.5, 0.5, 0.5), (0.4, 0.5, 0.6, 0.5), (0.30000000000000004, 0.5, 0.7, 0.5), (0.19999999999999996, 0.5, 0.8, 0.5), (0.09999999999999998, 0.5, 0.9, 0.5)],
  'bottom-left': [(0.0, 0.5, 0.5, 0.5), (0.0, 0.5, 0.1, 0.5), (0.0, 0.5, 0.2, 0.5), (0.0, 0.5, 0.3, 0.5), (0.0, 0.5, 0.4, 0.5), (0.0, 0.5, 0.5, 0.5), (0.0, 0.5, 0.6, 0.5), (0.0, 0.5, 0.7, 0.5), (0.0, 0.5, 0.8, 0.5), (0.0, 0.5, 0.9, 0.5)],
  'top-right': [(0.5, 0.0, 0.5, 0.5), (0.9, 0.0, 0.1, 0.5), (0.8, 0.0, 0.2, 0.5), (0.7, 0.0, 0.3, 0.5), (0.6, 0.0, 0.4, 0.5), (0.5, 0.0, 0.5, 0.5), (0.4, 0.0, 0.6, 0.5), (0.30000000000000004, 0.0, 0.7, 0.5), (0.19999999999999996, 0.0, 0.8, 0.5), (0.09999999999999998, 0.0, 0.9, 0.5)],
  'top-left': [(0.0, 0.0, 0.5, 0.5), (0.0, 0.0, 0.1, 0.5), (0.0, 0.0, 0.2, 0.5), (0.0, 0.0, 0.3, 0.5), (0.0, 0.0, 0.4, 0.5), (0.0, 0.0, 0.5, 0.5), (0.0, 0.0, 0.6, 0.5), (0.0, 0.0, 0.7, 0.5), (0.0, 0.0, 0.8, 0.5), (0.0, 0.0, 0.9, 0.5)],
  'left': [(0.0, 0.0, 0.5, 1), (0.0, 0.0, 0.1, 1), (0.0, 0.0, 0.2, 1), (0.0, 0.0, 0.3, 1), (0.0, 0.0, 0.4, 1), (0.0, 0.0, 0.5, 1), (0.0, 0.0, 0.6, 1), (0.0, 0.0, 0.7, 1), (0.0, 0.0, 0.8, 1), (0.0, 0.0, 0.9, 1)]
})
coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 40.902% when pulling 9f4975be77de0f23ca07e706b112983b525de91b on avaly:fix/round-numbers into f72b23bbf7b644906d15c49d02b2e9c71b0d001f on ssokolow:master.

ssokolow commented 7 years ago

Sorry for not responding earlier. Bit of a busy day. I'll have to test this a bit, but I'll try to get to it tomorrow or the following day.

If I can make time to work on QuickTile, this problem will get an even better solution, since I want to rework the tiling so that it actually calculates a grid and then just snaps windows into it. (Among other things, that'll provide for functionality like #10, which would allow a single keypress to resize all windows sharing a row/column with the current one.)

ssokolow commented 7 years ago

Sorry again for putting this off. To be perfectly honest, I just caught myself procrastinating trying to reverse-engineer the COLUMN_COUNT value which exhibits the problem so I could test the effects.

Mind telling me?

avaly commented 7 years ago

I used COLUMN_COUNT = 10 for the example above.

ssokolow commented 7 years ago

Hmm. COLUMN_COUNT = 10 and no other change produces broken results on my system with either version of the codebase. I think this might just be untestable on my 1280x1024 monitors.

That said, I don't see any significant regressions with the default COLUMN_COUNT = 3.

It does change a known bug (there's a 1px gap that moves from between columns 2 and 3 to between columns 1 and 2) but that's no reason to keep from merging it.

...and I really need to stop putting off setting up the types of automated regression tests which require a mock X server on Travis CI. This could've been done in 2 minutes if I'd automated that.