Created attachment 189249 [details] [review]
adds meta_window_move_resize_frame to mutter

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.

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.

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?

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()

> 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.

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?

(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.

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?

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;

Created attachment 200743 [details] [review]
adds meta_window_move_resize_frame to mutter (#3)

Updated as suggested, thanks.

Created attachment 200744 [details] [review]
adds meta_window_move_resize_frame to mutter (#4)

minor edit: removed unused `frame` variable

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.

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.

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.

I'm certainly fine with it.