Merge lp:~vanvugt/mir/parenting-client-api into lp:mir
- parenting-client-api
- Merge into development-branch
Status: | Rejected |
---|---|
Rejected by: | Daniel van Vugt |
Proposed branch: | lp:~vanvugt/mir/parenting-client-api |
Merge into: | lp:mir |
Prerequisite: | lp:~vanvugt/mir/parenting-server-api |
Diff against target: |
289 lines (+145/-1) 13 files modified
client-ABI-sha1sums (+1/-1) include/client/mir_toolkit/mir_surface.h (+16/-0) src/client/connection_surface_map.h (+1/-0) src/client/mir_connection.cpp (+5/-0) src/client/mir_connection.h (+3/-0) src/client/mir_surface.cpp (+7/-0) src/client/mir_surface.h (+1/-0) src/client/mir_surface_api.cpp (+22/-0) src/client/surface_map.cpp (+7/-0) src/client/surface_map.h (+1/-0) tests/acceptance-tests/test_client_library.cpp (+43/-0) tests/acceptance-tests/test_client_surface_events.cpp (+33/-0) tests/unit-tests/client/test_protobuf_rpc_channel.cpp (+5/-0) |
To merge this branch: | bzr merge lp:~vanvugt/mir/parenting-client-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Cemil Azizoglu (community) | Disapprove | ||
Alan Griffiths | Disapprove | ||
Andreas Pokorny (community) | Needs Fixing | ||
Robert Carr (community) | Needs Fixing | ||
Alberto Aguirre (community) | Needs Fixing | ||
Review via email:
|
Commit message
Introduce a client API for surface parenting.
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Alberto Aguirre (albaguirre) wrote : | # |
===
28 +MirWaitHandle* mir_surface_
===
So in the previous MP there was some points of discussion:
1. mir_surface_
2. The bigger issue - No semantic meaning in setting a parent (at least not yet).
So essentially the discussion is do we favor for example for specifying dialogs/popups
A -
set_parent(...)
set_surface_
set_relative_
or B as Chris suggests:
set_dialog(surface, parent)
set_popup(surface, parent, relative_position)
I favor B as it has the semantic information we need to implement for our immediate goals and it limits error checking the state combinations of A.
Not dissaprove as parts of this are still useful for approach B above.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Robert Carr (robertcarr) wrote : | # |
I didn't look for programming or style errors.
At a high level goal though im told our target is to enable menus next week and so this branch wont be enough as it lacks child surface positioning.
Of course it would be possible to implement menus with this branch and a little more:
mir_surface_
mir_surface_
mir_surface_
mir_surface_
It raises some debate: Can parents be changed? What happens if I set a relative position but am parented? Should mir_surface_
Ultimately I think all these questions do have answers but they are not very interesting. Its much more interesting to have menus next week and I think chriss proposal of mir_surface_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
Alberto:
I strongly disagree. Your proposed API is not only more complex but also less capable, more redundant and more sensitive to ABI breaks. Redundant because set_dialog is the same as set_type(dialog). And redundant because relative positioning is useful to things other than just popups (e.g. the proposed decoration design where the titlebar is also a relatively positioned subsurface). More sensitive because the type "dialog" is just the current design open to change. When things do change like "dialog" you don't want to be deprecating entire functions with the type their names.
The API design proposed here is simpler, more flexible, has zero redundancy and less sensitive to ABI breaks. Remember we're not designing a toolkit with fixed semantics, but an API to support _any_ toolkit.
Robert:
Can parents be changed? Yes, see GTK. And even still, it's not sensible to impose arbitrary limitations without good reason.
What happens if I set a relative position but am (not?) parented? Am: You get positioned; Not: You don't get positioned. But also that question is not relevant to this proposal.
Should you be able to set a parent without a relative position!? Yes, two immediate use cases at least: floating toolboxes, indicators (auxiliary surfaces design doc).
What should these functions be called? Intentionally not a question to be answered in this small proposal.
Is setting a menu type without a parent an error or does the shell simply fail to change type for you? Semantics are not fully defined in Mir itself. We're open to letting the shell enfoce that (as all shells/toolkits are different which is a good thing).
Does the creation of these sort of attribute dances effectively mean creating an implied Window Management protocol which starts to look very Xey? No, we are designing an API to support toolkits.
"mir_surface_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
A more visual explanation:
sometoolkit_
surf = mir_connection_
mir_
mir_
mir_
return surf;
So you see there are two important benefits:
1. The Mir API is minimal in size (in fact intentionally normalized [1])
2. The Mir API is maximally flexible; Mir doesn't need to know what a "menu" is. The toolkit is free to define whatever it likes.
[1] http://
It's also important to remember that our design documents detail the design of the Unity shell. Mir needs to support more than just Unity.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2044
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Alan Griffiths (alan-griffiths) wrote : | # |
Failure is lp:1390388
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2045
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Pokorny (andreas-pokorny) wrote : | # |
> A more visual explanation:
>
> sometoolkit_
> surf = mir_connection_
> mir_surface_
> mir_surface_
> mir_surface_
> return surf;
>
> So you see there are two important benefits:
> 1. The Mir API is minimal in size (in fact intentionally normalized [1])
> 2. The Mir API is maximally flexible; Mir doesn't need to know what a "menu"
> is. The toolkit is free to define whatever it likes.
> [1] http://
The above also means that menus do not exist. Instead there will only be a combination of flags that might yield a related outcome. But the client/toolkit never states that intent. If a shell plans to implement a specific behavior or look for menues it will end up searching for those combinations of flags. Additionally menues and other popovers might not even be distinguishable based on the flags provided by the client/toolkit.
On the other hand of course I do like the idea of defining a menu as a set of behavioral flags that will be independently interpreted by different components in the shell. But thats an implementation detail on the server.
Still in a distant future we might decide to expose such things - a soon as they are or can be relatively well defined - to the client api, but I do think that we then still need a client API to declare the semantics or intent of a surface. Both things may coexist but I think the latter has a higher priority and is already in a well defined state (modulo permanent changes).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cemil Azizoglu (cemil-azizoglu) wrote : | # |
+1 for a semantics-based API as opposed to a state-based one. I wouldn't want to have to use an API where there are hundreds of states to choose from and a small subset of each has some special meaning (a menu, a tooltip, a dialog, etc) and other combinations are not even valid - giving too much flexibility to the user to shoot themselves in the foot.
I'd like to be able to just call one function to create, say, a menu. Simple enough for the user to get started using our API, simple enough for us to manage. Of course, as the need arises, we can later break down the API and provide finer level of control as is being proposed here. We can do this on a case-by-case basis as we gain some insight into what users want to achieve and where it makes sense to have more flexibility. It's easy, and backwards-
So IMO the API that is being proposed here is rather the implementation and could/should stay as an internal API, which could then be made public, with some extra state validation, if needed.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Alan Griffiths (alan-griffiths) wrote : | # |
> +1 for a semantics-based API as opposed to a state-based one. I wouldn't want
> to have to use an API where there are hundreds of states to choose from and a
> small subset of each has some special meaning (a menu, a tooltip, a dialog,
> etc) and other combinations are not even valid - giving too much flexibility
> to the user to shoot themselves in the foot.
>
> I'd like to be able to just call one function to create, say, a menu. Simple
> enough for the user to get started using our API, simple enough for us to
> manage. Of course, as the need arises, we can later break down the API and
> provide finer level of control as is being proposed here. We can do this on a
> case-by-case basis as we gain some insight into what users want to achieve and
> where it makes sense to have more flexibility. It's easy, and backwards-
> compatible, to do that. However, it's not easy to take it back cf. server ABI.
>
> So IMO the API that is being proposed here is rather the implementation and
> could/should stay as an internal API, which could then be made public, with
> some extra state validation, if needed.
If we have such an "internal API" in the client code I think we lose any benefit of having a "semantics-based API". The value I can see from in the latter approach would come from passing the semantic information "this is a menu" to the server so that the shell can implement policies to control any fine grained state.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cemil Azizoglu (cemil-azizoglu) wrote : | # |
> If we have such an "internal API" in the client code I think we lose any
> benefit of having a "semantics-based API". The value I can see from in the
> latter approach would come from passing the semantic information "this is a
> menu" to the server so that the shell can implement policies to control any
> fine grained state.
No I didn't mean it as a client-side API. The entry points of the client API defined by this MP would have corresponding functions on the server-side, which collectively implement the, say 'menu', functionality. I realize that it's confusing.. So feel free to scratch that comment altogether :-).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
More details and much more elaborate explanations are now duplicated on the mailing list: mir-devel. So I'll resist repeating myself here.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
I've just reviewed the Unity design doc with the misleading title "MIR: Surface Management v0.2" (an issue I've raised before because it's actually the design of Unity8, not Mir). It outlines 8 surface types/roles and 7 of them can have parents. Some required and some optional parenting.
So despite the fact I said I was going to look for a compromise, the design doc actually supports this current proposal rather strongly. Certainly we don't want to write 7 different set_parent client functions. Best case then is you've made the API 7 times more complex and still strictly support only one shell. More client functions might then be required for other shells or other surface types. It doesn't scale.
The design docs also detail complex state transitions and all the various combinations allowed. It's very complex for both surface_state and surface_type combinations, in concert with surface position and size etc. Some of these attributes whose consistency needs to be maintained for valid states exist only on the server side. So the client API can't enforce everything by itself obviously. You always have to "ask" the shell if any setting is allowed or has been accepted.
State validity and transitions will indeed be implemented on the server side because they're defined by the shell. There might be some sane defaults (e.g. Set state_fullscreen results in a resize), however the shell will have the power to wrap any state changes to guarantee its own desired semantics. And semantics may change at runtime; e.g. Unity8 in the phone form-factor only supports the "maximized" state for apps.
Back on the "menu" topic, I've proposed that as a real surface type today, separately. However since "menu" is only one of the seven Unity surface types that need parenting, and only some of those use parenting for relative positioning, it's clear we need a generic mir_surface_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2047
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Alan Griffiths (alan-griffiths) wrote : | # |
We've rejected this approach to API design.
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
I do make mistakes... And am happy to admit when I do.
However nobody has yet disproved (or even replied to) any of the logical arguments I have made thus far.
I am not always right. But no one has yet disproved anything I have said on this topic so far.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2048
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2050
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cemil Azizoglu (cemil-azizoglu) wrote : | # |
I believe this horse has been beaten to death - discussed over email, in this MP, and on hangout with the architect. Rejecting.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
It's the only working solution right now for an important feature.
I'll keep the review up till someone proposes a better solution (or lands a worse one :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2051
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel van Vugt (vanvugt) wrote : | # |
Alright, if someone decides later the alternate solutions are too painful then this branch is still here if you want it.
But I'll hide it again for now...
Unmerged revisions
- 2051. By Daniel van Vugt
-
Merge latest paren branch (and trunk) and fix a conflict.
- 2050. By Daniel van Vugt
-
Merge latest parent branch including trunk
- 2049. By Daniel van Vugt
-
Merge latest parent branch
- 2048. By Daniel van Vugt
-
Merge latest parent branch.
- 2047. By Daniel van Vugt
-
Merge latest parent (resolve criss-cross)
- 2046. By Daniel van Vugt
-
Merge latest trunk and fix a conflict
- 2045. By Daniel van Vugt
-
Merge latest parent branch (and trunk)
- 2044. By Daniel van Vugt
-
Merge latest parent branch
- 2043. By Daniel van Vugt
-
Merge latest parent branch (and trunk)
- 2042. By Daniel van Vugt
-
Merge latest parent branch (test enhancements only)
Preview Diff
1 | === modified file 'client-ABI-sha1sums' |
2 | --- client-ABI-sha1sums 2014-11-14 03:03:37 +0000 |
3 | +++ client-ABI-sha1sums 2014-11-14 03:03:37 +0000 |
4 | @@ -5,7 +5,7 @@ |
5 | 1ef8f51a3e3f8d1559266c5af58fbfde7cfabf0a include/client/mir_toolkit/mir_cursor_configuration.h |
6 | 6ff12425fed19f2a5aa37390904c14ff6a647bcd include/client/mir_toolkit/mir_prompt_session.h |
7 | 21d07e655e85eeec8a3523e1c6f9c2252176ec01 include/client/mir_toolkit/mir_screencast.h |
8 | -8609754db3be20e11e43858dd2c36b5bd480d5ec include/client/mir_toolkit/mir_surface.h |
9 | +a77d1b2ae1c15944477512cee129d5746890239d include/client/mir_toolkit/mir_surface.h |
10 | b141c4d79802ad626d969249c0004744e5c2a525 include/client/mir_toolkit/mir_wait.h |
11 | 9907751d046e4aea81881cf19e5df52c7a6a813e include/common/mir_toolkit/client_types.h |
12 | 77a5955e53e1901766372b4fb29e34872ef62667 include/common/mir_toolkit/common.h |
13 | |
14 | === modified file 'include/client/mir_toolkit/mir_surface.h' |
15 | --- include/client/mir_toolkit/mir_surface.h 2014-10-01 06:25:56 +0000 |
16 | +++ include/client/mir_toolkit/mir_surface.h 2014-11-14 03:03:37 +0000 |
17 | @@ -287,6 +287,22 @@ |
18 | */ |
19 | MirOrientation mir_surface_get_orientation(MirSurface *surface); |
20 | |
21 | +/** |
22 | + * Set the parent of a surface. |
23 | + * \param [in] surface The child surface that is being parented |
24 | + * \param [in] parent The new parent surface (or NULL if none) |
25 | + * \return A wait handle that is signaled when the operation |
26 | + * has completed. |
27 | + */ |
28 | +MirWaitHandle* mir_surface_set_parent(MirSurface* surface, MirSurface* parent); |
29 | + |
30 | +/** |
31 | + * Get the parent of a surface. |
32 | + * \param [in] surface The surface being queried |
33 | + * \return The parent surface (or NULL if none). |
34 | + */ |
35 | +MirSurface* mir_surface_get_parent(MirSurface* surface); |
36 | + |
37 | #ifdef __cplusplus |
38 | } |
39 | /**@}*/ |
40 | |
41 | === modified file 'src/client/connection_surface_map.h' |
42 | --- src/client/connection_surface_map.h 2014-10-01 06:25:56 +0000 |
43 | +++ src/client/connection_surface_map.h 2014-11-14 03:03:37 +0000 |
44 | @@ -36,6 +36,7 @@ |
45 | ~ConnectionSurfaceMap() noexcept; |
46 | |
47 | void with_surface_do(int surface_id, std::function<void(MirSurface*)> exec) const override; |
48 | + MirSurface* find(int surface_id) const noexcept override; |
49 | void insert(int surface_id, MirSurface* surface); |
50 | void erase(int surface_id); |
51 | |
52 | |
53 | === modified file 'src/client/mir_connection.cpp' |
54 | --- src/client/mir_connection.cpp 2014-11-11 15:50:11 +0000 |
55 | +++ src/client/mir_connection.cpp 2014-11-14 03:03:37 +0000 |
56 | @@ -549,3 +549,8 @@ |
57 | { |
58 | return logger; |
59 | } |
60 | + |
61 | +MirSurface* MirConnection::get_surface(int id) const noexcept |
62 | +{ |
63 | + return surface_map->find(id); |
64 | +} |
65 | |
66 | === modified file 'src/client/mir_connection.h' |
67 | --- src/client/mir_connection.h 2014-10-28 00:29:39 +0000 |
68 | +++ src/client/mir_connection.h 2014-11-14 03:03:37 +0000 |
69 | @@ -136,6 +136,9 @@ |
70 | mir::protobuf::DisplayServer& display_server(); |
71 | std::shared_ptr<mir::logging::Logger> const& the_logger() const; |
72 | |
73 | + /// Find a surface by id, or return null. |
74 | + MirSurface* get_surface(int id) const noexcept; |
75 | + |
76 | private: |
77 | // MUST be first data member so it is destroyed last. |
78 | struct Deregisterer |
79 | |
80 | === modified file 'src/client/mir_surface.cpp' |
81 | --- src/client/mir_surface.cpp 2014-11-11 15:50:11 +0000 |
82 | +++ src/client/mir_surface.cpp 2014-11-14 03:03:37 +0000 |
83 | @@ -171,6 +171,12 @@ |
84 | return surface.id().value(); |
85 | } |
86 | |
87 | +MirSurface* MirSurface::parent() const noexcept |
88 | +{ |
89 | + std::lock_guard<decltype(mutex)> lock(mutex); |
90 | + return connection->get_surface(attrib_cache[mir_surface_attrib_parent]); |
91 | +} |
92 | + |
93 | bool MirSurface::is_valid(MirSurface* query) |
94 | { |
95 | std::lock_guard<decltype(handle_mutex)> lock(handle_mutex); |
96 | @@ -487,6 +493,7 @@ |
97 | case mir_surface_attrib_focus: |
98 | case mir_surface_attrib_swapinterval: |
99 | case mir_surface_attrib_dpi: |
100 | + case mir_surface_attrib_parent: |
101 | if (configure_result.has_ivalue()) |
102 | attrib_cache[a] = configure_result.ivalue(); |
103 | else |
104 | |
105 | === modified file 'src/client/mir_surface.h' |
106 | --- src/client/mir_surface.h 2014-11-11 15:50:11 +0000 |
107 | +++ src/client/mir_surface.h 2014-11-14 03:03:37 +0000 |
108 | @@ -87,6 +87,7 @@ |
109 | MirSurfaceParameters get_parameters() const; |
110 | char const * get_error_message(); |
111 | int id() const; |
112 | + MirSurface* parent() const noexcept; |
113 | MirWaitHandle* next_buffer(mir_surface_callback callback, void * context); |
114 | MirWaitHandle* get_create_wait_handle(); |
115 | |
116 | |
117 | === modified file 'src/client/mir_surface_api.cpp' |
118 | --- src/client/mir_surface_api.cpp 2014-11-11 15:50:11 +0000 |
119 | +++ src/client/mir_surface_api.cpp 2014-11-14 03:03:37 +0000 |
120 | @@ -332,3 +332,25 @@ |
121 | |
122 | return result; |
123 | } |
124 | + |
125 | +MirWaitHandle* mir_surface_set_parent(MirSurface* surf, MirSurface* parent) |
126 | +{ |
127 | + MirWaitHandle *result = nullptr; |
128 | + try |
129 | + { |
130 | + if (surf && (!parent || MirSurface::is_valid(parent))) |
131 | + { |
132 | + int parent_id = parent ? parent->id() : mir_surface_parent_none; |
133 | + result = surf->configure(mir_surface_attrib_parent, parent_id); |
134 | + } |
135 | + } |
136 | + catch (std::exception const&) |
137 | + { |
138 | + } |
139 | + return result; |
140 | +} |
141 | + |
142 | +MirSurface* mir_surface_get_parent(MirSurface* surf) |
143 | +{ |
144 | + return surf ? surf->parent() : nullptr; |
145 | +} |
146 | |
147 | === modified file 'src/client/surface_map.cpp' |
148 | --- src/client/surface_map.cpp 2014-10-01 06:25:56 +0000 |
149 | +++ src/client/surface_map.cpp 2014-11-14 03:03:37 +0000 |
150 | @@ -62,6 +62,13 @@ |
151 | } |
152 | } |
153 | |
154 | +MirSurface* mcl::ConnectionSurfaceMap::find(int surface_id) const noexcept |
155 | +{ |
156 | + std::lock_guard<std::mutex> lk(guard); |
157 | + auto const it = surfaces.find(surface_id); |
158 | + return (it != surfaces.end()) ? it->second : nullptr; |
159 | +} |
160 | + |
161 | void mcl::ConnectionSurfaceMap::insert(int surface_id, MirSurface* surface) |
162 | { |
163 | std::lock_guard<std::mutex> lk(guard); |
164 | |
165 | === modified file 'src/client/surface_map.h' |
166 | --- src/client/surface_map.h 2013-12-17 17:55:59 +0000 |
167 | +++ src/client/surface_map.h 2014-11-14 03:03:37 +0000 |
168 | @@ -33,6 +33,7 @@ |
169 | public: |
170 | virtual void with_surface_do( |
171 | int surface_id, std::function<void(MirSurface*)> exec) const = 0; |
172 | + virtual MirSurface* find(int surface_id) const noexcept = 0; |
173 | |
174 | protected: |
175 | virtual ~SurfaceMap() = default; |
176 | |
177 | === modified file 'tests/acceptance-tests/test_client_library.cpp' |
178 | --- tests/acceptance-tests/test_client_library.cpp 2014-11-11 17:46:49 +0000 |
179 | +++ tests/acceptance-tests/test_client_library.cpp 2014-11-14 03:03:37 +0000 |
180 | @@ -329,6 +329,49 @@ |
181 | mir_connection_release(connection); |
182 | } |
183 | |
184 | +TEST_F(ClientLibrary, surface_parenting) |
185 | +{ |
186 | + connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__); |
187 | + ASSERT_TRUE(mir_connection_is_valid(connection)); |
188 | + |
189 | + MirSurfaceParameters const parm = |
190 | + { |
191 | + __PRETTY_FUNCTION__, |
192 | + 640, 480, |
193 | + mir_pixel_format_abgr_8888, |
194 | + mir_buffer_usage_hardware, |
195 | + mir_display_output_id_invalid |
196 | + }; |
197 | + |
198 | + auto parent = mir_connection_create_surface_sync(connection, &parm); |
199 | + ASSERT_TRUE(mir_surface_is_valid(parent)); |
200 | + |
201 | + auto child = mir_connection_create_surface_sync(connection, &parm); |
202 | + ASSERT_TRUE(mir_surface_is_valid(child)); |
203 | + |
204 | + EXPECT_EQ(NULL, mir_surface_get_parent(child)); |
205 | + |
206 | + mir_wait_for(mir_surface_set_parent(child, parent)); |
207 | + EXPECT_EQ(parent, mir_surface_get_parent(child)); |
208 | + |
209 | + int foo; |
210 | + auto invalid_surface = reinterpret_cast<MirSurface*>(&foo); |
211 | + mir_wait_for(mir_surface_set_parent(child, invalid_surface)); |
212 | + EXPECT_EQ(parent, mir_surface_get_parent(child)); |
213 | + |
214 | + mir_wait_for(mir_surface_set_parent(child, NULL)); |
215 | + EXPECT_EQ(NULL, mir_surface_get_parent(child)); |
216 | + |
217 | + mir_wait_for(mir_surface_set_parent(child, parent)); |
218 | + EXPECT_EQ(parent, mir_surface_get_parent(child)); |
219 | + |
220 | + mir_surface_release_sync(parent); |
221 | + EXPECT_EQ(NULL, mir_surface_get_parent(child)); |
222 | + mir_surface_release_sync(child); |
223 | + |
224 | + mir_connection_release(connection); |
225 | +} |
226 | + |
227 | #ifndef ANDROID |
228 | TEST_F(ClientLibrary, surface_scanout_flag_toggles) |
229 | { |
230 | |
231 | === modified file 'tests/acceptance-tests/test_client_surface_events.cpp' |
232 | --- tests/acceptance-tests/test_client_surface_events.cpp 2014-10-27 22:31:16 +0000 |
233 | +++ tests/acceptance-tests/test_client_surface_events.cpp 2014-11-14 03:03:37 +0000 |
234 | @@ -224,6 +224,39 @@ |
235 | } |
236 | } |
237 | |
238 | +TEST_F(ClientSurfaceEvents, client_receives_parenting_events) |
239 | +{ |
240 | + int surface_id = mir_debug_surface_id(surface); |
241 | + int other_surface_id = mir_debug_surface_id(other_surface); |
242 | + |
243 | + set_event_filter(mir_event_type_surface); |
244 | + |
245 | + { |
246 | + reset_last_event(); |
247 | + mir_wait_for(mir_surface_set_parent(surface, other_surface)); |
248 | + |
249 | + std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex}; |
250 | + EXPECT_THAT(last_event_surface, Eq(surface)); |
251 | + EXPECT_THAT(last_event.type, Eq(mir_event_type_surface)); |
252 | + EXPECT_THAT(last_event.surface.id, Eq(surface_id)); |
253 | + EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_parent)); |
254 | + EXPECT_THAT(last_event.surface.value, Eq(other_surface_id)); |
255 | + } |
256 | + |
257 | + { |
258 | + reset_last_event(); |
259 | + mir_wait_for(mir_surface_set_parent(surface, NULL)); |
260 | + |
261 | + std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex}; |
262 | + EXPECT_THAT(last_event_surface, Eq(surface)); |
263 | + EXPECT_THAT(last_event.type, Eq(mir_event_type_surface)); |
264 | + EXPECT_THAT(last_event.surface.id, Eq(surface_id)); |
265 | + EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_parent)); |
266 | + EXPECT_THAT(last_event.surface.value, Eq(mir_surface_parent_none)); |
267 | + } |
268 | +} |
269 | + |
270 | + |
271 | struct OrientationEvents : ClientSurfaceEvents, ::testing::WithParamInterface<MirOrientation> {}; |
272 | |
273 | TEST_P(OrientationEvents, surface_receives_orientation_events) |
274 | |
275 | === modified file 'tests/unit-tests/client/test_protobuf_rpc_channel.cpp' |
276 | --- tests/unit-tests/client/test_protobuf_rpc_channel.cpp 2014-10-28 13:59:51 +0000 |
277 | +++ tests/unit-tests/client/test_protobuf_rpc_channel.cpp 2014-11-14 03:03:37 +0000 |
278 | @@ -51,6 +51,11 @@ |
279 | void with_surface_do(int /*surface_id*/, std::function<void(MirSurface*)> /*exec*/) const |
280 | { |
281 | } |
282 | + |
283 | + MirSurface* find(int) const noexcept |
284 | + { |
285 | + return nullptr; |
286 | + } |
287 | }; |
288 | |
289 | class MockStreamTransport : public mclr::StreamTransport |
PASSED: Continuous integration, rev:2042 jenkins. qa.ubuntu. com/job/ mir-ci/ 2024/ jenkins. qa.ubuntu. com/job/ mir-android- vivid-i386- build/16 jenkins. qa.ubuntu. com/job/ mir-clang- vivid-amd64- build/16 jenkins. qa.ubuntu. com/job/ mir-vivid- amd64-ci/ 13 jenkins. qa.ubuntu. com/job/ mir-vivid- amd64-ci/ 13/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/mir- ci/2024/ rebuild
http://