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
867 stars 78 forks source link

KDE doesn't cycle properly because of window geometry issues #1

Closed danhs closed 13 years ago

danhs commented 15 years ago

KDE can't cycle properly because of window geometry issues.

I wrote a quick fix so that cycleDimensions uses Euclidean distance to determine if a window dimension is "identical" to one of the options. This way, the fact that it will never perfectly match ideal dimensions isn't an issue.

I'm using Kubuntu 8.10.

Here's the diff:


diff --git a/quicktile.py b/quicktile.py
index db7510c..612fe4c 100755
--- a/quicktile.py
+++ b/quicktile.py
@@ -41,7 +41,7 @@ **author**  = "Stephan Sokolow (deitarion/SSokolow)"
 **version** = "0.1.4"
 **license** = "GNU GPL 2.0 or later"
## 

+from heapq import heappop, heappush
 import pygtk
 pygtk.require('2.0')

@@ -236,12 +236,25 @@ class WindowManager(object):
                         int(tup[2] \* monitorGeom.width),
                         int(tup[3] \* monitorGeom.height)))
-        result = gtk.gdk.Rectangle(*dims[0])
-        for pos, val in enumerate(dims):
-            if tuple(winGeom) == tuple(val):
-                result = gtk.gdk.Rectangle(*dims[(pos + 1) % len(dims)])
-                break
-        logging.debug("winGeom %r", tuple(winGeom))
- ```
     logging.debug("dims %r", dims)
  ```
- ```
     euclid_distance = []
  ```
-        for pos, val in enumerate(dims):
-            distance = sum([(wg-vv)**2 for (wg, vv) in zip(tuple(winGeom), tuple(val))])
-            heappush(euclid_distance, (distance, pos))
  +
-        # Get minimum euclidean distance.
-        pos = heappop(euclid_distance)[1]
-        result = gtk.gdk.Rectangle(*dims[(pos + 1) % len(dims)])
-   
  +#        result = gtk.gdk.Rectangle(_dims[0])
  +#        for pos, val in enumerate(dims):
  +#            if tuple(winGeom) == tuple(val):
  +#                result = gtk.gdk.Rectangle(_dims[(pos + 1) % len(dims)])
  +#                break
  +
-        logging.debug("result %r", tuple(result))
       self.reposition(win, result, monitorGeom)
       return result
  <\code><\pre>            
ssokolow commented 15 years ago

Thanks. I'll take a look at that as soon as I've finished a couple of nearly-overdue projects.

In future, it's probably best to leave your forked repo up and then use the pull request button. As you can see from the web-view of your bug report, Diffs and "GitHub Flavored Markdown" don't get along very well. (Thankfully, I had e-mail notification turned on and the non-HTML e-mail version seems to be un-mangled. I don't want people counting on that though.)

...using the pull request button also lets git give you proper credit for the commit if I accept it.

ssokolow commented 14 years ago

Just a heads up. I haven't been making as much progress as I'd hoped on my coursework, so it's possible your patch will sit here until the beginning of November when my vacation starts.

ssokolow commented 14 years ago

Sorry for the huge wait. I had issues with obsessive-compulsively reading fanfiction and had to spend some time writing a tool to help deal with that.

Anyway, I got around to looking at your code and I finally recognized what was bothering me about it:

First, there's no tolerance value provision. That means that, if you trigger QuickTile for a window that isn't already tiled, it'll snap to whichever shape just happens to be most similar. That limits the user's ability to develop muscle-memory associations for various window shapes but, for all I know, you may consider it a desirable feature so I don't want to just fix it.

Second, is there a purpose to your squaring the euclidean distances beyond just ensuring a positive value? (In other words, did you just not think to use abs() or does squaring them change the behaviour in certain cases?) I'm more a UI designer than a mathematician, but I can't think of a reason not to use abs() in this situation.

ssokolow commented 13 years ago

It's been a very busy year, so I didn't have time to do all the testing I'd have liked, but I haven't seen or heard of any problems from this patch, so I'll close this bug and consider the potential issue mentioned in the commit message separately.

SHA: 5fad40b4675e169b41e4616781154930635d6295