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 651899 - allow shell extensions to move / resize a window's *outer* rect
allow shell extensions to move / resize a window's *outer* rect
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-06-05 00:59 UTC by Tim Cuthbertson
Modified: 2011-11-05 22:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adds meta_window_move_resize_frame to mutter (2.56 KB, patch)
2011-06-05 01:10 UTC, Tim Cuthbertson
none Details | Review
adds meta_window_move_resize_frame to mutter (#2) (2.82 KB, patch)
2011-11-05 03:21 UTC, Tim Cuthbertson
needs-work Details | Review
adds meta_window_move_resize_frame to mutter (#3) (2.76 KB, patch)
2011-11-05 09:50 UTC, Tim Cuthbertson
none Details | Review
adds meta_window_move_resize_frame to mutter (#4) (2.72 KB, patch)
2011-11-05 10:15 UTC, Tim Cuthbertson
committed Details | Review

Description Tim Cuthbertson 2011-06-05 00:59:58 UTC
For tiling window extensions (and others), there is a need to fit a window's entire contents into a given rectangle. meta_window_move_resize does not fulfil this need, as it doesn't take the frame decorations into account.

I notice there is a meta_window_move_frame in mutter, this patch adds a meta_window_move_resize_frame that does the same thing (i.e moves / resizes the window such that the entire frame fits into the given dimensions).

There is a similar method meta_frames_move_resize_frame in the mutter source, but I could not figure out how to expose that in .gir bindings, nor now to get my hands on a MetaFrames object from within gnome-shell.
Comment 1 Tim Cuthbertson 2011-06-05 01:10:48 UTC
Created attachment 189249 [details] [review]
adds meta_window_move_resize_frame to mutter
Comment 2 Tim Cuthbertson 2011-06-05 01:13:18 UTC
erm, when I say this function "does the same thing" as meta_window_move_frame, I mean it takes into account the frame, but obviously (from the name) it does a move *and* resize, instead of just a move.
Comment 3 Tomas Frydrych 2011-06-06 15:37:41 UTC
IMO, if we want to allow extensions to override the standard constrains policy, then the constraints mechanism (src/core/constraints.[hc]) should be made extensible. Moblin did this by adding a constraints() vfunction to the plugin, but with a hindsight that is not a particularly good solution, you simply want extensions to be able to register a custom constraints function with a given priority level.
Comment 4 Tim Cuthbertson 2011-06-07 02:28:16 UTC
I'm not too familiar with the constraints code. Will it be flexible enough to allow arbitrary placement in response to user actions? It sounds like it would only take effect upon window events (like moving, mapping / unmapping etc)

e.g say I want to have a keybinding that will swap the position and size of two specific windows, and a keybinding that will increase the width of all windows on the left-hand side of the screen by 100 pixels. Will that be possible using constraints?
Comment 5 Owen Taylor 2011-07-08 18:58:47 UTC
I could see that extending constraints code could be good for some things. But it's not meant for placement, or for a "tile all windows" action or something like that. So, it doesn't seem appropriate here.

But is there a reason to add this instead of just exposing the decoration dimensions and letting the caller roll their own? (Note that there's a big patch outstanding from Jasper to rework frame sizing and add MetaFrameBorders, so exposing the decoration dimensions would probably have to wait for that to land.)

If we do add these as convenience function, the set of exposed functions can't be:

 move()
 resize()
 move_frame()
 move_resize_frame()

I think we'd want to expose the entire orthogonal set of:

 move()
 resize()
 move_resize() 
 move_frame()
 resize_frame()
 move_resize_frame()
Comment 6 Tim Cuthbertson 2011-07-09 06:51:40 UTC
> But is there a reason to add this instead of just exposing the decoration
dimensions and letting the caller roll their own?

Mostly for convenience, but also for correctness - if your intent is to resize the entire window (including decorations), rolling your own leads to a lot of duplication and potentially forgetting edge cases if there are intricacies in the calculations. And when the logic / interface changes (as it sounds like it may when the mentioned frame sizing rework lnads), you'd have to fix all the home-rolled instances of the exact same code. If you expose the intention to move the entire window, then mutter can maintain that action correctly regardless of how the decoration sizing changes over time.

As for the entire orthogonal set, yes that makes sense. I only exposed the minimal set that I needed, but am happy to add the others for consistency if it will be accepted.
Comment 7 Tim Cuthbertson 2011-11-04 12:04:23 UTC
Can I ask where things lie in relation to this and mutter 3.2? Has jasper's patch to rework frame sizing landed, and is any of the frame sizing been exposed to gnome-shell?
Comment 8 Jasper St. Pierre (not reading bugmail) 2011-11-04 13:51:00 UTC
(In reply to comment #7)
> Can I ask where things lie in relation to this and mutter 3.2? Has jasper's
> patch to rework frame sizing landed, and is any of the frame sizing been
> exposed to gnome-shell?

If you mean the invisible borders work, yes, so now your patches are incorrect. You need to call meta_frame_calc_borders, like I did in bug #659643. Owen is traveling this week, so he should be able to take mutter patches next week.
Comment 9 Tim Cuthbertson 2011-11-05 03:21:44 UTC
Created attachment 200729 [details] [review]
adds meta_window_move_resize_frame to mutter (#2)

Thanks Jasper. This is an updated patch that includes window borders in the calculation.

I think the fact that it now involves 3 measurements (window contents + frame + borders) is justification for implementing it once rather than just exposing the individual properties and letting clients do it themselves, but if that's still not an option then can you let me know what needs to be done to expose those properties (window + frame + border dimensions) to gnome-shell?
Comment 10 Jasper St. Pierre (not reading bugmail) 2011-11-05 03:33:13 UTC
Review of attachment 200729 [details] [review]:

::: src/core/window.c
@@ +4998,3 @@
+ * @root_y_nw: new y
+ * @w: desired width
+ * @h: desired width

desired height.

@@ +5005,3 @@
+void
+meta_window_move_resize_frame (MetaWindow  *window,
+                  gboolean     user_op,

This needs to be indented correctly.

@@ +5015,3 @@
+      MetaFrameBorders borders;
+      meta_frame_calc_borders (window->frame, &borders);
+      MetaFrame* frame = window->frame;

ANSI C. All variable declarations need to be at the top of a block.

@@ +5019,3 @@
+       * and the origin of the enclosing window decorations ( + border)
+       */
+      root_x_nw += frame->child_x - borders.invisible.left;

frame->child_x = borders.total.left, so this should just be:

   root_x_nw += borders.visible.left;
   root_y_nw += borders.visible.top;

@@ +5021,3 @@
+      root_x_nw += frame->child_x - borders.invisible.left;
+      root_y_nw += frame->child_y - borders.invisible.top;
+      w -= frame->right_width + frame->child_x  - (borders.invisible.left + borders.invisible.right);

Similarly, right_width = borders.total.right, so we can make this:

    w -= borders.visible.left + borders.visible.right;
    h -= borders.visible.top + borders.visible.bottom;
Comment 11 Tim Cuthbertson 2011-11-05 09:50:43 UTC
Created attachment 200743 [details] [review]
adds meta_window_move_resize_frame to mutter (#3)

Updated as suggested, thanks.
Comment 12 Tim Cuthbertson 2011-11-05 10:15:57 UTC
Created attachment 200744 [details] [review]
adds meta_window_move_resize_frame to mutter (#4)

minor edit: removed unused `frame` variable
Comment 13 Jasper St. Pierre (not reading bugmail) 2011-11-05 14:25:18 UTC
Review of attachment 200744 [details] [review]:

Assuming you've tested this, looks fine to me. If you need someone to push this for you, just let me know.
Comment 14 Jasper St. Pierre (not reading bugmail) 2011-11-05 15:56:21 UTC
Note that if you want something right now, something like:

    function moveAndResizeFrame(metaWindow, user_op, x, y, w, h) {
            let clientRect = metaWindow.get_rect();
            let outerRect = metaWindow.get_outer_rect();
             
            metaWindow.resize(user_op,
                              w + outerRect.width - clientRect.width,
                              h + outerRect.height - clientRect.height);
            metaWindow.move_frame(user_op, x, y);
    }

should work.
Comment 15 Tim Cuthbertson 2011-11-05 22:46:05 UTC
I'd certainly like it to be pushed (yup, I have tested it). Owen's last comment showed some reservations, but that was some time ago. If the full orthogonal set is desired, we can always add that later as necessary.

And thanks for the client code, I didn't know that was possible. I still think it should go into mutter, but that at least makes it possible (with some work) outside.
Comment 16 Jasper St. Pierre (not reading bugmail) 2011-11-05 22:53:40 UTC
I'm certainly fine with it.