After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 607826 - Window previews change positions when the number of workspaces changes
Window previews change positions when the number of workspaces changes
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-01-22 23:59 UTC by Marina Zhurakhinskaya
Modified: 2010-02-22 17:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Window preview positions before another workspace is added (610.55 KB, image/png)
2010-01-23 00:01 UTC, Marina Zhurakhinskaya
  Details
Window preview positions after another workspace is added (383.01 KB, image/png)
2010-01-23 00:01 UTC, Marina Zhurakhinskaya
  Details
positionWindows use old order of previews, if it is possible. (2.70 KB, patch)
2010-01-23 21:19 UTC, Maxim Ermilov
none Details | Review
_computeWindowMotion calculate difference between relative positions (4.85 KB, patch)
2010-02-22 06:20 UTC, Maxim Ermilov
committed Details | Review

Description Marina Zhurakhinskaya 2010-01-22 23:59:53 UTC
Window previews on a given workspace change positions when an additional workspace is added or removed.

In the case of the attached screenshots, the window on the top left and the window on the bottom right change places diagonally. It might have something to do with the non-typical dimensions of the Empathy Buddy List.
Comment 1 Marina Zhurakhinskaya 2010-01-23 00:01:01 UTC
Created attachment 152057 [details]
Window preview positions before another workspace is added
Comment 2 Marina Zhurakhinskaya 2010-01-23 00:01:56 UTC
Created attachment 152058 [details]
Window preview positions after another workspace is added
Comment 3 Maxim Ermilov 2010-01-23 21:19:47 UTC
Created attachment 152111 [details] [review]
positionWindows use old order of previews, if it is possible.

_orderWindowsByMotionAndStartup can return different sequences(for same arguments)
if we have same windows, we can use old windows order.
Comment 4 Dan Winship 2010-01-25 16:34:48 UTC
without looking too closely, this patch seems like it's doing a lot of work...

why does _orderWindowsByMotionAndStartup return different sequences?
Comment 5 Maxim Ermilov 2010-01-25 21:39:04 UTC
> why does _orderWindowsByMotionAndStartup return different sequences?

_computeWindowSlot return absolute position(will be different on different width).
So _computeWindowMotion will return different result on different sizes.

> without looking too closely, this patch seems like it's doing a lot of work...

_orderWindowsGreedy has complexity O(n^2)
_orderWindowsPermutations has complexity O(n!)

Check from this patch has O(n*ln(n)). This function will be call on widndow-added/removed signal, so will be used this check (when windows.length doesnot change) OR _orderWindowsByMotionAndStartup.
Comment 6 Maxim Ermilov 2010-01-25 21:55:53 UTC
(In reply to comment #5)
> _computeWindowSlot return absolute position(will be different on different
> width).
> So _computeWindowMotion will return different result on different sizes.

_computeWindowSlot return relative position, but in _computeWindowMotion use absolute position.
Comment 7 Maxim Ermilov 2010-02-22 06:20:39 UTC
Created attachment 154365 [details] [review]
_computeWindowMotion calculate difference between relative positions
Comment 8 Dan Winship 2010-02-22 16:26:18 UTC
Comment on attachment 154365 [details] [review]
_computeWindowMotion calculate difference between relative positions

ah, this looks much better

>-        distanceSquared = xDelta * xDelta + yDelta * yDelta;
>+        xDelta = Math.abs((rect.x + rect.width / 2.0) / global.screen_width - xCenter);
>+        yDelta = Math.abs((rect.y + rect.height / 2.0) / global.screen_height - yCenter);
> 
>-        return distanceSquared;
>+        return xDelta + yDelta;

since the units are arbitrary, you can make this a little faster by multiplying the second term rather than dividing the first. Also, I think we want to continue squaring the result, as explained in bug 582654 comment 9. So:

    xDelta = rect.x + rect.width / 2.0 - xCenter * global.screen_width
    yDelta = rect.y + rect.height / 2.0 - yCenter * global.screen_height
    distanceSquared = xDelta * xDelta + yDelta * yDelta;
    return distanceSquared;

(and undo the change to the comment above the function)

and then it's good to commit