Mir

Merge lp:~kdub/mir/RFC-surface-arrangements into lp:mir

Proposed by Kevin DuBois
Status: Superseded
Proposed branch: lp:~kdub/mir/RFC-surface-arrangements
Merge into: lp:mir
Prerequisite: lp:~kdub/mir/correct-dbc-public-api
Diff against target: 891 lines (+557/-48)
16 files modified
include/client/mir_toolkit/mir_surface.h (+79/-1)
include/server/mir/compositor/scene_element.h (+4/-7)
playground/demo-shell/demo_compositor.cpp (+2/-2)
playground/demo-shell/demo_renderer.cpp (+2/-2)
playground/demo-shell/demo_renderer.h (+1/-1)
src/client/mir_surface_api.cpp (+31/-4)
src/server/scene/surface_stack.cpp (+10/-4)
tests/acceptance-tests/CMakeLists.txt (+2/-0)
tests/acceptance-tests/test_client_surface_arrangement.cpp (+255/-0)
tests/acceptance-tests/test_render_override.cpp (+130/-0)
tests/include/mir_test_doubles/stub_scene_element.h (+6/-0)
tests/include/mir_test_framework/connected_client_with_a_surface.h (+8/-0)
tests/mir_test_framework/connected_client_with_a_surface.cpp (+1/-10)
tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp (+15/-8)
tests/unit-tests/examples/test_demo_compositor.cpp (+2/-1)
tests/unit-tests/scene/test_surface_stack.cpp (+9/-8)
To merge this branch: bzr merge lp:~kdub/mir/RFC-surface-arrangements
Reviewer Review Type Date Requested Status
Alan Griffiths Needs Information
Daniel van Vugt Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Kevin DuBois (community) Needs Resubmitting
Review via email: mp+254447@code.launchpad.net

This proposal has been superseded by a proposal from 2015-04-01.

Commit message

do not land.

Description of the change

Just a RFC branch!

In looking at how to bypass nested servers, it seemed useful that advanced clients (such as a multi-windowed client, a OS virtualizer, or a nested mir server) need a way to specify the ordering of the surfaces that they have created. This obviously has to be done within the SurfaceSpecs to prevent violating the policies the shell has (eg, you can't lower a tooltip beneath your primary surface), but normal surfaces should be arrangeable as the client wishes.

This version has the arrangement done batched together (so multiple changes are applied to the SurfaceStack at once), but I suppose some clients might not care that the arrangement updates are batched.

Also, the arrangement only applies to the surfaces in client's connection, the client neither sets an absolute x,y position, nor an absolute z-ordering.

I got to the point of writing a failing acceptance test for this feature when I thought I'd gather some comments on the client api change.

To post a comment you must log in.
Revision history for this message
Kevin DuBois (kdub) wrote :

Just an RFC on an unimplemented client api change. Resubmit after implementation.

review: Needs Resubmitting
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

(0) I've proposed an attribute batching API that will probably land soon and might be helpful. Check it out:
https://code.launchpad.net/~vanvugt/mir/rename-api/+merge/252864

Using that you could put relative ordering preferences of child surfaces in their MirSurfaceSpec instead. Which I think makes more sense.

(1) I think "order" is a more widely understood term than "arrangement" here. But also think the functions should be different in other ways than just name. See (0).

Instead, child surfaces could have an "int child_depth". Then their final Z-order is determined by (a) the depth of the parent surface within the main stack, plus (b) their relative int child_depth.

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

*Discussion*

I think this only addresses a part of a range of problems and would like to be sure that we have a consistent solution. As Daniel mentions, the stacking order of surfaces should be part of the problem domain here (at least to the extent that a client can e.g. "raise" some of its surfaces). We should also allow a client to shift the focus (or default focus) to a specific surface.

As proposed, this is making requests for a specific action without sufficient semantic content for the server to implement good alternative behavior if the request can't be met in full: if a window encounters an edge to the display should it overlap, resize or have an adjusted position?

Specifying top-left locations of surfaces seems like a simplistic layout rule. We should consider the existing solution domain of layout managers and, at least /consider/, allowing the specification of anchors, insets, fills, weights, etc.

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

> Specifying top-left locations of surfaces seems like a simplistic layout rule.
> We should consider the existing solution domain of layout managers and, at
> least /consider/, allowing the specification of anchors, insets, fills,
> weights, etc.

I'm not saying we can't do something adequate for now and evolve a more complex solution as needed, but that the current proposal is too specific to facilitate that evolution.

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

0) The batching api looks like it could be helpful in making sure that changes are applied at once. Packing the z-order into the MirSurfaceSpec might be more uniform, although I did not see a way to change the z order or the position once the surface was created.
1) I did consider specifying an int-depth as an attribute, but that seemed like additional processing for us, when the client is really just trying to establish a list of its surfaces. Also, if you have 2 surfaces at z-depth 4 and 5, and want 3rd surface in between, the client has to reassign one of the surfaces to fit that surface where they want (which seemed a bit easier to do if its just an array). A list or an array also eliminates the corner case of two surfaces at the same level that we'd have to check for.

I guess the other thing to point out is that I put this as a mir_connection_ function to avoid the idea of parent/child surfaces. It seems more flexible to just have a collection of surfaces ordered by the client in the way it prefers (while still sticking to our policies)

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

> I think this only addresses a part of a range of problems and would like to be
> sure that we have a consistent solution. As Daniel mentions, the stacking
> order of surfaces should be part of the problem domain here (at least to the
> extent that a client can e.g. "raise" some of its surfaces). We should also
> allow a client to shift the focus (or default focus) to a specific surface.

So the intent here is to let the client specify its own surface ordering, we're making no guarantees about what the final z-order or position the surface will be presented in on-screen. Its more that we're guaranteeing the client that their surfaces will be composited in a specific way amongst themselves.

>
> As proposed, this is making requests for a specific action without sufficient
> semantic content for the server to implement good alternative behavior if the
> request can't be met in full: if a window encounters an edge to the display
> should it overlap, resize or have an adjusted position?

I don't know that we want to reinterpret what the client has said it wants to do... If it violates our window policies, we'll reject the request. (or perhaps I've misunderstood the point)

>
> Specifying top-left locations of surfaces seems like a simplistic layout rule.
> We should consider the existing solution domain of layout managers and, at
> least /consider/, allowing the specification of anchors, insets, fills,
> weights, etc.

I'm interested in being able to represent a SceneElementSequence-like specification of the composition that the client wants, although don't mind further expansion.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> Specifying top-left locations of surfaces seems like a simplistic layout rule.
> We should consider the existing solution domain of layout managers and, at
> least /consider/, allowing the specification of anchors, insets, fills,
> weights, etc.

I think we need to distinguish between surfaces and renderbuffers/scenelements here. What you wrote would be true for forwarding surfaces...

I do not think this is part of the problem to be solved (at least for now). A solution is needed that allows a nested server to forward all information necessary to allow the hosting server to draw the scene as if the nested server did the composition. At the moment and in the near future the super set of our efficient drawing operations (hwc + setting a scan out buffer + atomic planes) is what DefaultDisplayBufferCompositor implements. Hence the nested server has to submit the information a DefaultDisplayBufferCompositor needs for rendering. Thus a sequence of Renderables (i think we can ignore the Decoration accessor in SceneElement for that, and also put aside the oddness around the semantics Renderable::transformation() vs Renderable::screen_position()).

Still the idea of forwarding surfaces could be interesting too.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

PS: oops hit save comment too soon.

So I see two different cases, a nested server should be allowed to forward surfaces as surfaces. That would nicely fulfill the case of the OS virtualizer, forwarding all windows of the guest system to the host system, hence the host is supposed to do all the window management and dispatch logic. In that case the nested server creates a surface at the host server for each client surfaces, and relays changes between those.

The other topic is that a nested server should be able to just relay all Renderable changes within the scene and buffer changes to the host server. So as a result a Surface then HasA sequence of Renderables.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

Back to the nested bypass: On the client maybe:

MirBufferStreamAttachment* mir_surface_create_buffer_stream_attachment(MirSurface*, MirBufferStream*, int relative_pos_x, int relative_pos_y, float* transformation?, float opacity);

and respective setters, and a mir_surface_submit_buffer_stream_changes that sends all pending changes in a single batch. Or make that more generic mir_surfae_submit_changes?

Revision history for this message
Chris Halse Rogers (raof) wrote :

So, relevant prior art here is:
http://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml#n1994 (for the purposes of that section, wl_surface == MirBufferStream, wl_surface.commit == mir_buffer_stream_swap_buffers)

The API contains the three obvious things - set_position, place_above, place_below - but also the non-obvious but important set_sync/set_desync. This serves two purposes - one is to batch up changes into a transaction, which we can easily satisfy by other means, the other is to ensure that the surface content of the whole subsurface tree is consistent.

When in synchronised mode, swap_buffers in all the subsurfaces is deferred until the parent surface enters swap_buffers; this ensures that the client can change the subsurface graph without flashing or components transiently moving out of place.

Like Andreas, I think that Surfaces are the wrong level to do this nesting. Surfaces have a whole bunch of semantics that USC simply doesn't care about, and can't do anything sensible with. What happens with input? Does USC generate enter/leave events? Etc.

Since Robert's animated-cursor branch has landed we've got a client API to create a raw BufferStream; What would seem most sensible to me is for this to add the ability to attach extra buffer streams to a surface.

mir_subsurface_position(MirSurface* in, MirBufferStream* to_place, int x, int y);
mir_subsurface_stack_above(MirSurface* in, MirBufferStream* to_raise, MirBufferStream* below);
mir_subsurface_stack_below(MirSurface* in, MirBufferStream* to_lower, MirBufferStream* above);
... etc

Here we never need to reject a (well-formed) subsurface request; as far as the shell is concerned there is only one surface, just with funky content.

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

> Specifying top-left locations of surfaces seems like a simplistic layout rule.
> We should consider the existing solution domain of layout managers and, at
> least /consider/, allowing the specification of anchors, insets, fills,
> weights, etc.

That's going way too far down the path of making Mir a full-blown toolkit. Making Mir a toolkit with enforced semantics just makes it less welcoming to supporting actual toolkits with their own semantics.

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

If you want a good example of what level of complexity to target this functionality at, have a look at:
  http://tronche.com/gui/x/xlib/window/stacking-order.html

That's _not_ a suggestion for a design, but an example of the level of detail we should be considering.

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

> > I think this only addresses a part of a range of problems and would like to
> be
> > sure that we have a consistent solution. As Daniel mentions, the stacking
> > order of surfaces should be part of the problem domain here (at least to the
> > extent that a client can e.g. "raise" some of its surfaces). We should also
> > allow a client to shift the focus (or default focus) to a specific surface.
>
> So the intent here is to let the client specify its own surface ordering,
> we're making no guarantees about what the final z-order or position the
> surface will be presented in on-screen. Its more that we're guaranteeing the
> client that their surfaces will be composited in a specific way amongst
> themselves.

I think the use case of "raise these surfaces and direct input towards this one" needs more explicit support than having to list all surfaces.

> > As proposed, this is making requests for a specific action without
> sufficient
> > semantic content for the server to implement good alternative behavior if
> the
> > request can't be met in full: if a window encounters an edge to the display
> > should it overlap, resize or have an adjusted position?
>
> I don't know that we want to reinterpret what the client has said it wants to
> do... If it violates our window policies, we'll reject the request. (or
> perhaps I've misunderstood the point)

The general approach we've taken with other requests is that the client makes a request and the server decided what to do and (mostly) tells the client that something happened.

But in the case of positioning the client doesn't know what it has to work with:

  o if a surface has a "snapped" border that limits its positioning.
  o The client is not informed when surfaces are moved by the user.
  o The client usually has no indication which screen(s) its surfaces are on.
  o if the WM is doing tiling there's no information on current tile size.

So, unless we provide more information to the client it can't always determine the perfect layout. Allowing the server the flexibility to make a "best effort" is consistent with other APIs.

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

Yay, the rfc is working (and has generated comments)!

With the client api structuring... (MirConnections have MirSurfaces, etc)
We currently have MirConnections have multiple MirSurfaces, and MirConnections have a single MirBufferStream. I didn't quite see why MirConnections with multiple MirSurfaces, each with multiple BufferStreams is better than keeping the structure that we have.

I think we're all pointing out that some sort of batching of position updates is needed if the client is to request arrangements of its own surfaces. The start of the batching work in Daniel's rename-api branch seems like a sensible starting point. The other suggestion seems to be the one in this original rfc (which seems to underpowered), and the suggestion in the wayland subsurface protocol of having a 'synced' and 'unsynced' surface. I don't know that we need swaps to batch with positioning updates, and I also don't know that we need a 'desynchronized mode'

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

> I think the use case of "raise these surfaces and direct input towards this
> one" needs more explicit support than having to list all surfaces.

Fair enough, and I think the rename-api detaches the user from having to list all the surfaces, and it also avoids the wart of "if you don't specify what to do with a surface, the server can do whatever it wants", as the other surfaces wo,uld just keep their state.

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

> But in the case of positioning the client doesn't know what it has to work
> with:
>
> o if a surface has a "snapped" border that limits its positioning.
> o The client is not informed when surfaces are moved by the user.
> o The client usually has no indication which screen(s) its surfaces are on.
> o if the WM is doing tiling there's no information on current tile size.

It is somewhat the goal to keep the client from knowing what its absolute final position though, and to mesh the preference the client gives us in terms of the coordinates of 'the clients plane' with the other policies that the window manager wants.

Like, if the client says "surface A at 100, 100, surface B at 200,100" the server won't honor that if it violates some of the rules, or if the user goes and moves the surface otherwise. Maybe the client needs a way to know if its request is being honored (or maybe how its being honored) via the event system so it can sort out what it wants to do.

Revision history for this message
Chris Halse Rogers (raof) wrote :

> Yay, the rfc is working (and has generated comments)!
>
> With the client api structuring... (MirConnections have MirSurfaces, etc)
> We currently have MirConnections have multiple MirSurfaces, and MirConnections
> have a single MirBufferStream. I didn't quite see why MirConnections with
> multiple MirSurfaces, each with multiple BufferStreams is better than keeping
> the structure that we have.

I think the idea of letting clients do relative positioning of their own surfaces is a poor one - Alan's raised all sorts of issues with it, and to that I'll add “what happens when a user moves one of the surfaces”?

The MirConnection has zero or more MirSurfaces which have one or more MirBufferStreams arrangement resolves this by providing a layer at which the client is in total control. The shell sees and acts on MirConnections and MirSurfaces, and Mir handles compositing (or not) the MirBufferStreams internally. In this case there's never any need to reject a MirBufferStream request (as long as all the objects it references exist and are valid, of course).

> I think we're all pointing out that some sort of batching of position updates
> is needed if the client is to request arrangements of its own surfaces. The
> start of the batching work in Daniel's rename-api branch seems like a sensible
> starting point. The other suggestion seems to be the one in this original rfc
> (which seems to underpowered), and the suggestion in the wayland subsurface
> protocol of having a 'synced' and 'unsynced' surface. I don't know that we
> need swaps to batch with positioning updates,

The obvious case where you need swaps to batch with positioning updates is when you're not drawing anything behind the overlaying surface. When you move the overlaying surface you need the underlay to be updated on the same frame as the move, otherwise you see bits of the unrendered area.

> and I also don't know that we need a 'desynchronized mode'

We don't, really. It's basically a client-courtesy-optimisation, where if you *don't* need frame-accurate moving etc you don't have to continually swap buffers on your main surface.

Revision history for this message
Robert Carr (robertcarr) wrote :

>> I think the idea of letting clients do relative positioning of their own surfaces is a poor >> one - Alan's raised all sorts of issues with it, and to that I'll add “what happens when a >> user moves one of the surfaces”?

>> The MirConnection has zero or more MirSurfaces which have one or more MirBufferStreams
>> arrangement resolves this by providing a layer at which the client is in total control. The >> shell sees and acts on MirConnections and MirSurfaces, and Mir handles compositing (or not) >> the MirBufferStreams internally.

+1

Revision history for this message
Chris Halse Rogers (raof) wrote :

54 +/** Specify the position of the stream within the surface. This establishes
55 + * positions relative to the other streams in the surface, and is not a
56 + * guarantee of the final composited ons

You appear to have garbled a comment here ☺. I'm not sure if what the comment says is right; we *should* be able to say that the relative position is guaranteed to be correct, but we guarantee nothing about the position of (0,0) onscreen.

This does raise the question of whether or not we want to clip streams to the parent surface; Wayland doesn't for subsurfaces, and I think GTK uses this for tooltips and such. That said, we know how to position tooltips and such better than the toolkit, so they shouldn't really be using it for this purpose.

I don't know if we should clip to the parent window or not; whichever way we go, we should document and test it, though.

This otherwise looks like sensible API to me. I do think we'll need at least an optional synchronise-on-swap_buffers mode, though.

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 2015-03-31 02:35:42 +0000
3+++ include/client/mir_toolkit/mir_surface.h 2015-04-01 19:59:57 +0000
4@@ -577,10 +577,88 @@
5 * Change the title (name) of a surface.
6 * \param [in] surface The surface to rename
7 * \param [in] name The new name
8- * \returns When the change has completed
9+ * \return When the change has completed
10 */
11 MirWaitHandle* mir_surface_set_title(MirSurface* surf, char const* name);
12
13+
14+/** generate a batch of changes to apply to the surface via
15+ * mir_surface_spec_commit_changes. The returned spec must be released with
16+ * mir_surface_spec_release;
17+ * \param [in] surface The surface the changes will take effect on
18+ * \return A wait handle that can be passed to mir_wait_for
19+ */
20+MirSurfaceSpec* mir_surface_begin_changes(MirSurface* surface);
21+
22+/** Commit the changes accummulated in the MirSurfaceSpect to the surface
23+ * \param [in] spec The spec to apply
24+ * \return A wait handle that is
25+ */
26+MirWaitHandle* mir_surface_spec_commit_changes(MirSurfaceSpec* spec);
27+
28+/** rearrange the ordering of the buffer streams in the connection, placing
29+ * stream_to_place above stream_below. If stream_to_place is not associated
30+ * the surface, it will become associated with the surface. If it is already
31+ * in the ordering of the surface, it will be removed from its current position
32+ * and placed in the requested position.
33+ *
34+ * Associated buffer streams are not owned by the surface and still must be
35+ * released.
36+ *
37+ * \param [in] spec The spec to accumulate the request in
38+ * \param [in] stream_to_place The stream to place
39+ * \param [in] reference_stream The stream which will be above stream_to_place
40+ */
41+void mir_surface_spec_place_buffer_stream_below(
42+ MirSurfaceSpec* spec, MirBufferStream* stream_to_place, MirBufferStream* reference_stream);
43+
44+/** Same as mir_surface_spec_place_buffer_stream_below, except placing stream_to_place
45+ * below reference_stream.
46+ *
47+ * \param [in] spec The spec to accumulate the request in
48+ * \param [in] stream_to_place The stream to place
49+ * \param [in] reference_stream The stream which will be below stream_to_place
50+ */
51+void mir_surface_spec_place_buffer_stream_above(
52+ MirSurfaceSpec* spec, MirBufferStream* stream_to_place, MirBufferStream* reference_stream);
53+
54+/** Specify the position of the stream within the surface. This establishes
55+ * positions relative to the other streams in the surface, and is not a
56+ * guarantee of the final composited ons
57+ *
58+ * If stream_to_place is not associated the surface yet, it will become
59+ * associated with the surface at the given position.
60+ *
61+ * Associated buffer streams are not owned by the surface and still must be
62+ * released.
63+ *
64+ * \param [in] spec The spec to accumulate the request in
65+ * \param [in] stream_to_move The stream to movee
66+ * \param [in] x The x coordinate of the stream
67+ * \param [in] y The x coordinate of the stream
68+ */
69+void mir_surface_spec_place_buffer_stream_position(
70+ MirSurfaceSpec* spec, MirBufferStream* stream_to_move, int x, int y);
71+
72+/* Returns the number of streams currently associated with surface
73+ * \param [in] surface A surface
74+ * \return The number of streams associated with surface
75+ */
76+unsigned int mir_surface_get_number_of_streams(MirSurface* surface);
77+
78+/* Query the streams associated with this surface and their positions.
79+ * streams and positions must have the size of num_streams.
80+ * streams and positions are filled in their z-order. (with is streams[0]
81+ * being the bottom-most surface)
82+ *
83+ * \param [in] surface The surface to query
84+ * \param [out] streams An array of MirBufferStream* that is of size num_streams
85+ * \param [out] positions An array of MirRectangles that is of size num_streams
86+ * \param [in] num_streams The size of both positions and streams array
87+ */
88+void mir_surface_get_streams(
89+ MirSurface* surface, MirBufferStream** streams, MirRectangle* positions, unsigned int num_streams);
90+
91 #ifdef __cplusplus
92 }
93 /**@}*/
94
95=== modified file 'include/server/mir/compositor/scene_element.h'
96--- include/server/mir/compositor/scene_element.h 2015-03-31 02:35:42 +0000
97+++ include/server/mir/compositor/scene_element.h 2015-04-01 19:59:57 +0000
98@@ -19,7 +19,6 @@
99 #ifndef MIR_COMPOSITOR_SCENE_ELEMENT_H_
100 #define MIR_COMPOSITOR_SCENE_ELEMENT_H_
101
102-#include "mir/compositor/decoration.h"
103 #include <memory>
104
105 namespace mir
106@@ -30,7 +29,7 @@
107 }
108 namespace compositor
109 {
110-
111+class Decoration;
112 class SceneElement
113 {
114 public:
115@@ -40,11 +39,9 @@
116 virtual void rendered() = 0;
117 virtual void occluded() = 0;
118
119- virtual Decoration const& decoration() const
120- {
121- static const Decoration none;
122- return none;
123- }
124+ //TODO: Decoration is opaque on purpose. It is only used by an internal example,
125+ // and this function should be removed from the public API.
126+ virtual std::unique_ptr<Decoration> decoration() const = 0;
127
128 protected:
129 SceneElement() = default;
130
131=== modified file 'playground/demo-shell/demo_compositor.cpp'
132--- playground/demo-shell/demo_compositor.cpp 2015-03-31 02:35:42 +0000
133+++ playground/demo-shell/demo_compositor.cpp 2015-04-01 19:59:57 +0000
134@@ -83,8 +83,8 @@
135 auto embellished = renderer.would_embellish(*renderable, viewport);
136 auto any_part_drawn = (viewport.overlaps(renderable->screen_position()) || embellished);
137
138- if (auto const& decor = it->decoration())
139- decorated[renderable->id()] = decor;
140+ if (auto decor = it->decoration())
141+ decorated[renderable->id()] = std::move(decor);
142 if (any_part_drawn)
143 {
144 renderable_list.push_back(renderable);
145
146=== modified file 'playground/demo-shell/demo_renderer.cpp'
147--- playground/demo-shell/demo_renderer.cpp 2015-03-31 02:35:42 +0000
148+++ playground/demo-shell/demo_renderer.cpp 2015-04-01 19:59:57 +0000
149@@ -221,7 +221,7 @@
150
151 void DemoRenderer::begin(DecorMap&& d) const
152 {
153- decor_map = d;
154+ decor_map = std::move(d);
155 title_cache.drop_unused();
156 title_cache.mark_all_unused();
157 }
158@@ -236,7 +236,7 @@
159 auto& decor = d->second;
160 tessellate_shadow(primitives, renderable, shadow_radius);
161 tessellate_frame(primitives, renderable, titlebar_height,
162- decor.name.c_str());
163+ decor->name.c_str());
164 }
165 }
166
167
168=== modified file 'playground/demo-shell/demo_renderer.h'
169--- playground/demo-shell/demo_renderer.h 2015-03-31 02:35:42 +0000
170+++ playground/demo-shell/demo_renderer.h 2015-04-01 19:59:57 +0000
171@@ -39,7 +39,7 @@
172 };
173
174 typedef std::unordered_map<graphics::Renderable::ID,
175- compositor::Decoration> DecorMap;
176+ std::unique_ptr<compositor::Decoration>> DecorMap;
177
178 class DemoRenderer : public compositor::GLRenderer
179 {
180
181=== modified file 'src/client/mir_surface_api.cpp'
182--- src/client/mir_surface_api.cpp 2015-03-31 02:35:42 +0000
183+++ src/client/mir_surface_api.cpp 2015-04-01 19:59:57 +0000
184@@ -580,8 +580,6 @@
185 return nullptr;
186 }
187
188-namespace { // Private for now. TODO: Finalize and publish later (LP: #1422522)
189-
190 MirSurfaceSpec* mir_surface_begin_changes(MirSurface* surf)
191 {
192 mir::require(mir_surface_is_valid(surf));
193@@ -608,8 +606,6 @@
194 return surface->modify(*spec);
195 }
196
197-} // Private namespace. TODO: finalize morphing API and publish.
198-
199 MirWaitHandle* mir_surface_set_title(MirSurface* surf, char const* name)
200 {
201 MirWaitHandle* result = nullptr;
202@@ -621,3 +617,34 @@
203 }
204 return result;
205 }
206+
207+void mir_surface_spec_place_buffer_stream_below(
208+ MirSurfaceSpec* spec, MirBufferStream* stream_to_place, MirBufferStream* reference_stream)
209+{
210+ (void) spec;
211+ (void) stream_to_place;
212+ (void) reference_stream;
213+}
214+
215+void mir_surface_spec_place_buffer_stream_position(
216+ MirSurfaceSpec* spec, MirBufferStream* stream_to_move, int x, int y)
217+{
218+ (void) spec;
219+ (void) stream_to_move;
220+ (void) x;
221+ (void) y;
222+}
223+
224+unsigned int mir_surface_get_number_of_streams(MirSurface*)
225+{
226+ return 1;
227+}
228+
229+void mir_surface_get_streams(
230+ MirSurface* surface, MirBufferStream** streams, MirRectangle* positions, unsigned int num_streams)
231+{
232+ (void) surface;
233+ (void) streams;
234+ (void) positions;
235+ (void) num_streams;
236+}
237
238=== renamed file 'include/server/mir/compositor/decoration.h' => 'src/include/server/mir/compositor/decoration.h'
239=== modified file 'src/server/scene/surface_stack.cpp'
240--- src/server/scene/surface_stack.cpp 2015-03-31 02:35:42 +0000
241+++ src/server/scene/surface_stack.cpp 2015-04-01 19:59:57 +0000
242@@ -23,6 +23,7 @@
243 #include "mir/scene/surface.h"
244 #include "mir/scene/scene_report.h"
245 #include "mir/compositor/scene_element.h"
246+#include "mir/compositor/decoration.h"
247 #include "mir/graphics/renderable.h"
248
249 #include <boost/throw_exception.hpp>
250@@ -52,7 +53,7 @@
251 : renderable_{surface->compositor_snapshot(id)},
252 tracker{tracker},
253 cid{id},
254- decor(mc::Decoration::Type::surface, surface->name())
255+ surface_name(surface->name())
256 {
257 }
258
259@@ -71,16 +72,16 @@
260 tracker->occluded_in(cid);
261 }
262
263- mc::Decoration const& decoration() const override
264+ std::unique_ptr<mc::Decoration> decoration() const override
265 {
266- return decor;
267+ return std::make_unique<mc::Decoration>(mc::Decoration::Type::surface, surface_name);
268 }
269
270 private:
271 std::shared_ptr<mg::Renderable> const renderable_;
272 std::shared_ptr<ms::RenderingTracker> const tracker;
273 mc::CompositorID cid;
274- mc::Decoration const decor;
275+ std::string const surface_name;
276 };
277
278 //note: something different than a 2D/HWC overlay
279@@ -106,6 +107,11 @@
280 {
281 }
282
283+ std::unique_ptr<mc::Decoration> decoration() const override
284+ {
285+ return std::make_unique<mc::Decoration>();
286+ }
287+
288 private:
289 std::shared_ptr<mg::Renderable> const renderable_;
290 };
291
292=== modified file 'tests/acceptance-tests/CMakeLists.txt'
293--- tests/acceptance-tests/CMakeLists.txt 2015-03-31 02:35:42 +0000
294+++ tests/acceptance-tests/CMakeLists.txt 2015-04-01 19:59:57 +0000
295@@ -30,6 +30,7 @@
296 test_prompt_session_client_api.cpp
297 test_client_screencast.cpp
298 test_client_surface_visibility.cpp
299+ test_client_surface_arrangement.cpp
300 test_client_with_custom_display_config_deadlock.cpp
301 test_server_without_active_outputs.cpp
302 test_server_startup.cpp
303@@ -39,6 +40,7 @@
304 test_input_device_hub.cpp
305 test_latency.cpp
306 test_shell_control_of_surface_configuration.cpp
307+ test_render_override.cpp
308 )
309
310 if (MIR_TEST_PLATFORM STREQUAL "mesa")
311
312=== added file 'tests/acceptance-tests/test_client_surface_arrangement.cpp'
313--- tests/acceptance-tests/test_client_surface_arrangement.cpp 1970-01-01 00:00:00 +0000
314+++ tests/acceptance-tests/test_client_surface_arrangement.cpp 2015-04-01 19:59:57 +0000
315@@ -0,0 +1,255 @@
316+/*
317+ * Copyright © 2015 Canonical Ltd.
318+ *
319+ * This program is free software: you can redistribute it and/or modify it
320+ * under the terms of the GNU General Public License version 3,
321+ * as published by the Free Software Foundation.
322+ *
323+ * This program is distributed in the hope that it will be useful,
324+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
325+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
326+ * GNU General Public License for more details.
327+ *
328+ * You should have received a copy of the GNU General Public License
329+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
330+ *
331+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
332+ */
333+
334+#include "mir_toolkit/mir_client_library.h"
335+#include "mir_test_framework/connected_client_with_a_surface.h"
336+#include "mir/compositor/display_buffer_compositor.h"
337+#include "mir/compositor/display_buffer_compositor_factory.h"
338+#include "mir/compositor/scene_element.h"
339+#include "mir/graphics/renderable.h"
340+
341+#include <mutex>
342+#include <condition_variable>
343+#include <stdexcept>
344+#include <gtest/gtest.h>
345+#include <gmock/gmock.h>
346+
347+namespace mtf = mir_test_framework;
348+namespace geom = mir::geometry;
349+namespace mc = mir::compositor;
350+namespace mg = mir::graphics;
351+namespace
352+{
353+
354+MirPixelFormat an_available_format(MirConnection* connection)
355+{
356+ using namespace testing;
357+ MirPixelFormat format{mir_pixel_format_invalid};
358+ unsigned int valid_formats{0};
359+ mir_connection_get_available_surface_formats(connection, &format, 1, &valid_formats);
360+ return format;
361+}
362+
363+struct Stream
364+{
365+ Stream(MirBufferStream* stream, geom::Point pt) :
366+ stream(stream),
367+ pos{pt},
368+ needs_release{false}
369+ {
370+ }
371+
372+ Stream(MirConnection* connection, geom::Rectangle rect) :
373+ stream(mir_connection_create_buffer_stream_sync(
374+ connection,
375+ rect.size.width.as_int(),
376+ rect.size.height.as_int(),
377+ an_available_format(connection),
378+ mir_buffer_usage_hardware)),
379+ pos{rect.top_left},
380+ needs_release{true}
381+ {
382+ mir_buffer_stream_swap_buffers_sync(stream);
383+ }
384+
385+ ~Stream()
386+ {
387+ if (needs_release)
388+ mir_buffer_stream_release_sync(stream);
389+ }
390+
391+ operator MirBufferStream*() const
392+ {
393+ return stream;
394+ }
395+
396+ geom::Point position()
397+ {
398+ return pos;
399+ }
400+
401+ Stream(Stream const&) = delete;
402+ Stream& operator=(Stream const&) = delete;
403+private:
404+ MirBufferStream* stream;
405+ geom::Point const pos;
406+ bool const needs_release;
407+};
408+
409+struct Ordering
410+{
411+ void note_scene_element_sequence(mc::SceneElementSequence const& sequence)
412+ {
413+ std::unique_lock<decltype(mutex)> lk(mutex);
414+ rectangles.clear();
415+ for(auto const& element : sequence)
416+ rectangles.emplace_back(element->renderable()->screen_position());
417+ post_count++;
418+ cv.notify_all();
419+ }
420+
421+ template<typename T, typename S>
422+ bool wait_for_another_post_within(std::chrono::duration<T,S> duration)
423+ {
424+ std::unique_lock<decltype(mutex)> lk(mutex);
425+ auto count = post_count + 1;
426+ return cv.wait_for(lk, duration, [this, count]{return (post_count >= count);});
427+ }
428+
429+ bool ensure_last_ordering_is_consistent_with(
430+ std::vector<MirRectangle> const& arrangement)
431+ {
432+ if (rectangles.size() != arrangement.size())
433+ return false;
434+ for(auto i = 0u; i < rectangles.size(); i++)
435+ {
436+ if ((rectangles[i].top_left.x.as_int() != arrangement[i].left) ||
437+ (rectangles[i].top_left.y.as_int() != arrangement[i].top))
438+ return false;
439+ }
440+ return true;
441+ }
442+private:
443+ std::mutex mutex;
444+ std::condition_variable cv;
445+ unsigned int post_count{0};
446+ std::vector<geom::Rectangle> rectangles;
447+};
448+
449+struct OrderTrackingDBC : mc::DisplayBufferCompositor
450+{
451+ OrderTrackingDBC(
452+ std::shared_ptr<mc::DisplayBufferCompositor> const& wrapped,
453+ std::shared_ptr<Ordering> const& ordering) :
454+ wrapped(wrapped),
455+ ordering(ordering)
456+ {
457+ }
458+
459+ void composite(mc::SceneElementSequence&& scene_sequence) override
460+ {
461+ ordering->note_scene_element_sequence(scene_sequence);
462+ wrapped->composite(std::move(scene_sequence));
463+ }
464+
465+ std::shared_ptr<mc::DisplayBufferCompositor> const wrapped;
466+ std::shared_ptr<Ordering> const ordering;
467+};
468+
469+struct OrderTrackingDBCFactory : mc::DisplayBufferCompositorFactory
470+{
471+ OrderTrackingDBCFactory(
472+ std::shared_ptr<mc::DisplayBufferCompositorFactory> const& wrapped,
473+ std::shared_ptr<Ordering> const& ordering) :
474+ wrapped(wrapped),
475+ ordering(ordering)
476+ {
477+ }
478+
479+ std::unique_ptr<mc::DisplayBufferCompositor> create_compositor_for(mg::DisplayBuffer& db) override
480+ {
481+ return std::make_unique<OrderTrackingDBC>(wrapped->create_compositor_for(db), ordering);
482+ }
483+
484+ std::shared_ptr<OrderTrackingDBC> last_dbc{nullptr};
485+ std::shared_ptr<mc::DisplayBufferCompositorFactory> const wrapped;
486+ std::shared_ptr<Ordering> const ordering;
487+};
488+
489+struct MirSurfaceArrangements : mtf::ConnectedClientWithASurface
490+{
491+ void SetUp() override
492+ {
493+ ordering = std::make_shared<Ordering>();
494+ server.wrap_display_buffer_compositor_factory(
495+ [this](std::shared_ptr<mc::DisplayBufferCompositorFactory> const& wrapped)
496+ {
497+ order_tracker = std::make_shared<OrderTrackingDBCFactory>(wrapped, ordering);
498+ return order_tracker;
499+ });
500+
501+ ConnectedClientWithASurface::SetUp();
502+
503+ primary_stream = std::unique_ptr<Stream>(
504+ new Stream(mir_surface_get_buffer_stream(surface), geom::Point{0,0}));
505+
506+ int const additional_streams{3};
507+ for(auto i = 0; i < additional_streams; i++)
508+ {
509+ geom::Size size{30 * i + 1, 40* i + 1};
510+ geom::Point position{i * 2, i * 3};
511+ streams.emplace_back(std::unique_ptr<Stream>(
512+ new Stream(connection, geom::Rectangle{position, size})));
513+ }
514+ }
515+
516+ void TearDown() override
517+ {
518+ streams.clear();
519+ ConnectedClientWithASurface::TearDown();
520+ }
521+
522+ std::shared_ptr<Ordering> ordering;
523+ std::shared_ptr<OrderTrackingDBCFactory> order_tracker{nullptr};
524+ std::unique_ptr<Stream> primary_stream;
525+ std::vector<std::unique_ptr<Stream>> streams;
526+};
527+}
528+
529+TEST_F(MirSurfaceArrangements, arrangements_are_applied)
530+{
531+ using namespace testing;
532+ MirBufferStream* above_stream = *primary_stream;
533+ auto change_spec = mir_surface_begin_changes(surface);
534+ for(auto& stream : streams)
535+ {
536+ mir_surface_spec_place_buffer_stream_below(change_spec, *stream, above_stream);
537+ mir_surface_spec_place_buffer_stream_position(
538+ change_spec, *stream, stream->position().x.as_int(), stream->position().y.as_int());
539+ above_stream = *stream;
540+ }
541+ mir_wait_for(mir_surface_spec_commit_changes(change_spec));
542+ mir_surface_spec_release(change_spec);
543+
544+ unsigned int num_streams = mir_surface_get_number_of_streams(surface);
545+ ASSERT_THAT(num_streams, Eq(streams.size() + 1));
546+ std::vector<MirBufferStream*> buffer_streams{num_streams};
547+ std::vector<MirRectangle> positions{num_streams};
548+ mir_surface_get_streams(surface, buffer_streams.data(), positions.data(), num_streams);
549+ for(auto i = 0u; i < num_streams; ++i)
550+ {
551+ geom::Point pt{positions[i].top, positions[i].left};
552+ if (i == 0u)
553+ {
554+ EXPECT_THAT(buffer_streams[i], Eq(static_cast<MirBufferStream*>(*primary_stream)));
555+ EXPECT_THAT(pt, Eq(primary_stream->position()));
556+ }
557+ else
558+ {
559+ EXPECT_THAT(buffer_streams[i], Eq(static_cast<MirBufferStream*>(*streams[i-1])));
560+ EXPECT_THAT(pt, Eq(streams[i-1]->position()));
561+ }
562+ }
563+
564+ //check that the compositor rendered correctly
565+ using namespace std::literals::chrono_literals;
566+ EXPECT_TRUE(ordering->wait_for_another_post_within(1s))
567+ << "timed out waiting for another post";
568+ EXPECT_TRUE(ordering->ensure_last_ordering_is_consistent_with(positions))
569+ << "surface ordering was not consistent with the client request";
570+}
571
572=== added file 'tests/acceptance-tests/test_render_override.cpp'
573--- tests/acceptance-tests/test_render_override.cpp 1970-01-01 00:00:00 +0000
574+++ tests/acceptance-tests/test_render_override.cpp 2015-04-01 19:59:57 +0000
575@@ -0,0 +1,130 @@
576+/*
577+ * Copyright © 2015 Canonical Ltd.
578+ *
579+ * This program is free software: you can redistribute it and/or modify it
580+ * under the terms of the GNU General Public License version 3,
581+ * as published by the Free Software Foundation.
582+ *
583+ * This program is distributed in the hope that it will be useful,
584+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
585+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
586+ * GNU General Public License for more details.
587+ *
588+ * You should have received a copy of the GNU General Public License
589+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
590+ *
591+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
592+ */
593+
594+#include "mir_toolkit/mir_client_library.h"
595+#include "mir_test_framework/connected_client_with_a_surface.h"
596+#include "mir/compositor/display_buffer_compositor.h"
597+#include "mir/compositor/display_buffer_compositor_factory.h"
598+#include "mir/compositor/scene_element.h"
599+#include "mir/compositor/compositor.h"
600+#include "mir/graphics/renderable.h"
601+
602+#include <mutex>
603+#include <condition_variable>
604+#include <gtest/gtest.h>
605+#include <gmock/gmock.h>
606+
607+namespace mtf = mir_test_framework;
608+namespace geom = mir::geometry;
609+namespace mc = mir::compositor;
610+namespace mg = mir::graphics;
611+namespace
612+{
613+struct size_less
614+{
615+ bool operator() (geom::Size const& a, geom::Size const& b)
616+ {
617+ return a.width < b.width && a.height < b.height;
618+ }
619+};
620+
621+struct CompositionTracker
622+{
623+ bool wait_until_surface_is_rendered_with_size(geom::Size sz)
624+ {
625+ using namespace std::literals::chrono_literals;
626+ std::unique_lock<decltype(mutex)> lk(mutex);
627+ return cv.wait_for(lk, 4s, [this, sz] {
628+ return std::find(rendered_sizes.begin(), rendered_sizes.end(), sz) != rendered_sizes.end();
629+ });
630+ }
631+
632+ void surface_rendered_with_size(geom::Size sz)
633+ {
634+ std::unique_lock<decltype(mutex)> lk(mutex);
635+ rendered_sizes.emplace(sz);
636+ cv.notify_all();
637+ }
638+
639+private:
640+ std::set<geom::Size, size_less> rendered_sizes;
641+ std::mutex mutex;
642+ std::condition_variable cv;
643+};
644+
645+struct CustomDBC : mc::DisplayBufferCompositor
646+{
647+ CustomDBC(std::shared_ptr<CompositionTracker> const& tracker) :
648+ tracker(tracker)
649+ {
650+ }
651+
652+ void composite(mc::SceneElementSequence&& sequence) override
653+ {
654+ for(auto const& element : sequence)
655+ {
656+ auto renderable = element->renderable();
657+ renderable->buffer(); //consume buffer to stop compositor from spinning
658+ tracker->surface_rendered_with_size(renderable->screen_position().size);
659+ }
660+ }
661+private:
662+ std::shared_ptr<CompositionTracker> const tracker;
663+};
664+
665+struct CustomDBCFactory : mc::DisplayBufferCompositorFactory
666+{
667+ CustomDBCFactory(std::shared_ptr<CompositionTracker> const& tracker) :
668+ tracker(tracker)
669+ {
670+ }
671+
672+ std::unique_ptr<mc::DisplayBufferCompositor> create_compositor_for(mg::DisplayBuffer&) override
673+ {
674+ return std::make_unique<CustomDBC>(tracker);
675+ }
676+private:
677+ std::shared_ptr<CompositionTracker> const tracker;
678+};
679+
680+struct DisplayBufferCompositorOverride : mtf::ConnectedClientWithASurface
681+{
682+ void SetUp() override
683+ {
684+ using namespace std::literals::chrono_literals;
685+ server.override_the_display_buffer_compositor_factory([this]
686+ {
687+ return std::make_shared<CustomDBCFactory>(tracker);
688+ });
689+
690+ mtf::ConnectedClientWithASurface::SetUp();
691+ }
692+ void TearDown() override
693+ {
694+ mtf::ConnectedClientWithASurface::TearDown();
695+ }
696+protected:
697+ std::shared_ptr<CompositionTracker> const tracker{std::make_shared<CompositionTracker>()};
698+};
699+}
700+
701+TEST_F(DisplayBufferCompositorOverride, composite_called_with_surface)
702+{
703+ mir_buffer_stream_swap_buffers_sync(mir_surface_get_buffer_stream(surface));
704+ EXPECT_TRUE(tracker->wait_until_surface_is_rendered_with_size({surface_params.width, surface_params.height}));
705+}
706
707=== modified file 'tests/include/mir_test_doubles/stub_scene_element.h'
708--- tests/include/mir_test_doubles/stub_scene_element.h 2015-03-31 02:35:42 +0000
709+++ tests/include/mir_test_doubles/stub_scene_element.h 2015-04-01 19:59:57 +0000
710@@ -20,6 +20,7 @@
711 #define MIR_TEST_DOUBLES_STUB_SCENE_ELEMENT_H_
712
713 #include "mir/compositor/scene_element.h"
714+#include "mir/compositor/decoration.h"
715 #include "stub_renderable.h"
716
717 namespace mir
718@@ -55,6 +56,11 @@
719 {
720 }
721
722+ std::unique_ptr<compositor::Decoration> decoration() const override
723+ {
724+ return nullptr;
725+ }
726+
727 private:
728 std::shared_ptr<graphics::Renderable> const renderable_;
729 };
730
731=== modified file 'tests/include/mir_test_framework/connected_client_with_a_surface.h'
732--- tests/include/mir_test_framework/connected_client_with_a_surface.h 2014-11-18 16:15:00 +0000
733+++ tests/include/mir_test_framework/connected_client_with_a_surface.h 2015-04-01 19:59:57 +0000
734@@ -26,6 +26,14 @@
735 struct ConnectedClientWithASurface : ConnectedClientHeadlessServer
736 {
737 MirSurface* surface{nullptr};
738+ MirSurfaceParameters const surface_params =
739+ {
740+ "ConnectedClientWithASurfaceFixtureSurface",
741+ 640, 480,
742+ mir_pixel_format_abgr_8888,
743+ mir_buffer_usage_hardware,
744+ mir_display_output_id_invalid
745+ };
746
747 void SetUp() override;
748
749
750=== modified file 'tests/mir_test_framework/connected_client_with_a_surface.cpp'
751--- tests/mir_test_framework/connected_client_with_a_surface.cpp 2014-11-18 16:15:00 +0000
752+++ tests/mir_test_framework/connected_client_with_a_surface.cpp 2015-04-01 19:59:57 +0000
753@@ -24,16 +24,7 @@
754 {
755 ConnectedClientHeadlessServer::SetUp();
756
757- MirSurfaceParameters request_params =
758- {
759- __PRETTY_FUNCTION__,
760- 640, 480,
761- mir_pixel_format_abgr_8888,
762- mir_buffer_usage_hardware,
763- mir_display_output_id_invalid
764- };
765-
766- surface = mir_connection_create_surface_sync(connection, &request_params);
767+ surface = mir_connection_create_surface_sync(connection, &surface_params);
768 ASSERT_TRUE(mir_surface_is_valid(surface));
769 }
770
771
772=== modified file 'tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp'
773--- tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2015-03-31 02:35:42 +0000
774+++ tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2015-04-01 19:59:57 +0000
775@@ -18,12 +18,14 @@
776
777 #include "src/server/compositor/default_display_buffer_compositor.h"
778 #include "mir/compositor/display_buffer_compositor.h"
779+#include "mir/compositor/decoration.h"
780 #include "src/server/report/null_report_factory.h"
781 #include "mir/compositor/scene.h"
782 #include "mir/compositor/renderer.h"
783 #include "mir/geometry/rectangle.h"
784 #include "mir_test_doubles/mock_renderer.h"
785 #include "mir_test/fake_shared.h"
786+#include "mir_test/gmock_fixes.h"
787 #include "mir_test_doubles/mock_display_buffer.h"
788 #include "mir_test_doubles/mock_renderable.h"
789 #include "mir_test_doubles/fake_renderable.h"
790@@ -57,17 +59,22 @@
791 {
792 }
793
794- std::shared_ptr<mir::graphics::Renderable> renderable() const
795+ std::shared_ptr<mir::graphics::Renderable> renderable() const override
796 {
797 return renderable_;
798 }
799
800- void rendered()
801- {
802- }
803-
804- void occluded()
805- {
806+ void rendered() override
807+ {
808+ }
809+
810+ void occluded() override
811+ {
812+ }
813+
814+ std::unique_ptr<mc::Decoration> decoration() const override
815+ {
816+ return nullptr;
817 }
818
819 private:
820@@ -317,7 +324,7 @@
821 }
822
823 MOCK_CONST_METHOD0(renderable, std::shared_ptr<mir::graphics::Renderable>());
824- MOCK_CONST_METHOD0(decoration, mc::Decoration const&());
825+ MOCK_CONST_METHOD0(decoration, std::unique_ptr<mc::Decoration>());
826 MOCK_METHOD0(rendered, void());
827 MOCK_METHOD0(occluded, void());
828 };
829
830=== modified file 'tests/unit-tests/examples/test_demo_compositor.cpp'
831--- tests/unit-tests/examples/test_demo_compositor.cpp 2015-03-31 02:35:42 +0000
832+++ tests/unit-tests/examples/test_demo_compositor.cpp 2015-04-01 19:59:57 +0000
833@@ -30,6 +30,7 @@
834 #include "mir_test_doubles/stub_buffer_stream.h"
835 #include "mir_test_doubles/stub_renderable.h"
836 #include "mir_test/fake_shared.h"
837+#include "mir_test/gmock_fixes.h"
838
839 #include <gtest/gtest.h>
840 #include <gmock/gmock.h>
841@@ -47,7 +48,7 @@
842 struct MockSceneElement : mc::SceneElement
843 {
844 MOCK_CONST_METHOD0(renderable, std::shared_ptr<mir::graphics::Renderable>());
845- MOCK_CONST_METHOD0(decoration, mc::Decoration const&());
846+ MOCK_CONST_METHOD0(decoration, std::unique_ptr<mc::Decoration>());
847 MOCK_METHOD0(rendered, void());
848 MOCK_METHOD0(occluded, void());
849 };
850
851=== modified file 'tests/unit-tests/scene/test_surface_stack.cpp'
852--- tests/unit-tests/scene/test_surface_stack.cpp 2015-03-31 02:35:42 +0000
853+++ tests/unit-tests/scene/test_surface_stack.cpp 2015-04-01 19:59:57 +0000
854@@ -22,6 +22,7 @@
855 #include "mir/scene/observer.h"
856 #include "mir/scene/surface_creation_parameters.h"
857 #include "mir/compositor/scene_element.h"
858+#include "mir/compositor/decoration.h"
859 #include "src/server/report/null_report_factory.h"
860 #include "src/server/scene/basic_surface.h"
861 #include "mir/input/input_channel_factory.h"
862@@ -251,10 +252,10 @@
863
864 auto& element = elements.front();
865
866- auto const& decor = element->decoration();
867- EXPECT_TRUE(decor);
868- EXPECT_EQ(mc::Decoration::Type::surface, decor.type);
869- EXPECT_EQ("Mary had a little lamb", decor.name);
870+ auto decor = element->decoration();
871+ ASSERT_THAT(decor, Ne(nullptr));
872+ EXPECT_EQ(mc::Decoration::Type::surface, decor->type);
873+ EXPECT_EQ("Mary had a little lamb", decor->name);
874 }
875
876 TEST_F(SurfaceStack, gets_surface_renames)
877@@ -286,10 +287,10 @@
878
879 auto& element = elements.front();
880
881- auto const& decor = element->decoration();
882- EXPECT_TRUE(decor);
883- EXPECT_EQ(mc::Decoration::Type::surface, decor.type);
884- EXPECT_EQ("username@hostname: ~/Documents", decor.name);
885+ auto decor = element->decoration();
886+ ASSERT_THAT(decor, Ne(nullptr));
887+ EXPECT_EQ(mc::Decoration::Type::surface, decor->type);
888+ EXPECT_EQ("username@hostname: ~/Documents", decor->name);
889 }
890
891 TEST_F(SurfaceStack, scene_counts_pending_accurately)

Subscribers

People subscribed via source and target branches