Mir

Merge lp:~alan-griffiths/mir/add-mir_surface_spec_attach into lp:mir

Proposed by Alan Griffiths
Status: Work in progress
Proposed branch: lp:~alan-griffiths/mir/add-mir_surface_spec_attach
Merge into: lp:mir
Diff against target: 140 lines (+75/-0)
5 files modified
include/client/mir_toolkit/mir_surface.h (+14/-0)
include/common/mir_toolkit/common.h (+12/-0)
src/client/mir_surface_api.cpp (+10/-0)
src/client/symbols.map (+6/-0)
tests/acceptance-tests/test_client_surfaces.cpp (+33/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/add-mir_surface_spec_attach
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Needs Fixing
Daniel van Vugt Needs Fixing
Kevin DuBois (community) Approve
Alan Griffiths Needs Information
Cemil Azizoglu (community) Approve
Review via email: mp+300202@code.launchpad.net

Commit message

Add mir_surface_spec_set_placement() to (re)position surfaces. (Initially only supporting edge attachment to a hint rectangle relative to the parent.)

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3595
https://mir-jenkins.ubuntu.com/job/mir-ci/1299/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1524/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1568
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1568
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1568
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1539/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1539/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1539/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks unrelated:

15:34:10 [ RUN ] ClientStartupPerformance.create_surface_and_swap
15:34:10 [2016-07-15 15:34:10.696077] mirplatform: Found graphics driver: mir:android (version 0.24.0)
15:34:10 [2016-07-15 15:34:10.697104] mirplatform: Found graphics driver: mir:mesa-kms (version 0.24.0)
15:34:10 [2016-07-15 15:34:10.697384] mirplatform: Found graphics driver: mir:mesa-x11 (version 0.24.0)
15:34:10 [2016-07-15 15:34:10.698068] mirplatform: Found graphics driver: mir:stub-graphics (version 0.24.0)
15:34:10 [2016-07-15 15:34:10.698278] mirplatform: Found graphics driver: throw-on-creation (version 0.24.0)
15:34:19 libevdev error in fix_invalid_absinfo: BUG: Device "mtk-tpd" has invalid ABS_MT_TRACKING_ID rangeSegmentation fault (core dumped)

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3595
https://mir-jenkins.ubuntu.com/job/mir-ci/1301/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1526
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1579
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1570
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1570
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1570
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1541
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1541/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1541
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1541/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1541
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1541/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1541
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1541/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1541
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1541/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

My gut tells me we should maybe be resolving bug 1420334 at the same time. And this API appears to not.

review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> My gut tells me we should maybe be resolving bug 1420334 at the same time. And
> this API appears to not.

There are various feature requests in this area that may require additional APIs. These are the ones I'm aware of ATM:

1. Repositioning with respect to parent. (relates to, but doesn't solve lp:1603086)
2. "Update MirAttachmentEdge API for GTK requirements" (essentially support for GDK APIs https://bugzilla.gnome.org/show_bug.cgi?id=756579#c311)
3. An API for movement of a surface relative to its current location lp:1420334
4. The ongoing discussion around possible inconsistencies of the window management spec and the toolkit API. (c.f. the "tooltips - everything is simple until you understand the problem" thread.)

Resolving any or all of these may introduce or update some toolkit APIs in this area.

My gut says that the API proposed here helps with an immediate problem without getting in the way of further enhancements. But maybe this is a good time for sketching out a final solution?

Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

The name of the function isn't intuitive w.r.t. what it does. Perhaps, mir_surface_spec_attach_relative or mir_surface_spec_attach_parent or something like that might make it more intuitive.

No tests?

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

OK on the first issue. Although "helps with an immediate problem without getting in the way of further enhancements" is a benefit that's also contradictory to good engineering: Everything you write should ideally have multiple uses so that functionality grows exponentially faster than the code size. We're failing on that part.

So Abstain except for Cemil's comment...
I agree that mir_surface_spec_attach is not an adequate name on first glance. You should be able to roughly figure out the purpose from the function name.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> OK on the first issue. Although "helps with an immediate problem without
> getting in the way of further enhancements" is a benefit that's also
> contradictory to good engineering: Everything you write should ideally have
> multiple uses so that functionality grows exponentially faster than the code
> size. We're failing on that part.

Having a function that (re)specifies the attachment without reference to the surface type does provide multiple uses. If we later (as one suggestion has it) enhance MirEdgeAttachment it automatically gains that functionality.

> So Abstain except for Cemil's comment...
> I agree that mir_surface_spec_attach is not an adequate name on first glance.
> You should be able to roughly figure out the purpose from the function name.

I'm in the market for a better name, but I don't like the current suggestions as much as the proposed.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Here's an alternative API that might work better with a view to future direction.

If this is acceptable tests can follow.

Revision history for this message
Kevin DuBois (kdub) wrote :

> > OK on the first issue. Although "helps with an immediate problem without
> > getting in the way of further enhancements" is a benefit that's also
> > contradictory to good engineering: Everything you write should ideally have
> > multiple uses so that functionality grows exponentially faster than the code
> > size. We're failing on that part.
>
> Having a function that (re)specifies the attachment without reference to the
> surface type does provide multiple uses. If we later (as one suggestion has
> it) enhance MirEdgeAttachment it automatically gains that functionality.

I suppose there's also the issue of how easy things are to understand (which naming and typing helps with). I'd rather see two needs fulfilled by two different easy-to-understand functions than two needs fulfilled by one harder-to-understand function. It seems that we don't really know the future enhancements, so I'd rather just see

mir_surface_spec_attach_relative(MirSurfaceSpec*, MirPlaceMode, MirRectangle*)

without reserving the upper bits of the mode as opcodes that alter what the function does

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3596
https://mir-jenkins.ubuntu.com/job/mir-ci/1316/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1543
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1596
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1558
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1558/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1558
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1558/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1558
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1558/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1558
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1558/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1558
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1558/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I suppose there's also the issue of how easy things are to understand (which
> naming and typing helps with). I'd rather see two needs fulfilled by two
> different easy-to-understand functions than two needs fulfilled by one harder-
> to-understand function. It seems that we don't really know the future
> enhancements, so I'd rather just see
>
> mir_surface_spec_attach_relative(MirSurfaceSpec*, MirPlaceMode, MirRectangle*)

You mean mir_surface_spec_attach_relative(MirSurfaceSpec*, MirEdgeAttachment, MirRectangle*)

>
> without reserving the upper bits of the mode as opcodes that alter what the
> function does

That was motivated by skimming https://blog.gtk.org/2016/07/15/future-of-relative-window-positioning/ and https://bugzilla.gnome.org/show_bug.cgi?id=756579#c311

Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

LGTM... Just needs the tests.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

(1) Function naming getting better. My confusion stems from the function name syntax:
   mir_surface_spec_place
The reader assumes that 'place' is what you're doing with a 'spec'. Actually 'place' is what we're doing with the surface. So that's a source of confusion we're never really going to avoid with the current prefix.

(2) Enum name needs fixing. It sounds like an action rather than an attribute:
  enum MirPlaceSurface
That should be something like 'MirSurfacePlacement'.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Another day, another name, ...

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3599
https://mir-jenkins.ubuntu.com/job/mir-ci/1327/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1565
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1618
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1609
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1609
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1609
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1580
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1580/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1580
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1580/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1580
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1580/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1580
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1580/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1580
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1580/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Reaproving...

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I considered 'MirPlacement' but decided not to suggest it because 'MirPlacement' is too general. Once we have a "MirPlacement" that's just for surfaces then if something else needs a placement enum the naming will look inconsistent: MirPlacement, MirSomethingElsePlacement

Please mention it's for placing surfaces in the least, and ideally edges too. So something like 'MirSurfacePlacementEdge'.

I know the team often belittles the importance of naming, but this is a permanent decision about the clarity of our communication to developers.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I considered 'MirPlacement' but decided not to suggest it because
> 'MirPlacement' is too general. Once we have a "MirPlacement" that's just for
> surfaces then if something else needs a placement enum the naming will look
> inconsistent: MirPlacement, MirSomethingElsePlacement
>
> Please mention it's for placing surfaces in the least, and ideally edges too.
> So something like 'MirSurfacePlacementEdge'.

I should have been more explicit in my intent: I aim to provide a route to add further enum values unrelated to "edge attachment". (For an example, see the referenced GDK discussion which has other options such as "gravity".) For that reason I don't want to make the name specific to "Edge".

I don't believe we need to specify placement strategies for anything other than surfaces. (Buffer streams are positioned directly.)

To be fully explicit in our naming we could say MirSurfacePlacementStrategy, but that seems a bit long. I feel MirPlacement is enough.

> I know the team often belittles the importance of naming, but this is a
> permanent decision about the clarity of our communication to developers.

We agree about the importance of good names. Here are some options:

Mir[Surface|][Placement|Layout|Position][Strategy|Option|Constraints|]

Votes (or other options) please!

review: Needs Information
Revision history for this message
Gerry Boland (gerboland) wrote :

I'm ok with MirPlacement, as long as "placement" is comprehensively deployed.

"Position" to me implies coordinates, whereas placement is less specific. So I'm ok with it.

Revision history for this message
Kevin DuBois (kdub) wrote :

I'd still prefer:

typedef enum MirEdgeAttachment; //keep as-is in common.h
mir_surface_spec_set_attachment(MirSurfaceSpec* spec, MirEdgeAttachment mode, MirRectangle* hint);

Revision history for this message
Kevin DuBois (kdub) wrote :

after chatting on IRC, seems the problem is wide enough to go with a packed-value or opcode approach for ABI stability. We have a packed-value approach in this MP, would slightly prefer an opcode approach, but am alright with the packed-value.

re MirPlacement discussion,
seeing as the future plans are to expand, MirPlacement seems to me to specific enough to suggest to the user what its for, while still being generic enough to allow different placement modes to be set, so I'm alright MirPlacement.

suppose that boils down to approve.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I'm not completely comfortable with "MirPlacement", but am sufficiently satisfied very few things other than Surfaces ever require placement.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Oops, just noticed: You're using the MirPlacement enum to hold potentially multiple values:
   void mir_surface_spec_set_placement(MirSurfaceSpec* spec, MirPlacement mode, MirRectangle* hint);

Compilers will start throwing up warnings and errors as soon as you pass in more than one value for 'mode'. So MirPlacement needs splitting into two types as we do for all other packed types:
    typedef enum MirPlacementBits ...
    typedef unsigned int MirPlacement;

Also missing a test case that will demonstrate the compiler error.

review: Needs Fixing
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I guess I'll revisit this when looking to extend the placement options. (Which is likely a few weeks away.)

Unmerged revisions

3599. By Alan Griffiths

ClientSurfaces.when_client_places_tooltip_then_window_manager_sees_corresponding_request

3598. By Alan Griffiths

merge lp:mir

3597. By Alan Griffiths

Try another naming style

3596. By Alan Griffiths

See if mir_surface_spec_place() is more popular

3595. By Alan Griffiths

Add mir_surface_spec_attach() to (re)position surfaces

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/client/mir_toolkit/mir_surface.h'
2--- include/client/mir_toolkit/mir_surface.h 2016-06-09 00:38:26 +0000
3+++ include/client/mir_toolkit/mir_surface.h 2016-07-20 09:00:59 +0000
4@@ -440,6 +440,20 @@
5 MirEdgeAttachment edge);
6
7 /**
8+ * Request that the surface be placed.
9+ *
10+ * If (mode&mir_place_mode_edge_attachment) then the hint is interpreted as an attachment
11+ * rectangle relative to the parent surface and (mode&0xFF) as a MirEdgeAttachment.
12+ * Otherwise the call has no effect.
13+ *
14+ * \param [in] spec Specification to mutate
15+ * \param [in] mode The placement mode.
16+ * \param [in] hint A hint region for placement.
17+ *
18+ */
19+void mir_surface_spec_set_placement(MirSurfaceSpec* spec, MirPlacement mode, MirRectangle* hint);
20+
21+/**
22 * Set the requested state.
23 * \param [in] spec Specification to mutate
24 * \param [in] mode Requested state
25
26=== modified file 'include/common/mir_toolkit/common.h'
27--- include/common/mir_toolkit/common.h 2016-06-08 21:16:46 +0000
28+++ include/common/mir_toolkit/common.h 2016-07-20 09:00:59 +0000
29@@ -199,6 +199,18 @@
30 mir_edge_attachment_horizontal
31 } MirEdgeAttachment;
32
33+// bits 24+ reserved for placement style
34+// bits 0-15 reserved for placement options
35+// bits 16-23 reserved for future
36+typedef enum MirPlacement
37+{
38+ mir_place_mode_edge_attachment = 1 << 24,
39+
40+ mir_place_edge_attachment_vertical = mir_place_mode_edge_attachment|mir_edge_attachment_vertical,
41+ mir_place_edge_attachment_horizontal= mir_place_mode_edge_attachment|mir_edge_attachment_horizontal,
42+ mir_place_edge_attachment_any = mir_place_mode_edge_attachment|mir_edge_attachment_any,
43+} MirPlacement;
44+
45 /**
46 * Form factor associated with a physical output
47 */
48
49=== modified file 'src/client/mir_surface_api.cpp'
50--- src/client/mir_surface_api.cpp 2016-06-08 13:49:11 +0000
51+++ src/client/mir_surface_api.cpp 2016-07-20 09:00:59 +0000
52@@ -760,6 +760,16 @@
53 return true;
54 }
55
56+void mir_surface_spec_set_placement(MirSurfaceSpec* spec, MirPlacement mode, MirRectangle* hint)
57+{
58+ if (mode & mir_place_mode_edge_attachment)
59+ {
60+ mir::require(hint != nullptr);
61+ spec->aux_rect = *hint;
62+ spec->edge_attachment = MirEdgeAttachment(mode&0xFF);
63+ }
64+}
65+
66 char const* mir_persistent_id_as_string(MirPersistentId *id)
67 {
68 return id->as_string().c_str();
69
70=== modified file 'src/client/symbols.map'
71--- src/client/symbols.map 2016-07-07 09:54:02 +0000
72+++ src/client/symbols.map 2016-07-20 09:00:59 +0000
73@@ -415,3 +415,9 @@
74 mir_input_device_state_event_device_pointer_buttons;
75 mir_surface_spec_set_pointer_confinement;
76 } MIR_CLIENT_0.22;
77+
78+MIR_CLIENT_0.25 { # New functions in Mir 0.25
79+ global:
80+ mir_surface_spec_set_placement;
81+} MIR_CLIENT_0.24;
82+
83
84=== modified file 'tests/acceptance-tests/test_client_surfaces.cpp'
85--- tests/acceptance-tests/test_client_surfaces.cpp 2016-05-03 06:55:25 +0000
86+++ tests/acceptance-tests/test_client_surfaces.cpp 2016-07-20 09:00:59 +0000
87@@ -28,6 +28,7 @@
88 #include "mir_test_framework/any_surface.h"
89 #include "mir/test/validity_matchers.h"
90 #include "mir/test/fake_shared.h"
91+#include "mir/test/signal.h"
92
93 #include <gmock/gmock.h>
94 #include <gtest/gtest.h>
95@@ -42,6 +43,8 @@
96 namespace mt = mir::test;
97 namespace mg = mir::geometry;
98
99+using namespace testing;
100+
101 namespace
102 {
103 struct SurfaceSync
104@@ -263,6 +266,36 @@
105 mir_surface_release_sync(tooltip);
106 }
107
108+TEST_F(ClientSurfaces, when_client_places_tooltip_then_window_manager_sees_corresponding_request)
109+{
110+ auto parent = mtf::make_any_surface(connection);
111+ MirRectangle zone_rect{100, 200, 100, 100};
112+
113+ auto spec = mir_connection_create_spec_for_tooltip(connection, 640, 480,
114+ mir_pixel_format_abgr_8888, parent, &zone_rect);
115+ auto tooltip = mir_surface_create_sync(spec);
116+ mir_surface_spec_release(spec);
117+
118+ spec = mir_connection_create_spec_for_changes(connection);
119+ mir_surface_spec_set_placement(spec, mir_place_edge_attachment_vertical, &zone_rect);
120+
121+ mt::Signal signal;
122+ EXPECT_CALL(window_manager, modify_surface(_, _, _)).
123+ WillOnce(WithArg<2>(Invoke([&](msh::SurfaceSpecification const& specification)
124+ {
125+ EXPECT_TRUE(specification.aux_rect.is_set());
126+ EXPECT_TRUE(specification.edge_attachment.is_set());
127+ signal.raise();
128+ })));
129+
130+ mir_surface_apply_spec(tooltip, spec);
131+ mir_surface_spec_release(spec);
132+
133+ signal.wait_for(std::chrono::seconds(10));
134+ mir_surface_release_sync(tooltip);
135+ mir_surface_release_sync(parent);
136+}
137+
138 TEST_F(ClientSurfaces, can_be_dialogs)
139 {
140 auto spec = mir_connection_create_spec_for_dialog(connection, 640, 480,

Subscribers

People subscribed via source and target branches