Mir

Merge lp:~alan-griffiths/mir/support-gdk_window_move_to_rect into lp:mir

support-gdk_window_move_to_rect
Merge into development-branch

Proposed by Alan Griffiths on 2016-08-31

Status:

Merged

Merged at revision:

3685

Proposed branch:

lp:~alan-griffiths/mir/support-gdk_window_move_to_rect

Merge into:

lp:mir

Diff against target:

588 lines (+366/-12)

13 files modified

include/client/mir_toolkit/mir_surface.h (+33/-1)
include/client/mir_toolkit/version.h (+2/-2)
include/common/mir_toolkit/common.h (+94/-1)
include/server/mir/scene/surface_creation_parameters.h (+6/-1)
include/server/mir/shell/surface_specification.h (+6/-1)
src/client/mir_connection.cpp (+6/-1)
src/client/mir_surface.cpp (+6/-1)
src/client/mir_surface.h (+6/-1)
src/client/mir_surface_api.cpp (+24/-1)
src/client/symbols.map (+1/-0)
src/protobuf/mir_protobuf.proto (+13/-0)
src/server/frontend/session_mediator.cpp (+11/-1)
tests/acceptance-tests/test_custom_window_management.cpp (+158/-1)

To merge this branch:

bzr merge lp:~alan-griffiths/mir/support-gdk_window_move_to_rect

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Needs Fixing on 2016-09-06
Chris Halse Rogers			Approve on 2016-09-05
Kevin DuBois (community)			Approve on 2016-09-01
Gerry Boland	vote	2016-08-31	Approve on 2016-09-01
Review via email: mp+304474@code.launchpad.net

Commit Message

Add the API and IPC required to pass the surface placement requests needed to support gdk_window_move_to_rect() to the window manager. It does not include the window manager logic to service the request. (There's an associated window management implementation proposed for MirAL: lp:~alan-griffiths/miral/connect-gdk_window_move_to_rect-inspired-placement-logic-to-Mir-0.25-API/+merge/304481)

Mir CI Bot (mir-ci-bot) wrote on 2016-08-31:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1586/rebuild

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2016-09-01:

190 + optional_value<MirPlacementHints> placement_hints;
191 + optional_value<MirPlacementGravity> surface_placement_gravity;
192 + optional_value<MirPlacementGravity> aux_rect_placement_gravity;
193 + optional_value<int> aux_rect_placement_offset_x;
194 + optional_value<int> aux_rect_placement_offset_y;

It seems like these values only ever make sense to be set together? Should this be rolled into a single
optional_value<SurfaceAttachmentParameters>?

Otherwise looks sensible to me. There's presumably a follow-up message where Mir tells the client where the surface was actually placed?

Alan Griffiths (alan-griffiths) wrote on 2016-09-01:

> 190 + optional_value<MirPlacementHints> placement_hints;
> 191 + optional_value<MirPlacementGravity> surface_placement_gravity;
> 192 + optional_value<MirPlacementGravity> aux_rect_placement_gravity;
> 193 + optional_value<int> aux_rect_placement_offset_x;
> 194 + optional_value<int> aux_rect_placement_offset_y;
>
> It seems like these values only ever make sense to be set together? Should
> this be rolled into a single
> optional_value<SurfaceAttachmentParameters>?

I did think about that, but there's an overlap with...

optional_value<geometry::Rectangle> aux_rect;
optional_value<MirEdgeAttachment> edge_attachment;

...which once might also have been said to only make sense together. In the end I decided
that this imposed fewer restrictions on future evolution of the code. It only affects a small part of the system.

> Otherwise looks sensible to me. There's presumably a follow-up message where
> Mir tells the client where the surface was actually placed?

I've intentionally left that out of scope for this MP as we don't currently notify the result of edge attachments and our policy on what placement information to disclose isn't clear to me.

Gerry Boland (gerboland) wrote on 2016-09-01:

Looks ok to me. API comprehensive enough for GTK's needs.

review: Approve (vote)

Kevin DuBois (kdub) wrote on 2016-09-01:

I guess the one-way requesting without confirmation or denial of the requests still is something that confuses me a bit in general about the placement/surface_spec api. Notwithstanding that, don't mind a request to place a surface in a certain way.

curiousity (since your testing opinions are typically more well-formed than mine)

+ EXPECT_TRUE(spec.placement_hints.is_set());
+ if (spec.placement_hints.is_set())
+ EXPECT_THAT(spec.placement_hints.value(), Eq(placement_hints));

why not:
ASSERT_TRUE(spec.placement_hints.is_set());
EXPECT_THAT(spec.placement_hints.value(), Eq(placement_hints));

nits/needs fixing:
extra indentation:
+ mir_placement_hints_flip_any = mir_placement_hints_flip_x|mir_placement_hints_flip_y,
+ mir_placement_hints_slide_any = mir_placement_hints_slide_x|mir_placement_hints_slide_y,
+ mir_placement_hints_resize_any = mir_placement_hints_resize_x|mir_placement_hints_resize_y,

+ optional_value<int> aux_rect_placement_offset_x;
+ optional_value<int> aux_rect_placement_offset_y;
why not optional_value<geom::Displacement>?

review: Needs Fixing

Alan Griffiths (alan-griffiths) wrote on 2016-09-01:

> I guess the one-way requesting without confirmation or denial of the requests
> still is something that confuses me a bit in general about the
> placement/surface_spec api. Notwithstanding that, don't mind a request to
> place a surface in a certain way.
>
> curiousity (since your testing opinions are typically more well-formed than
> mine)
>
> + EXPECT_TRUE(spec.placement_hints.is_set());
> + if (spec.placement_hints.is_set())
> + EXPECT_THAT(spec.placement_hints.value(), Eq(placement_hints));
>
> why not:
> ASSERT_TRUE(spec.placement_hints.is_set());
> EXPECT_THAT(spec.placement_hints.value(), Eq(placement_hints));

Because this callback is happening on the wrong thread, we want a report on any incorrect expectations and to ensure we raise the signal (not wait for a timeout).

> nits/needs fixing:
> extra indentation:
> + mir_placement_hints_flip_any =
> mir_placement_hints_flip_x|mir_placement_hints_flip_y,
> + mir_placement_hints_slide_any =
> mir_placement_hints_slide_x|mir_placement_hints_slide_y,
> + mir_placement_hints_resize_any =
> mir_placement_hints_resize_x|mir_placement_hints_resize_y,

Fixed.

> + optional_value<int> aux_rect_placement_offset_x;
> + optional_value<int> aux_rect_placement_offset_y;
> why not optional_value<geom::Displacement>?

We have more regular code if we match the protobuf structures:

+ COPY_IF_SET(aux_rect_placement_offset_x);
+ COPY_IF_SET(aux_rect_placement_offset_y);
...
+ COPY_IF_SET(aux_rect_placement_offset_x);
+ COPY_IF_SET(aux_rect_placement_offset_y);

The MirAL interface (which is what I want downstreams to use) does wrap this:

+ if (params.aux_rect_placement_offset_x.is_set() && params.aux_rect_placement_offset_y.is_set())
+ aux_rect_placement_offset = Displacement{params.aux_rect_placement_offset_x.value(), params.aux_rect_placement_offset_y.value()};

Alan Griffiths (alan-griffiths) wrote on 2016-09-01:

> Because this callback is happening on the wrong thread, we want a report on
> any incorrect expectations

I.e. Not just the first.

Kevin DuBois (kdub) wrote on 2016-09-01:

alright, lgtm then

review: Approve

William Hua (attente) wrote on 2016-09-01:

Looks good, it isn't clear to me how the positioning feedback will work exactly. Will it be via a configure event? Or is that postponed to another branch?

Alan Griffiths (alan-griffiths) wrote on 2016-09-01:

> Looks good, it isn't clear to me how the positioning feedback will work
> exactly. Will it be via a configure event? Or is that postponed to another
> branch?

I left position feedback out of this MP because it will cause discussion. I need to do some investigation to determine the minimum information needed, and if there's prior art to follow in Weston.

Mir CI Bot (mir-ci-bot) wrote on 2016-09-01:

FAILED: Continuous integration, rev:3686
https://mir-jenkins.ubuntu.com/job/mir-ci/1607/
Executed test runs:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1607/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-09-02:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1618/rebuild

review: Needs Fixing (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2016-09-02:

10:09:23 11: [ FAILED ] NestedServer.animated_cursor_image_changes_are_forwarded_to_host (6334 ms)

Known flaky test: lp:1523621

Mir CI Bot (mir-ci-bot) wrote on 2016-09-05:

review: Needs Fixing (continuous-integration)

Chris Halse Rogers (raof) wrote on 2016-09-05:

I'm good with this.

review: Approve

Mir CI Bot (mir-ci-bot) wrote on 2016-09-06:

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/client/mir_toolkit/mir_surface.h'
 --- include/client/mir_toolkit/mir_surface.h	2016-07-18 10:21:04 +0000
 +++ include/client/mir_toolkit/mir_surface.h	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012-2014 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -567,6 +567,38 @@
  void mir_surface_spec_set_pointer_confinement(MirSurfaceSpec* spec, MirPointerConfinementState state);
  /**
++ * Set the surface placement on the spec.
++ *
++ * \param [in] spec             the spec to update
++ * \param [in] rect             the destination rectangle to align with
++ * \param [in] rect_gravity     the point on \p rect to align with
++ * \param [in] surface_gravity  the point on the surface to align with
++ * \param [in] placement_hints  positioning hints to use when limited on space
++ * \param [in] offset_dx        horizontal offset to shift w.r.t. \p rect
++ * \param [in] offset_dy        vertical offset to shift w.r.t. \p rect
++ *
++ * Moves a surface to \p rect, aligning their reference points.
++ *
++ * \p rect is relative to the top-left corner of the parent surface.
++ * \p rect_gravity and \p surface_gravity determine the points on \p rect and
++ * the surface to pin together. \p rect's alignment point can be offset by
++ * \p offset_dx and \p offset_dy, which is equivalent to offsetting the
++ * position of the surface.
++ *
++ * \p placement_hints determine how the window should be positioned in the case
++ * that the surface would fall off-screen if placed in its ideal position.
++ * See \ref MirPlacementHints for details.
++ */
++void mir_surface_spec_set_placement(
++    MirSurfaceSpec*     spec,
++    const MirRectangle* rect,
++    MirPlacementGravity rect_gravity,
++    MirPlacementGravity surface_gravity,
++    MirPlacementHints   placement_hints,
++    int                 offset_dx,
++    int                 offset_dy);
++
++/**
   * Set the event handler to be called when events arrive for a surface.
   *   \warning event_handler could be called from another thread. You must do
   *            any locking appropriate to protect your data accessed in the
 === modified file 'include/client/mir_toolkit/version.h'
 --- include/client/mir_toolkit/version.h	2016-05-03 06:55:25 +0000
 +++ include/client/mir_toolkit/version.h	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2014 Canonical Ltd.
++ * Copyright © 2014-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -43,7 +43,7 @@
+  *
   * See also: http://semver.org/
   */
--#define MIR_CLIENT_MINOR_VERSION (3)
++#define MIR_CLIENT_MINOR_VERSION (4)
  /**
   * MIR_CLIENT_MICRO_VERSION
 === modified file 'include/common/mir_toolkit/common.h'
 --- include/common/mir_toolkit/common.h	2016-08-09 06:44:52 +0000
 +++ include/common/mir_toolkit/common.h	2016-09-02 09:12:06 +0000
@@ -1,7 +1,7 @@
  /*
   * Simple definitions common to client and server.
+  *
-- * Copyright © 2013-2014 Canonical Ltd.
++ * Copyright © 2013-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify
   * it under the terms of the GNU Lesser General Public License version 3 as
@@ -199,6 +199,99 @@
                                mir_edge_attachment_horizontal
  } MirEdgeAttachment;
++// Inspired by GdkGravity
++/**
++ * Reference point for aligning a surface relative to a rectangle.
++ * Each element (surface and rectangle) has a MirPlacementGravity assigned.
++ */
++typedef enum MirPlacementGravity
++{
++    /// the reference point is at the center.
++    mir_placement_gravity_center    = 0,
++
++    /// the reference point is at the middle of the left edge.
++    mir_placement_gravity_west      = 1 << 0,
++
++    /// the reference point is at the middle of the right edge.
++    mir_placement_gravity_east      = 1 << 1,
++
++    /// the reference point is in the middle of the top edge.
++    mir_placement_gravity_north     = 1 << 2,
++
++    /// the reference point is at the middle of the lower edge.
++    mir_placement_gravity_south     = 1 << 3,
++
++    /// the reference point is at the top left corner.
++    mir_placement_gravity_northwest = mir_placement_gravity_north | mir_placement_gravity_west,
++
++    /// the reference point is at the top right corner.
++    mir_placement_gravity_northeast = mir_placement_gravity_north | mir_placement_gravity_east,
++
++    /// the reference point is at the lower left corner.
++    mir_placement_gravity_southwest = mir_placement_gravity_south | mir_placement_gravity_west,
++
++    /// the reference point is at the lower right corner.
++    mir_placement_gravity_southeast = mir_placement_gravity_south | mir_placement_gravity_east
++} MirPlacementGravity;
++
++// Inspired by GdkAnchorHints
++/**
++ * Positioning hints for aligning a window relative to a rectangle.
++ *
++ * These hints determine how the window should be positioned in the case that
++ * the surface would fall off-screen if placed in its ideal position.
++ *
++ * For example, \p mir_placement_hints_flip_x will invert the x component of
++ * \p aux_rect_placement_offset and replace \p mir_placement_gravity_northwest
++ * with \p mir_placement_gravity_northeast and vice versa if the window extends
++ * beyond the left or right edges of the monitor.
++ *
++ * If \p mir_placement_hints_slide_x is set, the window can be shifted
++ * horizontally to fit on-screen.
++ *
++ * If \p mir_placement_hints_resize_x is set, the window can be shrunken
++ * horizontally to fit.
++ *
++ * If \p mir_placement_hints_antipodes is set then the rect gravity may be
++ * substituted with the opposite corner (e.g. \p mir_placement_gravity_northeast
++ * to \p mir_placement_gravity_southwest) in combination with other options.
++ *
++ * When multiple flags are set, flipping should take precedence over sliding,
++ * which should take precedence over resizing.
++ */
++typedef enum MirPlacementHints
++{
++    /// allow flipping anchors horizontally
++    mir_placement_hints_flip_x   = 1 << 0,
++
++    /// allow flipping anchors vertically
++    mir_placement_hints_flip_y   = 1 << 1,
++
++    /// allow sliding window horizontally
++    mir_placement_hints_slide_x  = 1 << 2,
++
++    /// allow sliding window vertically
++    mir_placement_hints_slide_y  = 1 << 3,
++
++    /// allow resizing window horizontally
++    mir_placement_hints_resize_x = 1 << 4,
++
++    /// allow resizing window vertically
++    mir_placement_hints_resize_y = 1 << 5,
++
++    /// allow flipping aux_anchor to opposite corner
++    mir_placement_hints_antipodes= 1 << 6,
++
++    /// allow flipping anchors on both axes
++    mir_placement_hints_flip_any = mir_placement_hints_flip_x|mir_placement_hints_flip_y,
++
++    /// allow sliding window on both axes
++    mir_placement_hints_slide_any  = mir_placement_hints_slide_x|mir_placement_hints_slide_y,
++
++    /// allow resizing window on both axes
++    mir_placement_hints_resize_any = mir_placement_hints_resize_x|mir_placement_hints_resize_y,
++} MirPlacementHints;
++
  /**
   * Form factor associated with a physical output
   */
 === modified file 'include/server/mir/scene/surface_creation_parameters.h'
 --- include/server/mir/scene/surface_creation_parameters.h	2016-06-21 21:31:05 +0000
 +++ include/server/mir/scene/surface_creation_parameters.h	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2013 Canonical Ltd.
++ * Copyright © 2013-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 3,
@@ -87,6 +87,11 @@
      mir::optional_value<frontend::BufferStreamId> content_id;
      mir::optional_value<geometry::Rectangle> aux_rect;
      mir::optional_value<MirEdgeAttachment> edge_attachment;
++    optional_value<MirPlacementHints> placement_hints;
++    optional_value<MirPlacementGravity> surface_placement_gravity;
++    optional_value<MirPlacementGravity> aux_rect_placement_gravity;
++    optional_value<int> aux_rect_placement_offset_x;
++    optional_value<int> aux_rect_placement_offset_y;
      std::weak_ptr<Surface> parent;
 === modified file 'include/server/mir/shell/surface_specification.h'
 --- include/server/mir/shell/surface_specification.h	2016-06-08 13:49:11 +0000
 +++ include/server/mir/shell/surface_specification.h	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2015 Canonical Ltd.
++ * Copyright © 2015-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 3,
@@ -63,6 +63,11 @@
      optional_value<frontend::SurfaceId> parent_id;
      optional_value<geometry::Rectangle> aux_rect;
      optional_value<MirEdgeAttachment> edge_attachment;
++    optional_value<MirPlacementHints> placement_hints;
++    optional_value<MirPlacementGravity> surface_placement_gravity;
++    optional_value<MirPlacementGravity> aux_rect_placement_gravity;
++    optional_value<int> aux_rect_placement_offset_x;
++    optional_value<int> aux_rect_placement_offset_y;
      optional_value<geometry::Width> min_width;
      optional_value<geometry::Height> min_height;
      optional_value<geometry::Width> max_width;
 === modified file 'src/client/mir_connection.cpp'
 --- src/client/mir_connection.cpp	2016-08-31 09:41:42 +0000
 +++ src/client/mir_connection.cpp	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -122,6 +122,11 @@
      SERIALIZE_OPTION_IF_SET(state);
      SERIALIZE_OPTION_IF_SET(pref_orientation);
      SERIALIZE_OPTION_IF_SET(edge_attachment);
++    SERIALIZE_OPTION_IF_SET(placement_hints);
++    SERIALIZE_OPTION_IF_SET(surface_placement_gravity);
++    SERIALIZE_OPTION_IF_SET(aux_rect_placement_gravity);
++    SERIALIZE_OPTION_IF_SET(aux_rect_placement_offset_x);
++    SERIALIZE_OPTION_IF_SET(aux_rect_placement_offset_y);
      SERIALIZE_OPTION_IF_SET(min_width);
      SERIALIZE_OPTION_IF_SET(min_height);
      SERIALIZE_OPTION_IF_SET(max_width);
 === modified file 'src/client/mir_surface.cpp'
 --- src/client/mir_surface.cpp	2016-08-24 02:09:08 +0000
 +++ src/client/mir_surface.cpp	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012, 2015 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -550,6 +550,11 @@
      // parent_id is a special case (below)
      // aux_rect is a special case (below)
      COPY_IF_SET(edge_attachment);
++    COPY_IF_SET(aux_rect_placement_gravity);
++    COPY_IF_SET(surface_placement_gravity);
++    COPY_IF_SET(placement_hints);
++    COPY_IF_SET(aux_rect_placement_offset_x);
++    COPY_IF_SET(aux_rect_placement_offset_y);
      COPY_IF_SET(min_width);
      COPY_IF_SET(min_height);
      COPY_IF_SET(max_width);
 === modified file 'src/client/mir_surface.h'
 --- src/client/mir_surface.h	2016-06-09 00:38:26 +0000
 +++ src/client/mir_surface.h	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012, 2015 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -111,6 +111,11 @@
      std::shared_ptr<MirPersistentId> parent_id;
      mir::optional_value<MirRectangle> aux_rect;
      mir::optional_value<MirEdgeAttachment> edge_attachment;
++    mir::optional_value<MirPlacementHints> placement_hints;
++    mir::optional_value<MirPlacementGravity> surface_placement_gravity;
++    mir::optional_value<MirPlacementGravity> aux_rect_placement_gravity;
++    mir::optional_value<int> aux_rect_placement_offset_x;
++    mir::optional_value<int> aux_rect_placement_offset_y;
      mir::optional_value<int> min_width;
      mir::optional_value<int> min_height;
 === modified file 'src/client/mir_surface_api.cpp'
 --- src/client/mir_surface_api.cpp	2016-07-18 10:11:34 +0000
 +++ src/client/mir_surface_api.cpp	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2014 Canonical Ltd.
++ * Copyright © 2014-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU Lesser General Public License version 3,
@@ -721,6 +721,29 @@
      MIR_LOG_UNCAUGHT_EXCEPTION(ex);
+ }
++void mir_surface_spec_set_placement(
++    MirSurfaceSpec*     spec,
++    MirRectangle const* rect,
++    MirPlacementGravity rect_gravity,
++    MirPlacementGravity surface_gravity,
++    MirPlacementHints   placement_hints,
++    int                 offset_dx,
++    int                 offset_dy)
++try
++{
++    spec->aux_rect = *rect;
++    spec->aux_rect_placement_gravity = rect_gravity;
++    spec->surface_placement_gravity = surface_gravity;
++    spec->placement_hints = placement_hints;
++    spec->aux_rect_placement_offset_x = offset_dx;
++    spec->aux_rect_placement_offset_y = offset_dy;
++}
++catch (std::exception const& ex)
++{
++    MIR_LOG_UNCAUGHT_EXCEPTION(ex);
++}
++
++
  MirWaitHandle* mir_surface_request_persistent_id(MirSurface* surface, mir_surface_id_callback callback, void* context)
+ {
      mir::require(mir_surface_is_valid(surface));
 === modified file 'src/client/symbols.map'
 --- src/client/symbols.map	2016-08-31 09:41:42 +0000
 +++ src/client/symbols.map	2016-09-02 09:12:06 +0000
@@ -141,6 +141,7 @@
      mir_surface_spec_set_name;
      mir_surface_spec_set_parent;
      mir_surface_spec_set_pixel_format;
++    mir_surface_spec_set_placement;
      mir_surface_spec_set_preferred_orientation;
      mir_surface_spec_set_state;
      mir_surface_spec_set_streams;
 === modified file 'src/protobuf/mir_protobuf.proto'
 --- src/protobuf/mir_protobuf.proto	2016-08-09 06:44:52 +0000
 +++ src/protobuf/mir_protobuf.proto	2016-09-02 09:12:06 +0000
@@ -46,6 +46,12 @@
    optional int32 shell_chrome = 24;
    repeated StreamConfiguration stream = 25;
    optional int32 confine_pointer = 26;
++
++  optional int32 placement_hints = 27;
++  optional int32 surface_placement_gravity = 28;
++  optional int32 aux_rect_placement_gravity = 29;
++  optional int32 aux_rect_placement_offset_x = 30;
++  optional int32 aux_rect_placement_offset_y = 31;
+ }
  message SurfaceAspectRatio
@@ -86,6 +92,13 @@
    repeated Rectangle input_shape = 23;
    optional int32 shell_chrome = 24;
    optional int32 confine_pointer = 25;
++
++  // Intentionally missing 26 to get any further additions in line with SurfaceParameters
++  optional int32 placement_hints = 27;
++  optional int32 surface_placement_gravity = 28;
++  optional int32 aux_rect_placement_gravity = 29;
++  optional int32 aux_rect_placement_offset_x = 30;
++  optional int32 aux_rect_placement_offset_y = 31;
+ }
  message StreamConfiguration {
 === modified file 'src/server/frontend/session_mediator.cpp'
 --- src/server/frontend/session_mediator.cpp	2016-08-31 09:41:42 +0000
 +++ src/server/frontend/session_mediator.cpp	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012-2014 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 3,
@@ -285,6 +285,11 @@
      COPY_IF_SET(height_inc);
      COPY_IF_SET(shell_chrome);
      COPY_IF_SET(confine_pointer);
++    COPY_IF_SET(aux_rect_placement_gravity);
++    COPY_IF_SET(surface_placement_gravity);
++    COPY_IF_SET(placement_hints);
++    COPY_IF_SET(aux_rect_placement_offset_x);
++    COPY_IF_SET(aux_rect_placement_offset_y);
      #undef COPY_IF_SET
@@ -621,6 +626,11 @@
      COPY_IF_SET(preferred_orientation);
      COPY_IF_SET(parent_id);
      // aux_rect is a special case (below)
++    COPY_IF_SET(aux_rect_placement_gravity);
++    COPY_IF_SET(surface_placement_gravity);
++    COPY_IF_SET(placement_hints);
++    COPY_IF_SET(aux_rect_placement_offset_x);
++    COPY_IF_SET(aux_rect_placement_offset_y);
      COPY_IF_SET(edge_attachment);
      COPY_IF_SET(min_width);
      COPY_IF_SET(min_height);
 === modified file 'tests/acceptance-tests/test_custom_window_management.cpp'
 --- tests/acceptance-tests/test_custom_window_management.cpp	2016-01-29 08:18:22 +0000
 +++ tests/acceptance-tests/test_custom_window_management.cpp	2016-09-02 09:12:06 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2015 Canonical Ltd.
++ * Copyright © 2015-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 3,
@@ -360,3 +360,160 @@
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
+ }
++
++TEST_F(CustomWindowManagement, when_the_client_places_a_new_surface_the_request_reaches_the_window_manager)
++{
++    int const width{800};
++    int const height{600};
++    MirPixelFormat const format{mir_pixel_format_bgr_888};
++    MirRectangle dummy_rect{13, 17, 19, 23};
++    MirRectangle const aux_rect{20, 20, 50, 50};
++    auto const rect_gravity = mir_placement_gravity_northeast;
++    auto const surface_gravity = mir_placement_gravity_northwest;
++    auto const placement_hints = mir_placement_hints_flip_x;
++    auto const offset_dx = 2;
++    auto const offset_dy = 3;
++
++    start_server();
++    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++    auto surface_spec = mir_connection_create_spec_for_normal_surface(connection, width, height, format);
++    auto parent = mir_surface_create_sync(surface_spec);
++    mir_surface_spec_release(surface_spec);
++
++    surface_spec = mir_connection_create_spec_for_tip(
++        connection, width, height, format, parent, &dummy_rect, mir_edge_attachment_any);
++
++    mir_surface_spec_set_placement(
++        surface_spec, &aux_rect, rect_gravity, surface_gravity, placement_hints, offset_dx, offset_dy);
++
++    mt::Signal received;
++
++    auto const check_placement = [&](
++        std::shared_ptr<ms::Session> const& session,
++        ms::SurfaceCreationParameters const& params,
++        std::function<mf::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& build)
++        {
++            EXPECT_TRUE(params.aux_rect.is_set());
++            if (params.aux_rect.is_set())
++            {
++                auto const actual_rect = params.aux_rect.value();
++                EXPECT_THAT(actual_rect.top_left.x, Eq(geom::X{aux_rect.left}));
++                EXPECT_THAT(actual_rect.top_left.y, Eq(geom::Y{aux_rect.top}));
++                EXPECT_THAT(actual_rect.size.width, Eq(geom::Width{aux_rect.width}));
++                EXPECT_THAT(actual_rect.size.height, Eq(geom::Height{aux_rect.height}));
++            }
++
++            EXPECT_TRUE(params.placement_hints.is_set());
++            if (params.placement_hints.is_set())
++                EXPECT_THAT(params.placement_hints.value(), Eq(placement_hints));
++
++            EXPECT_TRUE(params.surface_placement_gravity.is_set());
++            if (params.surface_placement_gravity.is_set())
++                EXPECT_THAT(params.surface_placement_gravity.value(), Eq(surface_gravity));
++
++            EXPECT_TRUE(params.aux_rect_placement_gravity.is_set());
++            if (params.aux_rect_placement_gravity.is_set())
++                EXPECT_THAT(params.aux_rect_placement_gravity.value(), Eq(rect_gravity));
++
++            EXPECT_TRUE(params.aux_rect_placement_offset_x.is_set());
++            if (params.aux_rect_placement_offset_x.is_set())
++                EXPECT_THAT(params.aux_rect_placement_offset_x.value(), Eq(offset_dx));
++
++            EXPECT_TRUE(params.aux_rect_placement_offset_y.is_set());
++            if (params.aux_rect_placement_offset_y.is_set())
++                EXPECT_THAT(params.aux_rect_placement_offset_y.value(), Eq(offset_dy));
++
++            received.raise();
++            return build(session, params);
++        };
++
++    EXPECT_CALL(window_manager, add_surface(_,_,_)).WillOnce(Invoke(check_placement));
++
++    auto child = mir_surface_create_sync(surface_spec);
++    mir_surface_spec_release(surface_spec);
++
++    EXPECT_TRUE(received.wait_for(400ms));
++
++    mir_surface_release_sync(child);
++    mir_surface_release_sync(parent);
++    mir_connection_release(connection);
++}
++
++TEST_F(CustomWindowManagement, when_the_client_places_an_existing_surface_the_request_reaches_the_window_manager)
++{
++    int const width{800};
++    int const height{600};
++    MirPixelFormat const format{mir_pixel_format_bgr_888};
++    MirRectangle dummy_rect{13, 17, 19, 23};
++    MirRectangle const aux_rect{42, 15, 24, 7};
++    auto const rect_gravity = mir_placement_gravity_north;
++    auto const surface_gravity = mir_placement_gravity_south;
++    auto const placement_hints = MirPlacementHints(mir_placement_hints_flip_y|mir_placement_hints_antipodes);
++    auto const offset_dx = 5;
++    auto const offset_dy = 7;
++
++    start_server();
++    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++    auto surface_spec = mir_connection_create_spec_for_normal_surface(connection, width, height, format);
++    auto parent = mir_surface_create_sync(surface_spec);
++    mir_surface_spec_release(surface_spec);
++
++    surface_spec = mir_connection_create_spec_for_menu(
++        connection, width, height, format, parent, &dummy_rect, mir_edge_attachment_any);
++    auto child = mir_surface_create_sync(surface_spec);
++    mir_surface_spec_release(surface_spec);
++
++    surface_spec = mir_connection_create_spec_for_changes(connection);
++    mir_surface_spec_set_placement(
++        surface_spec, &aux_rect, rect_gravity, surface_gravity, placement_hints, offset_dx, offset_dy);
++
++    mt::Signal received;
++
++    auto const check_placement = [&](
++        std::shared_ptr<ms::Session> const&,
++        std::shared_ptr<ms::Surface> const&,
++        msh::SurfaceSpecification const& spec)
++        {
++            EXPECT_TRUE(spec.aux_rect.is_set());
++            if (spec.aux_rect.is_set())
++            {
++                auto const actual_rect = spec.aux_rect.value();
++                EXPECT_THAT(actual_rect.top_left.x, Eq(geom::X{aux_rect.left}));
++                EXPECT_THAT(actual_rect.top_left.y, Eq(geom::Y{aux_rect.top}));
++                EXPECT_THAT(actual_rect.size.width, Eq(geom::Width{aux_rect.width}));
++                EXPECT_THAT(actual_rect.size.height, Eq(geom::Height{aux_rect.height}));
++            }
++
++            EXPECT_TRUE(spec.placement_hints.is_set());
++            if (spec.placement_hints.is_set())
++                EXPECT_THAT(spec.placement_hints.value(), Eq(placement_hints));
++
++            EXPECT_TRUE(spec.surface_placement_gravity.is_set());
++            if (spec.surface_placement_gravity.is_set())
++                EXPECT_THAT(spec.surface_placement_gravity.value(), Eq(surface_gravity));
++
++            EXPECT_TRUE(spec.aux_rect_placement_gravity.is_set());
++            if (spec.aux_rect_placement_gravity.is_set())
++                EXPECT_THAT(spec.aux_rect_placement_gravity.value(), Eq(rect_gravity));
++
++            EXPECT_TRUE(spec.aux_rect_placement_offset_x.is_set());
++            if (spec.aux_rect_placement_offset_x.is_set())
++                EXPECT_THAT(spec.aux_rect_placement_offset_x.value(), Eq(offset_dx));
++
++            EXPECT_TRUE(spec.aux_rect_placement_offset_y.is_set());
++            if (spec.aux_rect_placement_offset_y.is_set())
++                EXPECT_THAT(spec.aux_rect_placement_offset_y.value(), Eq(offset_dy));
++
++            received.raise();
++        };
++
++    EXPECT_CALL(window_manager, modify_surface(_,_,_)).WillOnce(Invoke(check_placement));
++    mir_surface_apply_spec(child, surface_spec);
++    mir_surface_spec_release(surface_spec);
++
++    EXPECT_TRUE(received.wait_for(400ms));
++
++    mir_surface_release_sync(child);
++    mir_surface_release_sync(parent);
++    mir_connection_release(connection);
++}

Mir

Merge lp:~alan-griffiths/mir/support-gdk_window_move_to_rect into lp:mir

Commit Message

Description of the Change

Preview Diff

Subscribers