Merge lp:~kdub/mir/RFC-surface-arrangements into lp:mir
- RFC-surface-arrangements
- Merge into development-branch
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 |
Related bugs: |
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.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2436
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
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:/
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.
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.
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.
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)
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 SceneElementSeq
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/
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 DefaultDisplayB
Still the idea of forwarding surfaces could be interesting too.
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.
Andreas Pokorny (andreas-pokorny) wrote : | # |
Back to the nested bypass: On the client maybe:
MirBufferStream
and respective setters, and a mir_surface_
Chris Halse Rogers (raof) wrote : | # |
So, relevant prior art here is:
http://
The API contains the three obvious things - set_position, place_above, place_below - but also the non-obvious but important set_sync/
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_
mir_subsurface_
mir_subsurface_
... 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.
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.
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://
That's _not_ a suggestion for a design, but an example of the level of detail we should be considering.
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.
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'
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.
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.
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-
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
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-
Preview Diff
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) |
Just an RFC on an unimplemented client api change. Resubmit after implementation.