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 617827 - Mis-positioning of a single small window
Mis-positioning of a single small window
Status: RESOLVED OBSOLETE
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-05-05 22:03 UTC by Owen Taylor
Modified: 2013-08-27 00:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[overview] Fix positioning of single small windows (1.41 KB, patch)
2010-05-09 09:29 UTC, drago01
none Details | Review
[overview] Fix positioning of single small windows (1.36 KB, patch)
2010-05-09 11:58 UTC, drago01
committed Details | Review
[overview] Pixel align window previous (3.53 KB, patch)
2010-05-11 12:55 UTC, drago01
none Details | Review
Pixel align window previews (3.52 KB, patch)
2010-05-11 12:57 UTC, drago01
reviewed Details | Review

Description Owen Taylor 2010-05-05 22:03:20 UTC
If you have a single window on a workspace, small enough that it doesn't need to be scaled down, it's supposed to be:

 Centered
 Pixel aligned

But instead it is:

 Towards the bottom
 Fuzzy

This is a regression that occurred maybe a month or two ago. I've been keeping on meaning to look at this, but never finding the time, so maybe if I file it here someone else will track it down for me. :-)
Comment 1 William Jon McCann 2010-05-06 13:29:08 UTC
If it helps, I think I first noticed this around the time we started aligning multiple windows by the bottoms.
Comment 2 drago01 2010-05-09 09:29:47 UTC
Created attachment 160629 [details] [review]
[overview] Fix positioning of single small windows

A single window that does not need to be scaled down, should be centered
and not placed at the bottom.

To avoid blurryness window positions should be pixel aligned.
Comment 3 drago01 2010-05-09 11:58:48 UTC
Created attachment 160635 [details] [review]
[overview] Fix positioning of single small windows

Fix a small bug and don't calculate y twice.
Comment 4 Florian Müllner 2010-05-09 12:15:28 UTC
Review of attachment 160635 [details] [review]:

Looks good.
Comment 5 drago01 2010-05-09 12:19:21 UTC
Attachment 160635 [details] pushed as 0e9c47b - [overview] Fix positioning of single small windows
Comment 6 Owen Taylor 2010-05-10 19:24:03 UTC
The centering is better now, but the pixel alignment isn't. Using floor() doesn't give pixel alignment if:

 A) The parent isn't pixel aligned (it isn't for me)
 B) The parent has a scale on it

Basic technique (we may have other examples - we did in the past) is to do something like:

 target_x = (round(target_parent_x + target_parent_scale * target_x) - target_parent_x) / target_parent_scale;

That is, convert to screen coordinates, round, then convert back.

Where by "target_" I meant "after animation" which might be different from parent.x or parent.scale_x
Comment 7 drago01 2010-05-11 12:55:39 UTC
Created attachment 160815 [details] [review]
[overview] Pixel align window previous

Unscaled windows should not be fuzzy in the overview, a previous
attempt to fix this only integer aligned the clones inside workspace which
wasn't enough to fix the issue.

Fix it correctly by aligning the absolute rather than the relative position
coords.
Comment 8 drago01 2010-05-11 12:57:11 UTC
Created attachment 160816 [details] [review]
Pixel align window previews

Unscaled windows should not be fuzzy in the overview, a previous
attempt to fix this only integer aligned the clones inside workspace which
wasn't enough to fix the issue.

Fix it correctly by aligning the absolute rather than the relative position
coords.

(Fixed subject)
Comment 9 Colin Walters 2010-05-19 20:46:10 UTC
Review of attachment 160816 [details] [review]:

::: js/ui/workspace.js
@@ +1131,3 @@
+                                    */
+                                    * of the parent.
+                                   /* We add a slight delay to be able to get the final positions

Rather than adding 0.05, can we do a Meta.later_add(Meta.LaterType.BEFORE_REDRAW) in the onComplete?
Comment 10 drago01 2010-05-19 21:35:00 UTC
(In reply to comment #9)
> Review of attachment 160816 [details] [review]:
> 
> ::: js/ui/workspace.js
> @@ +1131,3 @@
> +                                    */
> +                                    * of the parent.
> +                                   /* We add a slight delay to be able to get
> the final positions
> 
> Rather than adding 0.05, can we do a
> Meta.later_add(Meta.LaterType.BEFORE_REDRAW) in the onComplete?

No, tried it and it does not work (well we do want to call it _after_ the animation of the workspace so BEFORE_REDRAW does not really make sense).
Comment 11 Colin Walters 2010-05-19 21:52:33 UTC
(In reply to comment #10)
>
> No, tried it and it does not work (well we do want to call it _after_ the
> animation of the workspace so BEFORE_REDRAW does not really make sense).

Isn't onComplete() called after the animation is done?  Oh...I see, our tweener.js uses g_idle_add for onComplete, so a Meta.later_add() queued from there might potentially happen before the idle for a different onComplete() fires.

I'm not seeing an immediate elegant solution, so feel free to commit.
Comment 12 Owen Taylor 2010-05-19 22:49:03 UTC
My basic opinion is that this patch misses what I was getting at above - that instead tweening to unaligned coordinates and then fixing up after we stop tweening, we want to pixel align the coordinates that we tween to.

This does mean that we  have to know how things are positioned in the end - we can't simply rely the *current* values of .x/.y/.scale for actors.
Comment 13 Dan Winship 2010-05-19 23:57:34 UTC
(In reply to comment #11)
> Isn't onComplete() called after the animation is done?  Oh...I see, our
> tweener.js uses g_idle_add for onComplete, so a Meta.later_add() queued from
> there might potentially happen before the idle for a different onComplete()
> fires.

The idle is for delegate.onAnimationComplete (which we're apparently not even using for anything any more), not for the onComplete specified in the tweening params.

The problem (which we've run into before) is that the end of the animation ends up looking like this:

    move clone to almost-final scale and position

    move workspace to almost-final scale and position

    move clone to final scale and position
      run clone's onComplete

    move workspace to final scale and position position
      run workspace's onComplete

so the clone ends up pixel-aligned to the workspace's almost-final position instead of its final position.

We'd talked before about making tweener smarter about ordering...
Comment 14 Allan Day 2013-08-27 00:20:40 UTC
Old bug. We always scale down thumbnails in the overview now, so maybe it has been fixed.

Closing as obsolete. Please reopen if you think it's relevant still.