Code review comment for lp:~alan-griffiths/miral/initial-gdk_window_move_to_rect-inspired-placement-logic

> The obvious question is if MirAL needs to gain an API exactly suiting GDK, or
> if there is a simpler API that MirAL could offer that would offer GDK the same
> behaviour.
>
> I had originally thought MirEdgeAttachment was sufficient, but GDK's API did
> make me reconsider.
>
> Certainly GDK has the most comprehensive API for window positioning as it will
> be a good customer: Qt and SDL currently rely on knowledge of absolute window
> coordinates and screen geometry to calculate window position. But similarly,
> it means this API is of no use to Qt or SDL.
>
> I decided to compare with Wayland, to see what their proposed XDG-shell
> protocol supports:
> https://github.com/wayland-project/wayland-protocols/blob/master/unstable/xdg-
> shell/xdg-shell-unstable-v6.xml#L158
> and AFAICT, it provides all that GDK is using perfectly.
>
> As a result, I think this is a good direction to go. Keeping as close as we
> can to the XDG-shell spec is a good thing IMO.

That was my thinking too.

> If Mir's MirEdgeAttachment is not sufficient, should it be removed from Mir
> entirely?

1. I'm not sure that relates to this MP.
2. Removal would be a mirclient API/ABI break, so I don't advocate that, instead I'd look at deprecation pending such a break.

> === modified file 'include/miral/window_specification.h'
> + mir_placement_gravity_northwest = 1, // the reference point is at the top
> left corner.
> Top left corner of what? The anchor rect (aux_rect)?

The thing the gravity is applied to - either the window or the aux_rect.

> + * These hints determine how the window should be positioned in the case that
> + * the window would fall off-screen if placed in its ideal position.
> If client does not set these hints, then it is possible for the window to be
> placed off-screen. Do we ever want to allow that possibility? XDG-shell does,
> but I am not sure what use-case that satisfies.

We can decide that behaviour in a follow-up MP - there are no tests covering that case. Personally, I think a badly formed placement request should simply be rejected before the window management policy sees it.

The client API can be seen in lp:~alan-griffiths/miral/connect-gdk_window_move_to_rect-inspired-placement-logic-to-Mir-0.25-API/+register-merge

> + if (hints & mir_placement_hints_antipodes)
> + rect_gravities.push_back(antipodes(parameters.aux_rect_placement_gravity().v
> alue()));
> indent wrong
>
> Overall no major objection, just niggles!

Ack. Can I fix that in the final follow-up rather that rippling a single character change through the entire pre-req chain?