Mir

Merge lp:~mir-team/mir/improve-attribute-protocol-and-normalize-getters into lp:mir

  1. improve-attribute-protocol-and-normalize-getters
  2. Merge into development-branch
Proposed by Robert Carr
Status: Merged
Approved by: Alexandros Frantzis
Approved revision: no longer in the source branch.
Merged at revision: 1778
Proposed branch: lp:~mir-team/mir/improve-attribute-protocol-and-normalize-getters
Merge into: lp:mir
Diff against target: 933 lines (+383/-171)
16 files modified
include/client/mir_toolkit/mir_surface.h (+14/-0)
include/server/mir/frontend/surface.h (+1/-0)
include/shared/mir_toolkit/common.h (+4/-0)
include/test/mir_test_doubles/mock_frontend_surface.h (+1/-0)
include/test/mir_test_doubles/stub_scene_surface.h (+1/-0)
src/client/mir_surface.cpp (+6/-3)
src/client/mir_surface.h (+3/-0)
src/client/mir_surface_api.cpp (+49/-10)
src/server/frontend/session_mediator.cpp (+9/-0)
src/server/scene/basic_surface.cpp (+125/-62)
src/server/scene/basic_surface.h (+9/-8)
src/shared/protobuf/mir_protobuf.proto (+2/-0)
tests/acceptance-tests/test_client_surface_visibility.cpp (+4/-1)
tests/unit-tests/client/test_client_mir_surface.cpp (+38/-26)
tests/unit-tests/scene/test_basic_surface.cpp (+117/-15)
tests/unit-tests/scene/test_surface_impl.cpp (+0/-46)
To merge this branch: bzr merge lp:~mir-team/mir/improve-attribute-protocol-and-normalize-getters
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Alexandros Frantzis (community) Approve
Daniel van Vugt Abstain
Chris Halse Rogers Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+225912@code.launchpad.net

Commit message

Improve surface attribute protocol and add missing getters.

Description of the change

The motivation for this branch begins with attempting to address bug:

https://bugs.launchpad.net/mir/+bug/1336553

Seems simple enough at first but on investigation noticed a few irregularities in the surface attributes:

1. Some like focus, have no getter/persistent state.
2. Some like visibility have no getter.
3. Some like DPI have a getter on the client side, and thus support querying via the use of a "-1" value.
4. Some irregular observer behavior (missing observer calls, irregular behavior when values don't change).
5. Implementing client side getters requires use of a -1 value type trick, and leads to potentially blocking getters.

This branch sets out to clear up some of these abnormalities. You can see a series of tests instantiated for each attribute in test_basic_surface. There are a few small observer fixes, etc as mentioned. In order to make DPI behave normally (i.e. throw on invalid value rather than interpret as query), I've had to change surface attributes to be sent at surface creation time. This way the client library never has to query, it always has a value for each attribute.

Focus and visibility getters are added to the client library.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Just some quick comments:

(1) This should be 0..N:
+ /* Do not specify values...code relies on 1...N ordering. */

(2) The suffix "_state" is unnecessary and potentially confusing with the attribute called "state":
15 +MirSurfaceFocusState mir_surface_get_focus_state(MirSurface *surface);
23 +MirSurfaceVisibility mir_surface_get_visibility_state(MirSurface *surface);

(3) Returning an integer value outside the range of an enum is a violation of the implicit contract that a well designed API will only return values of those enums:
13 + * -1 if the surface is invalid.
21 + * -1 if the surface is invalid.
I think a more sensible fallback for invalid surfaces would be to return mir_surface_unfocused and mir_surface_visibility_occluded.

(4) src/shared/protobuf/mir_protobuf.proto: Protocol ABI change!
Please either avoid protocol changes, or record the change explicitly with a bump of libmirprotobuf0 to libmirprotobuf1. Not ideal.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1741
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2082/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/850/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/856/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/849/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/603/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/601/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2688/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2082/rebuild

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

PASSED: Continuous integration, rev:1741
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2085/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/853
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/859
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/852
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/606
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/606/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/604
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/604/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2700
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2700/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2000
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9435

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2085/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1743
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2089/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/857
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/863
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/856
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/610
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/610/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/608
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/608/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2706
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2706/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2004
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9442

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2089/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1748
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2098/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/867
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/873
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/866
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/619
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/619/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/617
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/617/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2760
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2760/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2010
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9490

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2098/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Yay! Surface attributes populated on construction! I'm broadly in favour of this :)

Your API docs don't match the behaviour; they say you'll return -1 for invalid surfaces.

While we're at it, I thought that, waaaaay back in London, we were going to abort() when passed an invalid surface? I'm very much against silently pretending that everything's OK. Likewise, I cringe at all the

try { <something> }
catch(...)
{
    //la di da! The client doesn't need to know about this...
}

in there (even though much of it is pre-existing).

review: Needs Fixing
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

370 + if (!set_focus_state(static_cast<MirSurfaceFocusState>(value)))
371 + BOOST_THROW_EXCEPTION(std::logic_error("Invalid focus state."));
372 + result = value;
373 break;
374 case mir_surface_attrib_swapinterval:
375 - allow_dropping = (value == 0);
376 - allow_framedropping(allow_dropping);
377 + if (!set_swap_interval(value))
378 + BOOST_THROW_EXCEPTION(std::logic_error("Invalid swapinterval"));
379 result = value;
380 break;
381 case mir_surface_attrib_dpi:
382 - if (value >= 0 && !set_dpi(value)) // Negative means query only
383 + if (!set_dpi(value))
384 BOOST_THROW_EXCEPTION(std::logic_error("Invalid DPI value"));

configure() is the only user of the set_* functions and it doesn't really need the return value. We might as well throw from the set_* functions instead of checking the return value and throwing in configure.

394 + switch (attrib)
395 + {
396 + case mir_surface_attrib_type:
397 + return type_value;
398 + case mir_surface_attrib_state:
399 + return state_value;
...

It seems like we storing the attributes in an array indexed by the attribute instead of separate variables would make life easier for BasicSurface.

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

Refactored the set_ functions

Added attribute array (caught several more missing locks in the meantime).

Removed misleading comment.

Lets do something about client library errors...but not in this branch...was our plan in london written down? I'll see what I can remember over lunch...I don't remember each detail.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1751
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2115/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/889
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/895
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/888
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/636
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/636/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/634
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/634/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2841
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2841/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2031
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9574

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2115/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Re: client errors - in one of those enthusiastic London days we decided that API constructors would never return NULL, but instead return an error object. Because this makes it very easy to write programs that fail in unexpected ways, we further decided that passing an error object into any API call that isn't explicitly a check for error status would die, so that developers get immediate and useful corefile-based feedback.

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

Bah. I was going to suggest std::array<int, > for the attrib array on the basis that operator[] would do nice bounds checking for us, but it doesn't because of silliness.

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

(4) Hmm, the protocol change uses "repeated" which actually appears to be backward compatible, so no ABI bump for libmirprotobuf is required after all;
https://developers.google.com/protocol-buffers/docs/proto#updating

(5) This single-use function doesn't need to exist. If you initialize your variables in the constructor then it's more obvious locking isn't required either...
ms::BasicSurface::initialize_attributes()

(6) I think the TODO comment is still valid and should not be deleted:
395 - /*
396 - * TODO: In future, query the shell implementation for the subset of
397 - * attributes/types it implements.
398 - */

Needs^H^H^H^H^HWants fixing, somewhat. I haven't gone back through in enough detail to approve yet but won't block any more.

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

The protocol change is the sort of thing that we won't be able to get away with when used on the desktop, but because the phone reboots to do any package update it's ok (as long as libmirclient updates in lockstep with libmirserver!)

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Nit:
260 + std::lock_guard<std::mutex> lg(guard);

As mentioned by Daniel, locking is not currently needed.

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

+ MirSurfaceType set_type(MirSurfaceType t); // Use configure() to make public changes
+ MirSurfaceState set_state(MirSurfaceState s);
+ int set_dpi(int);
+ MirSurfaceVisibility set_visibility(MirSurfaceVisibility v);
+ int set_swap_interval(int);
+ MirSurfaceFocusState set_focus_state(MirSurfaceFocusState f);

these look weird to me - why returning the value set? Could a different value be set than what I asked for? I'd be suspicious of it, but it's private so meh.
LGTM otherwise

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

Gerry:
We return the value applied because it might be different to the requested value, in some restricted shells. That's the TODO I was referring to above in (6).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/client/mir_toolkit/mir_surface.h'
--- include/client/mir_toolkit/mir_surface.h 2014-06-19 16:15:42 +0000
+++ include/client/mir_toolkit/mir_surface.h 2014-07-09 21:43:55 +0000
@@ -253,6 +253,20 @@
253 * \return The DPI of the surface, or zero if unknown.253 * \return The DPI of the surface, or zero if unknown.
254 */254 */
255int mir_surface_get_dpi(MirSurface* surface);255int mir_surface_get_dpi(MirSurface* surface);
256
257/**
258 * Query the focus state for a surface.
259 * \param [in] surface The surface to operate on
260 * \return The focus state of said surface
261 */
262MirSurfaceFocusState mir_surface_get_focus(MirSurface *surface);
263
264/**
265 * Query the visibility state for a surface.
266 * \param [in] surface The surface to operate on
267 * \return The visibility state of said surface
268 */
269MirSurfaceVisibility mir_surface_get_visibility(MirSurface *surface);
256270
257/**271/**
258 * Choose the cursor state for a surface: whether a cursor is shown, 272 * Choose the cursor state for a surface: whether a cursor is shown,
259273
=== modified file 'include/server/mir/frontend/surface.h'
--- include/server/mir/frontend/surface.h 2014-06-09 17:35:20 +0000
+++ include/server/mir/frontend/surface.h 2014-07-09 21:43:55 +0000
@@ -53,6 +53,7 @@
53 virtual int client_input_fd() const = 0;53 virtual int client_input_fd() const = 0;
5454
55 virtual int configure(MirSurfaceAttrib attrib, int value) = 0;55 virtual int configure(MirSurfaceAttrib attrib, int value) = 0;
56 virtual int query(MirSurfaceAttrib attrib) = 0;
5657
57 virtual void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& image) = 0;58 virtual void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& image) = 0;
5859
5960
=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h 2014-06-23 02:57:39 +0000
+++ include/shared/mir_toolkit/common.h 2014-07-09 21:43:55 +0000
@@ -33,12 +33,14 @@
33 */33 */
34typedef enum MirSurfaceAttrib34typedef enum MirSurfaceAttrib
35{35{
36 /* Do not specify values...code relies on 0...N ordering. */
36 mir_surface_attrib_type,37 mir_surface_attrib_type,
37 mir_surface_attrib_state,38 mir_surface_attrib_state,
38 mir_surface_attrib_swapinterval,39 mir_surface_attrib_swapinterval,
39 mir_surface_attrib_focus,40 mir_surface_attrib_focus,
40 mir_surface_attrib_dpi,41 mir_surface_attrib_dpi,
41 mir_surface_attrib_visibility,42 mir_surface_attrib_visibility,
43 /* Must be last */
42 mir_surface_attribs44 mir_surface_attribs
43} MirSurfaceAttrib;45} MirSurfaceAttrib;
4446
@@ -68,6 +70,8 @@
68 mir_surface_states70 mir_surface_states
69} MirSurfaceState;71} MirSurfaceState;
7072
73/* TODO: MirSurfaceFocusState MirSurfaceVisibility and MirLifecycleState use an inconsistent
74 naming convention. */
71typedef enum MirSurfaceFocusState75typedef enum MirSurfaceFocusState
72{76{
73 mir_surface_unfocused = 0,77 mir_surface_unfocused = 0,
7478
=== modified file 'include/test/mir_test_doubles/mock_frontend_surface.h'
--- include/test/mir_test_doubles/mock_frontend_surface.h 2014-06-03 17:00:23 +0000
+++ include/test/mir_test_doubles/mock_frontend_surface.h 2014-07-09 21:43:55 +0000
@@ -50,6 +50,7 @@
50 MOCK_METHOD1(set_cursor_image, void(std::shared_ptr<graphics::CursorImage> const&));50 MOCK_METHOD1(set_cursor_image, void(std::shared_ptr<graphics::CursorImage> const&));
5151
52 MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));52 MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));
53 MOCK_METHOD1(query, int(MirSurfaceAttrib));
53};54};
54}55}
55}56}
5657
=== modified file 'include/test/mir_test_doubles/stub_scene_surface.h'
--- include/test/mir_test_doubles/stub_scene_surface.h 2014-06-23 22:25:17 +0000
+++ include/test/mir_test_doubles/stub_scene_surface.h 2014-07-09 21:43:55 +0000
@@ -96,6 +96,7 @@
96 bool supports_input() const override { return true;}96 bool supports_input() const override { return true;}
97 int client_input_fd() const override { return fd;}97 int client_input_fd() const override { return fd;}
98 int configure(MirSurfaceAttrib, int) override { return 0; }98 int configure(MirSurfaceAttrib, int) override { return 0; }
99 int query(MirSurfaceAttrib) override { return 0; }
99 void with_most_recent_buffer_do(std::function<void(graphics::Buffer&)> const& ) override {}100 void with_most_recent_buffer_do(std::function<void(graphics::Buffer&)> const& ) override {}
100};101};
101102
102103
=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp 2014-06-23 02:57:39 +0000
+++ src/client/mir_surface.cpp 2014-07-09 21:43:55 +0000
@@ -67,9 +67,6 @@
6767
68 for (int i = 0; i < mir_surface_attribs; i++)68 for (int i = 0; i < mir_surface_attribs; i++)
69 attrib_cache[i] = -1;69 attrib_cache[i] = -1;
70 attrib_cache[mir_surface_attrib_type] = mir_surface_type_normal;
71 attrib_cache[mir_surface_attrib_state] = mir_surface_state_unknown;
72 attrib_cache[mir_surface_attrib_swapinterval] = 1;
7370
74 std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);71 std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
75 valid_surfaces.insert(this);72 valid_surfaces.insert(this);
@@ -228,6 +225,12 @@
228225
229 process_incoming_buffer();226 process_incoming_buffer();
230 accelerated_window = platform->create_egl_native_window(this);227 accelerated_window = platform->create_egl_native_window(this);
228
229 for(int i = 0; i < surface.attributes_size(); i++)
230 {
231 auto const& attrib = surface.attributes(i);
232 attrib_cache[attrib.attrib()] = attrib.ivalue();
233 }
231 }234 }
232235
233 connection->on_surface_created(id(), this);236 connection->on_surface_created(id(), this);
234237
=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h 2014-06-23 02:57:39 +0000
+++ src/client/mir_surface.h 2014-07-09 21:43:55 +0000
@@ -86,7 +86,10 @@
86 EGLNativeWindowType generate_native_window();86 EGLNativeWindowType generate_native_window();
8787
88 MirWaitHandle* configure(MirSurfaceAttrib a, int value);88 MirWaitHandle* configure(MirSurfaceAttrib a, int value);
89
90 // Non-blocking
89 int attrib(MirSurfaceAttrib a) const;91 int attrib(MirSurfaceAttrib a) const;
92
90 MirOrientation get_orientation() const;93 MirOrientation get_orientation() const;
91 94
92 MirWaitHandle* configure_cursor(MirCursorConfiguration const* cursor);95 MirWaitHandle* configure_cursor(MirCursorConfiguration const* cursor);
9396
=== modified file 'src/client/mir_surface_api.cpp'
--- src/client/mir_surface_api.cpp 2014-06-19 16:15:42 +0000
+++ src/client/mir_surface_api.cpp 2014-07-09 21:43:55 +0000
@@ -247,34 +247,73 @@
247247
248int mir_surface_get_swapinterval(MirSurface* surf)248int mir_surface_get_swapinterval(MirSurface* surf)
249{249{
250 return surf ? surf->attrib(mir_surface_attrib_swapinterval) : -1;250 int swap_interval = -1;
251
252 try
253 {
254 swap_interval = surf ? surf->attrib(mir_surface_attrib_swapinterval) : -1;
255 }
256 catch (...)
257 {
258 }
259
260 return swap_interval;
251}261}
252262
253int mir_surface_get_dpi(MirSurface* surf)263int mir_surface_get_dpi(MirSurface* surf)
254{264{
255 int dpi = 0;265 int dpi = -1;
256266
257 try267 try
258 {268 {
259 if (surf)269 if (surf)
260 {270 {
261 dpi = surf->attrib(mir_surface_attrib_dpi);271 dpi = surf->attrib(mir_surface_attrib_dpi);
262 if (dpi < 0)
263 {
264 // Officially we don't support setting DPI from the client.
265 // But this is a convenient way to query it on startup...
266 surf->configure(mir_surface_attrib_dpi, -1)->wait_for_all();
267 dpi = surf->attrib(mir_surface_attrib_dpi);
268 }
269 }272 }
270 }273 }
271 catch (std::exception const&)274 catch (...)
272 {275 {
273 }276 }
274277
275 return dpi;278 return dpi;
276}279}
277280
281MirSurfaceFocusState mir_surface_get_focus(MirSurface* surf)
282{
283 MirSurfaceFocusState state = mir_surface_unfocused;
284
285 try
286 {
287 if (surf)
288 {
289 state = static_cast<MirSurfaceFocusState>(surf->attrib(mir_surface_attrib_focus));
290 }
291 }
292 catch (...)
293 {
294 }
295
296 return state;
297}
298
299MirSurfaceVisibility mir_surface_get_visibility(MirSurface* surf)
300{
301 MirSurfaceVisibility state = static_cast<MirSurfaceVisibility>(mir_surface_visibility_occluded);
302
303 try
304 {
305 if (surf)
306 {
307 state = static_cast<MirSurfaceVisibility>(surf->attrib(mir_surface_attrib_visibility));
308 }
309 }
310 catch (...)
311 {
312 }
313
314 return state;
315}
316
278MirWaitHandle* mir_surface_configure_cursor(MirSurface* surface, MirCursorConfiguration const* cursor)317MirWaitHandle* mir_surface_configure_cursor(MirSurface* surface, MirCursorConfiguration const* cursor)
279{318{
280 MirWaitHandle *result = nullptr;319 MirWaitHandle *result = nullptr;
281320
=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp 2014-07-09 10:48:47 +0000
+++ src/server/frontend/session_mediator.cpp 2014-07-09 21:43:55 +0000
@@ -196,6 +196,15 @@
196196
197 if (surface->supports_input())197 if (surface->supports_input())
198 response->add_fd(surface->client_input_fd());198 response->add_fd(surface->client_input_fd());
199
200 for (unsigned int i = 0; i < mir_surface_attribs; i++)
201 {
202 auto setting = response->add_attributes();
203
204 setting->mutable_surfaceid()->set_value(surf_id.as_value());
205 setting->set_attrib(i);
206 setting->set_ivalue(surface->query(static_cast<MirSurfaceAttrib>(i)));
207 }
199208
200 advance_buffer(surf_id, *surface,209 advance_buffer(surf_id, *surface,
201 [lock, this, response, done, session]210 [lock, this, response, done, session]
202211
=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp 2014-07-09 07:54:29 +0000
+++ src/server/scene/basic_surface.cpp 2014-07-09 21:43:55 +0000
@@ -159,15 +159,24 @@
159 input_sender(input_sender),159 input_sender(input_sender),
160 configurator(configurator),160 configurator(configurator),
161 cursor_image_(cursor_image),161 cursor_image_(cursor_image),
162 report(report),162 report(report)
163 type_value(mir_surface_type_normal),
164 state_value(mir_surface_state_restored),
165 visibility_value(mir_surface_visibility_exposed),
166 dpi_value(0)
167{163{
164 initialize_attributes();
168 report->surface_created(this, surface_name);165 report->surface_created(this, surface_name);
169}166}
170167
168void ms::BasicSurface::initialize_attributes()
169{
170 std::lock_guard<std::mutex> lg(guard);
171
172 attrib_values[mir_surface_attrib_type] = mir_surface_type_normal;
173 attrib_values[mir_surface_attrib_state] = mir_surface_state_restored;
174 attrib_values[mir_surface_attrib_swapinterval] = 1;
175 attrib_values[mir_surface_attrib_focus] = mir_surface_unfocused;
176 attrib_values[mir_surface_attrib_dpi] = 0;
177 attrib_values[mir_surface_attrib_visibility] = mir_surface_visibility_exposed;
178}
179
171void ms::BasicSurface::force_requests_to_complete()180void ms::BasicSurface::force_requests_to_complete()
172{181{
173 surface_buffer_stream->force_requests_to_complete();182 surface_buffer_stream->force_requests_to_complete();
@@ -404,42 +413,94 @@
404413
405414
406MirSurfaceType ms::BasicSurface::type() const415MirSurfaceType ms::BasicSurface::type() const
407{416{
408 return type_value;417 std::unique_lock<std::mutex> lg(guard);
418 return static_cast<MirSurfaceType>(attrib_values[mir_surface_attrib_type]);
409}419}
410420
411bool ms::BasicSurface::set_type(MirSurfaceType t)421MirSurfaceType ms::BasicSurface::set_type(MirSurfaceType t)
412{422{
413 bool valid = false;423 std::unique_lock<std::mutex> lg(guard);
414424
415 if (t >= 0 && t < mir_surface_types)425 if (t < 0 || t > mir_surface_types)
416 {426 {
417 type_value = t;427 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
418 valid = true;428 "type."));
419 }429 }
420430
421 return valid;431 if (attrib_values[mir_surface_attrib_type] != t)
432 {
433 attrib_values[mir_surface_attrib_type] = t;
434 lg.unlock();
435
436 observers.attrib_changed(mir_surface_attrib_type, attrib_values[mir_surface_attrib_type]);
437 }
438
439 return t;
422}440}
423441
424MirSurfaceState ms::BasicSurface::state() const442MirSurfaceState ms::BasicSurface::state() const
425{443{
426 return state_value;444 std::unique_lock<std::mutex> lg(guard);
445 return static_cast<MirSurfaceState>(attrib_values[mir_surface_attrib_state]);
427}446}
428447
429bool ms::BasicSurface::set_state(MirSurfaceState s)448MirSurfaceState ms::BasicSurface::set_state(MirSurfaceState s)
430{449{
431 bool valid = false;450 if (s < mir_surface_state_unknown || s > mir_surface_states)
451 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface state."));
432452
433 if (s > mir_surface_state_unknown &&453 std::unique_lock<std::mutex> lg(guard);
434 s < mir_surface_states)454 if (attrib_values[mir_surface_attrib_state] != s)
435 {455 {
436 state_value = s;456 attrib_values[mir_surface_attrib_state] = s;
437 valid = true;457 lg.unlock();
438458
439 observers.attrib_changed(mir_surface_attrib_state, s);459 observers.attrib_changed(mir_surface_attrib_state, s);
440 }460 }
441461
442 return valid;462 return s;
463}
464
465int ms::BasicSurface::set_swap_interval(int interval)
466{
467 if (interval < 0)
468 {
469 BOOST_THROW_EXCEPTION(std::logic_error("Invalid swapinterval"));
470 }
471
472 std::unique_lock<std::mutex> lg(guard);
473 if (attrib_values[mir_surface_attrib_swapinterval] != interval)
474 {
475 attrib_values[mir_surface_attrib_swapinterval] = interval;
476 bool allow_dropping = (interval == 0);
477 allow_framedropping(allow_dropping);
478
479 lg.unlock();
480 observers.attrib_changed(mir_surface_attrib_swapinterval, interval);
481 }
482
483 return interval;
484}
485
486MirSurfaceFocusState ms::BasicSurface::set_focus_state(MirSurfaceFocusState new_state)
487{
488 if (new_state != mir_surface_focused &&
489 new_state != mir_surface_unfocused)
490 {
491 BOOST_THROW_EXCEPTION(std::logic_error("Invalid focus state."));
492 }
493
494 std::unique_lock<std::mutex> lg(guard);
495 if (attrib_values[mir_surface_attrib_focus] != new_state)
496 {
497 attrib_values[mir_surface_attrib_focus] = new_state;
498
499 lg.unlock();
500 observers.attrib_changed(mir_surface_attrib_focus, new_state);
501 }
502
503 return new_state;
443}504}
444505
445void ms::BasicSurface::take_input_focus(std::shared_ptr<msh::InputTargeter> const& targeter)506void ms::BasicSurface::take_input_focus(std::shared_ptr<msh::InputTargeter> const& targeter)
@@ -449,43 +510,26 @@
449510
450int ms::BasicSurface::configure(MirSurfaceAttrib attrib, int value)511int ms::BasicSurface::configure(MirSurfaceAttrib attrib, int value)
451{512{
452 bool allow_dropping = false;513 int result = configurator->select_attribute_value(*this, attrib, value);
453
454 /*
455 * TODO: In future, query the shell implementation for the subset of
456 * attributes/types it implements.
457 */
458 int result = value = configurator->select_attribute_value(*this, attrib, value);
459 switch (attrib)514 switch (attrib)
460 {515 {
461 case mir_surface_attrib_type:516 case mir_surface_attrib_type:
462 if (!set_type(static_cast<MirSurfaceType>(value)))517 result = set_type(static_cast<MirSurfaceType>(result));
463 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
464 "type."));
465 result = type();
466 break;518 break;
467 case mir_surface_attrib_state:519 case mir_surface_attrib_state:
468 if (value != mir_surface_state_unknown &&520 result = set_state(static_cast<MirSurfaceState>(result));
469 !set_state(static_cast<MirSurfaceState>(value)))
470 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface state."));
471 result = state();
472 break;521 break;
473 case mir_surface_attrib_focus:522 case mir_surface_attrib_focus:
474 observers.attrib_changed(attrib, value);523 result = set_focus_state(static_cast<MirSurfaceFocusState>(result));
475 break;524 break;
476 case mir_surface_attrib_swapinterval:525 case mir_surface_attrib_swapinterval:
477 allow_dropping = (value == 0);526 result = set_swap_interval(result);
478 allow_framedropping(allow_dropping);
479 result = value;
480 break;527 break;
481 case mir_surface_attrib_dpi:528 case mir_surface_attrib_dpi:
482 if (value >= 0 && !set_dpi(value)) // Negative means query only529 result = set_dpi(result);
483 BOOST_THROW_EXCEPTION(std::logic_error("Invalid DPI value"));
484 result = dpi();
485 break;530 break;
486 case mir_surface_attrib_visibility:531 case mir_surface_attrib_visibility:
487 set_visibility(static_cast<MirSurfaceVisibility>(value));532 result = set_visibility(static_cast<MirSurfaceVisibility>(result));
488 result = visibility_value;
489 break;533 break;
490 default:534 default:
491 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "535 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
@@ -498,6 +542,16 @@
498 return result;542 return result;
499}543}
500544
545int ms::BasicSurface::query(MirSurfaceAttrib attrib)
546{
547 if (attrib < 0 || attrib > mir_surface_attribs)
548 BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
549 "attribute."));
550
551 std::unique_lock<std::mutex> lg(guard);
552 return attrib_values[attrib];
553}
554
501void ms::BasicSurface::hide()555void ms::BasicSurface::hide()
502{556{
503 set_hidden(true);557 set_hidden(true);
@@ -528,24 +582,29 @@
528582
529int ms::BasicSurface::dpi() const583int ms::BasicSurface::dpi() const
530{584{
531 return dpi_value;585 std::unique_lock<std::mutex> lock(guard);
586 return attrib_values[mir_surface_attrib_dpi];
532}587}
533588
534bool ms::BasicSurface::set_dpi(int new_dpi)589int ms::BasicSurface::set_dpi(int new_dpi)
535{590{
536 bool valid = false;591 if (new_dpi < 0)
592 {
593 BOOST_THROW_EXCEPTION(std::logic_error("Invalid DPI value"));
594 }
537595
538 if (new_dpi >= 0)596 std::unique_lock<std::mutex> lg(guard);
597 if (attrib_values[mir_surface_attrib_dpi] != new_dpi)
539 {598 {
540 dpi_value = new_dpi;599 attrib_values[mir_surface_attrib_dpi] = new_dpi;
541 valid = true;600 lg.unlock();
542 observers.attrib_changed(mir_surface_attrib_dpi, dpi_value);601 observers.attrib_changed(mir_surface_attrib_dpi, new_dpi);
543 }602 }
544 603
545 return valid;604 return new_dpi;
546}605}
547606
548void ms::BasicSurface::set_visibility(MirSurfaceVisibility new_visibility)607MirSurfaceVisibility ms::BasicSurface::set_visibility(MirSurfaceVisibility new_visibility)
549{608{
550 if (new_visibility != mir_surface_visibility_occluded &&609 if (new_visibility != mir_surface_visibility_occluded &&
551 new_visibility != mir_surface_visibility_exposed)610 new_visibility != mir_surface_visibility_exposed)
@@ -553,11 +612,15 @@
553 BOOST_THROW_EXCEPTION(std::logic_error("Invalid visibility value"));612 BOOST_THROW_EXCEPTION(std::logic_error("Invalid visibility value"));
554 }613 }
555614
556 if (visibility_value != new_visibility)615 std::unique_lock<std::mutex> lg(guard);
616 if (attrib_values[mir_surface_attrib_visibility] != new_visibility)
557 {617 {
558 visibility_value = new_visibility;618 attrib_values[mir_surface_attrib_visibility] = new_visibility;
559 observers.attrib_changed(mir_surface_attrib_visibility, visibility_value);619 lg.unlock();
620 observers.attrib_changed(mir_surface_attrib_visibility, attrib_values[mir_surface_attrib_visibility]);
560 }621 }
622
623 return new_visibility;
561}624}
562625
563void ms::BasicSurface::add_observer(std::shared_ptr<SurfaceObserver> const& observer)626void ms::BasicSurface::add_observer(std::shared_ptr<SurfaceObserver> const& observer)
564627
=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h 2014-06-27 13:40:34 +0000
+++ src/server/scene/basic_surface.h 2014-07-09 21:43:55 +0000
@@ -139,6 +139,7 @@
139 MirSurfaceState state() const override;139 MirSurfaceState state() const override;
140 void take_input_focus(std::shared_ptr<shell::InputTargeter> const& targeter) override;140 void take_input_focus(std::shared_ptr<shell::InputTargeter> const& targeter) override;
141 int configure(MirSurfaceAttrib attrib, int value) override;141 int configure(MirSurfaceAttrib attrib, int value) override;
142 int query(MirSurfaceAttrib attrib) override;
142 void hide() override;143 void hide() override;
143 void show() override;144 void show() override;
144 145
@@ -152,10 +153,12 @@
152153
153private:154private:
154 bool visible(std::unique_lock<std::mutex>&) const;155 bool visible(std::unique_lock<std::mutex>&) const;
155 bool set_type(MirSurfaceType t); // Use configure() to make public changes156 MirSurfaceType set_type(MirSurfaceType t); // Use configure() to make public changes
156 bool set_state(MirSurfaceState s);157 MirSurfaceState set_state(MirSurfaceState s);
157 bool set_dpi(int);158 int set_dpi(int);
158 void set_visibility(MirSurfaceVisibility v);159 MirSurfaceVisibility set_visibility(MirSurfaceVisibility v);
160 int set_swap_interval(int);
161 MirSurfaceFocusState set_focus_state(MirSurfaceFocusState f);
159162
160 SurfaceObservers observers;163 SurfaceObservers observers;
161 std::mutex mutable guard;164 std::mutex mutable guard;
@@ -175,10 +178,8 @@
175 std::shared_ptr<graphics::CursorImage> cursor_image_;178 std::shared_ptr<graphics::CursorImage> cursor_image_;
176 std::shared_ptr<SceneReport> const report;179 std::shared_ptr<SceneReport> const report;
177180
178 MirSurfaceType type_value;181 void initialize_attributes();
179 MirSurfaceState state_value;182 int attrib_values[mir_surface_attribs];
180 MirSurfaceVisibility visibility_value;
181 int dpi_value;
182};183};
183184
184}185}
185186
=== modified file 'src/shared/protobuf/mir_protobuf.proto'
--- src/shared/protobuf/mir_protobuf.proto 2014-07-09 07:54:29 +0000
+++ src/shared/protobuf/mir_protobuf.proto 2014-07-09 21:43:55 +0000
@@ -103,6 +103,8 @@
103103
104 repeated sint32 fd = 7;104 repeated sint32 fd = 7;
105 optional int32 fds_on_side_channel = 8;105 optional int32 fds_on_side_channel = 8;
106
107 repeated SurfaceSetting attributes = 9;
106108
107 optional string error = 127;109 optional string error = 127;
108}110}
109111
=== modified file 'tests/acceptance-tests/test_client_surface_visibility.cpp'
--- tests/acceptance-tests/test_client_surface_visibility.cpp 2014-06-23 17:19:22 +0000
+++ tests/acceptance-tests/test_client_surface_visibility.cpp 2014-07-09 21:43:55 +0000
@@ -293,7 +293,10 @@
293 mt::WaitCondition event_received;293 mt::WaitCondition event_received;
294294
295 EXPECT_CALL(mock_visibility_callback, handle(surface, visibility))295 EXPECT_CALL(mock_visibility_callback, handle(surface, visibility))
296 .WillOnce(mt::WakeUp(&event_received));296 .WillOnce(DoAll(Invoke([&visibility](MirSurface *s, MirSurfaceVisibility)
297 {
298 EXPECT_EQ(visibility, mir_surface_get_visibility(s));
299 }), mt::WakeUp(&event_received)));
297300
298 action();301 action();
299302
300303
=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp 2014-06-24 12:55:16 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp 2014-07-09 21:43:55 +0000
@@ -36,11 +36,12 @@
36#include "mir_test/gmock_fixes.h"36#include "mir_test/gmock_fixes.h"
37#include "mir_test/fake_shared.h"37#include "mir_test/fake_shared.h"
3838
39#include <cstring>
40
41#include <gtest/gtest.h>39#include <gtest/gtest.h>
42#include <gmock/gmock.h>40#include <gmock/gmock.h>
4341
42#include <cstring>
43#include <map>
44
44#include <fcntl.h>45#include <fcntl.h>
4546
46namespace mcl = mir::client;47namespace mcl = mir::client;
@@ -96,6 +97,8 @@
96 int pf_sent;97 int pf_sent;
97 int stride_sent;98 int stride_sent;
9899
100 static std::map<int, int> sent_surface_attributes;
101
99private:102private:
100 void generate_unique_buffer()103 void generate_unique_buffer()
101 {104 {
@@ -143,16 +146,26 @@
143 response->set_width(server_package.width);146 response->set_width(server_package.width);
144 response->set_height(server_package.height);147 response->set_height(server_package.height);
145 }148 }
146149
147 void create_surface_response(mir::protobuf::Surface* response)150 void create_surface_response(mir::protobuf::Surface* response)
148 {151 {
152 unsigned int const id = 2;
153
149 response->set_fds_on_side_channel(1);154 response->set_fds_on_side_channel(1);
150155
151 response->mutable_id()->set_value(2);156 response->mutable_id()->set_value(id);
152 response->set_width(width_sent);157 response->set_width(width_sent);
153 response->set_height(height_sent);158 response->set_height(height_sent);
154 response->set_pixel_format(pf_sent);159 response->set_pixel_format(pf_sent);
155 response->add_fd(input_fd);160 response->add_fd(input_fd);
161
162 for (auto const& kv : sent_surface_attributes)
163 {
164 auto setting = response->add_attributes();
165 setting->mutable_surfaceid()->set_value(id);
166 setting->set_attrib(kv.first);
167 setting->set_ivalue(kv.second);
168 }
156169
157 create_buffer_response(response->mutable_buffer());170 create_buffer_response(response->mutable_buffer());
158 }171 }
@@ -167,6 +180,15 @@
167 int global_buffer_id;180 int global_buffer_id;
168};181};
169182
183std::map<int, int> MockServerPackageGenerator::sent_surface_attributes = {
184 { mir_surface_attrib_type, mir_surface_type_normal },
185 { mir_surface_attrib_state, mir_surface_state_restored },
186 { mir_surface_attrib_swapinterval, 1 },
187 { mir_surface_attrib_focus, mir_surface_focused },
188 { mir_surface_attrib_dpi, 19 },
189 { mir_surface_attrib_visibility, mir_surface_visibility_exposed }
190};
191
170struct MockBuffer : public mcl::ClientBuffer192struct MockBuffer : public mcl::ClientBuffer
171{193{
172 MockBuffer()194 MockBuffer()
@@ -469,6 +491,18 @@
469 create_and_wait_for_surface_with(*client_comm_channel, mock_buffer_factory);491 create_and_wait_for_surface_with(*client_comm_channel, mock_buffer_factory);
470}492}
471493
494TEST_F(MirClientSurfaceTest, attributes_set_on_surface_creation)
495{
496 using namespace testing;
497
498 auto surface = create_and_wait_for_surface_with(*client_comm_channel, mock_buffer_factory);
499
500 for (int i = 0; i < mir_surface_attribs; i++)
501 {
502 EXPECT_EQ(MockServerPackageGenerator::sent_surface_attributes[i], surface->attrib(static_cast<MirSurfaceAttrib>(i)));
503 }
504}
505
472TEST_F(MirClientSurfaceTest, create_wait_handle_really_blocks)506TEST_F(MirClientSurfaceTest, create_wait_handle_really_blocks)
473{507{
474 using namespace testing;508 using namespace testing;
@@ -630,28 +664,6 @@
630 EXPECT_THAT(after.height, Eq(new_height));664 EXPECT_THAT(after.height, Eq(new_height));
631}665}
632666
633TEST_F(MirClientSurfaceTest, default_surface_type)
634{
635 using namespace testing;
636
637 auto const surface = create_and_wait_for_surface_with(*client_comm_channel, stub_buffer_factory);
638
639 EXPECT_THAT(surface->attrib(mir_surface_attrib_type),
640 Eq(mir_surface_type_normal));
641}
642
643TEST_F(MirClientSurfaceTest, default_surface_state)
644{
645 using namespace testing;
646
647 auto const surface = create_and_wait_for_surface_with(*client_comm_channel, stub_buffer_factory);
648
649 // Test the default cached state value. It is always unknown until we
650 // get a real answer from the server.
651 EXPECT_THAT(surface->attrib(mir_surface_attrib_state),
652 Eq(mir_surface_state_unknown));
653}
654
655TEST_F(MirClientSurfaceTest, get_cpu_region_returns_correct_data)667TEST_F(MirClientSurfaceTest, get_cpu_region_returns_correct_data)
656{668{
657 using namespace testing;669 using namespace testing;
658670
=== modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
--- tests/unit-tests/scene/test_basic_surface.cpp 2014-07-09 07:54:29 +0000
+++ tests/unit-tests/scene/test_basic_surface.cpp 2014-07-09 21:43:55 +0000
@@ -468,48 +468,150 @@
468 EXPECT_EQ(mi::InputReceptionMode::receives_all_input, surface.reception_mode());468 EXPECT_EQ(mi::InputReceptionMode::receives_all_input, surface.reception_mode());
469}469}
470470
471TEST_F(BasicSurfaceTest, notifies_about_visibility_attrib_changes)471namespace
472{
473
474struct AttributeTestParameters
475{
476 MirSurfaceAttrib attribute;
477 int default_value;
478 int a_valid_value;
479 int an_invalid_value;
480};
481
482struct BasicSurfaceAttributeTest : public BasicSurfaceTest,
483 public ::testing::WithParamInterface<AttributeTestParameters>
484{
485};
486
487AttributeTestParameters const surface_visibility_test_parameters{
488 mir_surface_attrib_visibility,
489 mir_surface_visibility_exposed,
490 mir_surface_visibility_occluded,
491 -1
492};
493
494AttributeTestParameters const surface_type_test_parameters{
495 mir_surface_attrib_type,
496 mir_surface_type_normal,
497 mir_surface_type_freestyle,
498 -1
499};
500
501AttributeTestParameters const surface_state_test_parameters{
502 mir_surface_attrib_state,
503 mir_surface_state_restored,
504 mir_surface_state_fullscreen,
505 1178312
506};
507
508AttributeTestParameters const surface_swapinterval_test_parameters{
509 mir_surface_attrib_swapinterval,
510 1,
511 0,
512 -1
513};
514
515AttributeTestParameters const surface_dpi_test_parameters{
516 mir_surface_attrib_dpi,
517 0,
518 90,
519 -1
520};
521
522AttributeTestParameters const surface_focus_test_parameters{
523 mir_surface_attrib_focus,
524 mir_surface_unfocused,
525 mir_surface_focused,
526 -1
527};
528
529}
530
531TEST_P(BasicSurfaceAttributeTest, default_value)
532{
533 auto const& params = GetParam();
534 auto const& attribute = params.attribute;
535 auto const& default_value = params.default_value;
536
537 EXPECT_EQ(default_value, surface.query(attribute));
538}
539
540TEST_P(BasicSurfaceAttributeTest, notifies_about_attrib_changes)
472{541{
473 using namespace testing;542 using namespace testing;
543
544 auto const& params = GetParam();
545 auto const& attribute = params.attribute;
546 auto const& value1 = params.a_valid_value;
547 auto const& value2 = params.default_value;
548
474 MockSurfaceAttribObserver mock_surface_observer;549 MockSurfaceAttribObserver mock_surface_observer;
475550
476 InSequence s;551 InSequence s;
477 EXPECT_CALL(mock_surface_observer, attrib_changed(mir_surface_attrib_visibility, mir_surface_visibility_occluded))552 EXPECT_CALL(mock_surface_observer, attrib_changed(attribute, value1))
478 .Times(1);553 .Times(1);
479 EXPECT_CALL(mock_surface_observer, attrib_changed(mir_surface_attrib_visibility, mir_surface_visibility_exposed))554 EXPECT_CALL(mock_surface_observer, attrib_changed(attribute, value2))
480 .Times(1);555 .Times(1);
481556
482 surface.add_observer(mt::fake_shared(mock_surface_observer));557 surface.add_observer(mt::fake_shared(mock_surface_observer));
483558
484 surface.configure(mir_surface_attrib_visibility, mir_surface_visibility_occluded);559 surface.configure(attribute, value1);
485 surface.configure(mir_surface_attrib_visibility, mir_surface_visibility_exposed);560 surface.configure(attribute, value2);
486}561}
487562
488TEST_F(BasicSurfaceTest, does_not_notify_if_visibility_attrib_is_unchanged)563TEST_P(BasicSurfaceAttributeTest, does_not_notify_if_attrib_is_unchanged)
489{564{
490 using namespace testing;565 using namespace testing;
566
567 auto const& params = GetParam();
568 auto const& attribute = params.attribute;
569 auto const& default_value = params.default_value;
570 auto const& another_value = params.a_valid_value;
571
491 MockSurfaceAttribObserver mock_surface_observer;572 MockSurfaceAttribObserver mock_surface_observer;
492573
493 EXPECT_CALL(mock_surface_observer, attrib_changed(mir_surface_attrib_visibility, mir_surface_visibility_occluded))574 EXPECT_CALL(mock_surface_observer, attrib_changed(attribute, another_value))
494 .Times(1);575 .Times(1);
495576
496 surface.add_observer(mt::fake_shared(mock_surface_observer));577 surface.add_observer(mt::fake_shared(mock_surface_observer));
497578
498 surface.configure(mir_surface_attrib_visibility, mir_surface_visibility_exposed);579 surface.configure(attribute, default_value);
499 surface.configure(mir_surface_attrib_visibility, mir_surface_visibility_occluded);580 surface.configure(attribute, another_value);
500 surface.configure(mir_surface_attrib_visibility, mir_surface_visibility_occluded);581 surface.configure(attribute, another_value);
501}582}
502583
503TEST_F(BasicSurfaceTest, throws_on_invalid_visibility_attrib_value)584TEST_P(BasicSurfaceAttributeTest, throws_on_invalid_value)
504{585{
505 using namespace testing;586 using namespace testing;
506587
588 auto const& params = GetParam();
589 auto const& attribute = params.attribute;
590 auto const& invalid_value = params.an_invalid_value;
591
507 EXPECT_THROW({592 EXPECT_THROW({
508 surface.configure(mir_surface_attrib_visibility,593 surface.configure(attribute, invalid_value);
509 static_cast<int>(mir_surface_visibility_exposed) + 1);594 }, std::logic_error);
510 }, std::logic_error);
511}595}
512596
597INSTANTIATE_TEST_CASE_P(SurfaceTypeAttributeTest, BasicSurfaceAttributeTest,
598 ::testing::Values(surface_type_test_parameters));
599
600INSTANTIATE_TEST_CASE_P(SurfaceVisibilityAttributeTest, BasicSurfaceAttributeTest,
601 ::testing::Values(surface_visibility_test_parameters));
602
603INSTANTIATE_TEST_CASE_P(SurfaceStateAttributeTest, BasicSurfaceAttributeTest,
604 ::testing::Values(surface_state_test_parameters));
605
606INSTANTIATE_TEST_CASE_P(SurfaceSwapintervalAttributeTest, BasicSurfaceAttributeTest,
607 ::testing::Values(surface_swapinterval_test_parameters));
608
609INSTANTIATE_TEST_CASE_P(SurfaceDPIAttributeTest, BasicSurfaceAttributeTest,
610 ::testing::Values(surface_dpi_test_parameters));
611
612INSTANTIATE_TEST_CASE_P(SurfaceFocusAttributeTest, BasicSurfaceAttributeTest,
613 ::testing::Values(surface_focus_test_parameters));
614
513TEST_F(BasicSurfaceTest, configure_returns_value_set_by_configurator)615TEST_F(BasicSurfaceTest, configure_returns_value_set_by_configurator)
514{616{
515 using namespace testing;617 using namespace testing;
516618
=== modified file 'tests/unit-tests/scene/test_surface_impl.cpp'
--- tests/unit-tests/scene/test_surface_impl.cpp 2014-06-23 22:25:17 +0000
+++ tests/unit-tests/scene/test_surface_impl.cpp 2014-07-09 21:43:55 +0000
@@ -186,52 +186,6 @@
186 EXPECT_EQ(mir_surface_state_fullscreen, surf.state());186 EXPECT_EQ(mir_surface_state_fullscreen, surf.state());
187}187}
188188
189TEST_F(Surface, dpi_is_initialized)
190{
191 using namespace testing;
192
193 ms::BasicSurface surf(
194 std::string("stub"),
195 geom::Rectangle{{},{}},
196 false,
197 buffer_stream,
198 std::shared_ptr<mi::InputChannel>(),
199 stub_input_sender,
200 null_configurator,
201 std::shared_ptr<mg::CursorImage>(),
202 report);
203
204 EXPECT_EQ(0, surf.dpi()); // The current default. It will change.
205}
206
207TEST_F(Surface, dpi_changes)
208{
209 using namespace testing;
210
211 ms::BasicSurface surf(
212 std::string("stub"),
213 geom::Rectangle{{},{}},
214 false,
215 buffer_stream,
216 std::shared_ptr<mi::InputChannel>(),
217 stub_input_sender,
218 null_configurator,
219 std::shared_ptr<mg::CursorImage>(),
220 report);
221
222 EXPECT_EQ(123, surf.configure(mir_surface_attrib_dpi, 123));
223 EXPECT_EQ(123, surf.dpi());
224
225 EXPECT_EQ(456, surf.configure(mir_surface_attrib_dpi, 456));
226 EXPECT_EQ(456, surf.dpi());
227
228 surf.configure(mir_surface_attrib_dpi, -1);
229 EXPECT_EQ(456, surf.dpi());
230
231 EXPECT_EQ(789, surf.configure(mir_surface_attrib_dpi, 789));
232 EXPECT_EQ(789, surf.dpi());
233}
234
235bool operator==(MirEvent const& a, MirEvent const& b)189bool operator==(MirEvent const& a, MirEvent const& b)
236{190{
237 // We will always fill unused bytes with zero, so memcmp is accurate...191 // We will always fill unused bytes with zero, so memcmp is accurate...

Subscribers

People subscribed via source and target branches