Merge lp:~kdub/mir/interval0-signal into lp:~mir-team/mir/trunk

[Kevin DuBois]

Activate sending a "swapinterval" signal over IPC. Add client api for software clients to request different swapintervals. (eglSwapInterval is not glued together just yet)
Currently only swapinterval of 0 or 1 is supported.

Activate sending a "swapinterval" signal over IPC. Add client api for software clients to request different swapintervals. (eglSwapInterval is not glued together just yet)
Currently only swapinterval of 0 or 1 is supported.

note that the actual path taken over ipc is via the surface parameters, there is no new protobuf message introduced to support the message.

tests/integration-tests/test_swapinterval.cpp tests from the client's request until the point that the request hits the compositor systems at mc::BufferStream

still in pre-review, dependent on anativewindow-external-refcount

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:758
http://jenkins.qa.ubuntu.com/job/mir-ci/751/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/937/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/819/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/236/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/435

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/751/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:759
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~kdub/mir/interval0-signal/+merge/170423/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/771/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/969
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/851
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/9/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/771/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:758
http://jenkins.qa.ubuntu.com/job/mir-ci/772/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/971
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/853
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/10/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/451

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/772/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

1. Swap interval >1 is valid, but not usually useful. I wonder if there's any value in generalizing so interval>1 is supported...?

2. Documentation on the limitations of swap interval ([0,1]) is missing from the comment for mir_surface_set_swapinterval.

3. Consistency:
132 +typedef enum MirSurfacePerformanceHint
133 +{
134 + mir_surface_hint_synchronous,
135 + mir_surface_hint_drop_frames,
136 + mir_surface_hint_arraysize_
137 +} MirSurfacePerformanceHint;
"MirSurfacePerformanceHint" != "mir_surface_hint". I suggest removing the word "Performance". Also from:
124 + mir_surface_attrib_performance_hint,
Although "hint" is a bit vague. Why not make the attribute named "swap_interval" or similar? Then you don't need MirSurfacePerformanceHint at all because it's just an int.

4. Lack of indentation under "196 + switch (interval)" is a style I haven't seen in Mir before. Is it within policy? Do we care?

5. I tried adding mir_surface_set_swapinterval(surface, 0) to demo_client_unaccelerated and didn't expect what I saw:
I see the swap happening *much* less often with 0 than 1. However adding a frame rate counter in the client shows it's higher for the client, as it should be. I suspect we have a buffer scheduling problem that makes interval=0 look much slower than it should. But it might just be the natural randomness of frame dropping making longer periods of green/blue appear in demo_client_unaccelerated. I will have to modify/create a software client (to actually animate something) to investigate further if this is a real issue. And even if there is an issue with it I'm not even sure this branch is to blame...

So really just #2-#4 need attention right now.

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

> 4. Lack of indentation under "196 + switch (interval)" is a style I haven't seen in Mir before.
> Is it within policy? Do we care?

I see more indentation that we normally use, not a lack of it :)

274 + void mc::SwapperFactory::change_swapper_size(
297 + create_swapper_reuse_buffers(
298 - std::vector<std::shared_ptr<Buffer>>& list, size_t buffer_num, SwapperType type) const
299 + BufferProperties const& buffer_properties, std::vector<std::shared_ptr<Buffer>>& list

It feels strange that we remove/add buffers to the vector that is handed to us (although it has no ill effect, since the caller doesn't care about the buffers). I think it would be cleaner to create and manipulate a new vector internally (and make the create_swapper_reuse_buffers() parameter const).

289 + } else

else should be on separate line

review: Needs Fixing

[Kevin DuBois (kdub)]

@Daniel
#1, we don't have a driving use case for >1 swapinterval at the moment, plus it would take some changes to the swappers, so we can put it on the wishlist.
#2 hopefully better documentation added
#3 good idea, code is cleaner with this
#4 just bad indentation, was eliminated with #3
#5 i think this is just the unaccelerated client picking blue or green frames multiple times in a row. If you call this function with an eglapp.c based gl client, you'll see higher than vsync framerates reported.

@Alexandros
I agree, have to come up with a solution to allocate a buffer when going from 2->3 buffers, and preserve the buffer for any future 3->2 transitions. The android drivers didn't like switching buffers of the same size/format/usage out from under them.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:764
http://jenkins.qa.ubuntu.com/job/mir-ci/798/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1001
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/886/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/36/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/474

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/798/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Thanks Kevin. I can see the requested changes made but have not re-reviewed yet.

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:765
http://jenkins.qa.ubuntu.com/job/mir-ci/810/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1017
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/902
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/48
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/48/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/484

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/810/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

A lot has landed today. So now we have:

Text conflict in src/server/shell/surface.cpp
Text conflict in tests/unit-tests/surfaces/test_surface.cpp
2 conflicts encountered.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

1. Conflicts:
Text conflict in src/server/shell/surface.cpp
Text conflict in tests/unit-tests/surfaces/test_surface.cpp

2. Please resolve this inconsistency: "swapinterval" and "swap_interval".

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:766
http://jenkins.qa.ubuntu.com/job/mir-ci/828/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1043/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/928/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/66/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/495

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/828/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:767
http://jenkins.qa.ubuntu.com/job/mir-ci/830/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1045
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/930
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/68
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/68/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/497

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/830/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

OK, I have no more complaints. But I'm not going to redo all the testing I've already done on earlier revisions.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/client/mir_toolkit/mir_client_library.h'
--- include/client/mir_toolkit/mir_client_library.h	2013-06-20 09:43:30 +0000
+++ include/client/mir_toolkit/mir_client_library.h	2013-06-25 19:32:30 +0000
@@ -293,6 +293,27 @@
  */
 MirSurfaceState mir_surface_get_state(MirSurface *surface);
 
+/**
+ * Set the swapinterval for mir_surface_swap_buffers. EGL users should use
+ * eglSwapInterval directly.
+ * At the time being, only swapinterval of 0 or 1 is supported. 
+ *   \param [in] surface  The surface to operate on
+ *   \param [in] interval The number of vblank signals that
+ *                        mir_surface_swap_buffers will wait for 
+ *   \return              A wait handle that can be passed to mir_wait_for,
+ *                        or NULL if the interval could not be supported 
+ */
+MirWaitHandle* mir_surface_set_swapinterval(MirSurface* surface, int interval);
+
+/**
+ * Query the swapinterval that the surface is operating with.
+ * The default interval is 1.
+ *   \param [in] surface  The surface to operate on
+ *   \return              The swapinterval value that the client is operating with.
+ *                        Returns -1 if surface is invalid.
+ */
+int mir_surface_get_swapinterval(MirSurface* surface);
+
 #ifdef __cplusplus
 }
 /**@}*/

=== modified file 'include/server/mir/compositor/buffer_allocation_strategy.h'
--- include/server/mir/compositor/buffer_allocation_strategy.h	2013-06-17 10:26:18 +0000
+++ include/server/mir/compositor/buffer_allocation_strategy.h	2013-06-25 19:32:30 +0000
@@ -45,7 +45,7 @@
 class BufferAllocationStrategy
 {
 public:
-    virtual std::shared_ptr<BufferSwapper> create_swapper_reuse_buffers(
+    virtual std::shared_ptr<BufferSwapper> create_swapper_reuse_buffers(BufferProperties const&,
         std::vector<std::shared_ptr<Buffer>>&, size_t, SwapperType) const = 0;
     virtual std::shared_ptr<BufferSwapper> create_swapper_new_buffers(
         BufferProperties& actual_properties, BufferProperties const& requested_properties, SwapperType) const = 0;

=== modified file 'include/server/mir/compositor/buffer_stream_surfaces.h'
--- include/server/mir/compositor/buffer_stream_surfaces.h	2013-06-20 09:39:10 +0000
+++ include/server/mir/compositor/buffer_stream_surfaces.h	2013-06-25 19:32:30 +0000
@@ -46,6 +46,7 @@
 
     geometry::PixelFormat get_stream_pixel_format();
     geometry::Size stream_size();
+    void allow_framedropping(bool);
     void force_requests_to_complete();
 
 protected:

=== modified file 'include/server/mir/compositor/swapper_factory.h'
--- include/server/mir/compositor/swapper_factory.h	2013-06-11 15:05:27 +0000
+++ include/server/mir/compositor/swapper_factory.h	2013-06-25 19:32:30 +0000
@@ -38,14 +38,18 @@
         std::shared_ptr<GraphicBufferAllocator> const& gr_alloc,
         int number_of_buffers);
 
-    std::shared_ptr<BufferSwapper> create_swapper_reuse_buffers(
+    std::shared_ptr<BufferSwapper> create_swapper_reuse_buffers(BufferProperties const&,
         std::vector<std::shared_ptr<Buffer>>&, size_t, SwapperType) const;
     std::shared_ptr<BufferSwapper> create_swapper_new_buffers(
         BufferProperties& actual_properties, BufferProperties const& requested_properties, SwapperType) const;
 
 private:
+    void change_swapper_size(
+        std::vector<std::shared_ptr<Buffer>>&, size_t const, size_t, BufferProperties const&) const;
+
     std::shared_ptr<GraphicBufferAllocator> const gr_allocator;
-    int const number_of_buffers;
+    unsigned int const synchronous_number_of_buffers;
+    unsigned int const spin_number_of_buffers;
 };
 }
 }

=== modified file 'include/server/mir/shell/surface.h'
--- include/server/mir/shell/surface.h	2013-06-25 05:42:49 +0000
+++ include/server/mir/shell/surface.h	2013-06-25 19:32:30 +0000
@@ -87,6 +87,7 @@
 
     virtual void take_input_focus(std::shared_ptr<InputTargeter> const& targeter);
 
+    virtual void allow_framedropping(bool); 
 private:
     bool set_type(MirSurfaceType t);  // Use configure() to make public changes
     bool set_state(MirSurfaceState s);

=== modified file 'include/server/mir/surfaces/buffer_stream.h'
--- include/server/mir/surfaces/buffer_stream.h	2013-06-20 09:39:10 +0000
+++ include/server/mir/surfaces/buffer_stream.h	2013-06-25 19:32:30 +0000
@@ -45,6 +45,7 @@
     virtual std::shared_ptr<compositor::Buffer> lock_back_buffer() = 0;
     virtual geometry::PixelFormat get_stream_pixel_format() = 0;
     virtual geometry::Size stream_size() = 0;
+    virtual void allow_framedropping(bool) = 0;
     virtual void force_requests_to_complete() = 0;
 };
 

=== modified file 'include/server/mir/surfaces/surface.h'
--- include/server/mir/surfaces/surface.h	2013-06-25 05:42:49 +0000
+++ include/server/mir/surfaces/surface.h	2013-06-25 19:32:30 +0000
@@ -81,6 +81,8 @@
     bool supports_input() const;
     int client_input_fd() const;
     int server_input_fd() const;
+
+    void allow_framedropping(bool); 
 private:
     std::string surface_name;
     geometry::Point top_left_point;

=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h	2013-05-03 22:53:42 +0000
+++ include/shared/mir_toolkit/common.h	2013-06-25 19:32:30 +0000
@@ -35,6 +35,7 @@
 {
     mir_surface_attrib_type,
     mir_surface_attrib_state,
+    mir_surface_attrib_swapinterval,
     mir_surface_attrib_arraysize_
 } MirSurfaceAttrib;
 
@@ -62,7 +63,6 @@
     mir_surface_state_fullscreen,
     mir_surface_state_arraysize_
 } MirSurfaceState;
-
 /**@}*/
 
 #endif

=== modified file 'include/test/mir_test_doubles/mock_buffer_stream.h'
--- include/test/mir_test_doubles/mock_buffer_stream.h	2013-06-19 16:41:13 +0000
+++ include/test/mir_test_doubles/mock_buffer_stream.h	2013-06-25 19:32:30 +0000
@@ -36,6 +36,8 @@
 
     MOCK_METHOD0(get_stream_pixel_format, geometry::PixelFormat());
     MOCK_METHOD0(stream_size, geometry::Size());
+    MOCK_METHOD0(force_client_completion, void());
+    MOCK_METHOD1(allow_framedropping, void(bool));
     MOCK_METHOD0(force_requests_to_complete, void());
 };
 }

=== modified file 'include/test/mir_test_doubles/mock_swapper_factory.h'
--- include/test/mir_test_doubles/mock_swapper_factory.h	2013-06-11 14:27:33 +0000
+++ include/test/mir_test_doubles/mock_swapper_factory.h	2013-06-25 19:32:30 +0000
@@ -33,8 +33,8 @@
 public:
     ~MockSwapperFactory() noexcept {}
 
-    MOCK_CONST_METHOD3(create_swapper_reuse_buffers,
-        std::shared_ptr<compositor::BufferSwapper>(
+    MOCK_CONST_METHOD4(create_swapper_reuse_buffers,
+        std::shared_ptr<compositor::BufferSwapper>(compositor::BufferProperties const&,
             std::vector<std::shared_ptr<compositor::Buffer>>&, size_t, compositor::SwapperType));
     MOCK_CONST_METHOD3(create_swapper_new_buffers,
         std::shared_ptr<compositor::BufferSwapper>(

=== modified file 'include/test/mir_test_doubles/stub_buffer_stream.h'
--- include/test/mir_test_doubles/stub_buffer_stream.h	2013-06-19 16:41:13 +0000
+++ include/test/mir_test_doubles/stub_buffer_stream.h	2013-06-25 19:32:30 +0000
@@ -61,6 +61,10 @@
     {
     }
 
+    void allow_framedropping(bool)
+    {
+    }
+
     std::shared_ptr<compositor::Buffer> stub_client_buffer;
     std::shared_ptr<compositor::Buffer> stub_compositor_buffer;
 };

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-06-20 09:43:30 +0000
+++ src/client/mir_client_library.cpp	2013-06-25 19:32:30 +0000
@@ -316,3 +316,15 @@
 
     return state;
 }
+
+MirWaitHandle* mir_surface_set_swapinterval(MirSurface* surf, int interval)
+{
+    if ((interval < 0) || (interval > 1))
+        return NULL;
+    return surf ? surf->configure(mir_surface_attrib_swapinterval, interval) : NULL;
+}
+
+int mir_surface_get_swapinterval(MirSurface* surf)
+{
+    return surf ? surf->attrib(mir_surface_attrib_swapinterval) : -1;
+}

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-06-20 09:43:30 +0000
+++ src/client/mir_surface.cpp	2013-06-25 19:32:30 +0000
@@ -58,6 +58,7 @@
         attrib_cache[i] = -1;
     attrib_cache[mir_surface_attrib_type] = mir_surface_type_normal;
     attrib_cache[mir_surface_attrib_state] = mir_surface_state_unknown;
+    attrib_cache[mir_surface_attrib_swapinterval] = 1;
 }
 
 MirSurface::~MirSurface()
@@ -269,6 +270,7 @@
         {
         case mir_surface_attrib_type:
         case mir_surface_attrib_state:
+        case mir_surface_attrib_swapinterval:
             if (configure_result.has_ivalue())
                 attrib_cache[a] = configure_result.ivalue();
             else

=== modified file 'src/server/compositor/buffer_stream_surfaces.cpp'
--- src/server/compositor/buffer_stream_surfaces.cpp	2013-06-19 16:41:13 +0000
+++ src/server/compositor/buffer_stream_surfaces.cpp	2013-06-25 19:32:30 +0000
@@ -63,3 +63,7 @@
     buffer_bundle->force_requests_to_complete();
 }
 
+void mc::BufferStreamSurfaces::allow_framedropping(bool allow)
+{
+    buffer_bundle->allow_framedropping(allow);
+}

=== modified file 'src/server/compositor/swapper_factory.cpp'
--- src/server/compositor/swapper_factory.cpp	2013-06-11 19:55:10 +0000
+++ src/server/compositor/swapper_factory.cpp	2013-06-25 19:32:30 +0000
@@ -36,7 +36,8 @@
         std::shared_ptr<GraphicBufferAllocator> const& gr_alloc,
         int number_of_buffers)
     : gr_allocator(gr_alloc),
-      number_of_buffers(number_of_buffers)
+      synchronous_number_of_buffers(number_of_buffers),
+      spin_number_of_buffers(3) //spin algorithm always takes 3 buffers
 {
 }
 
@@ -46,16 +47,43 @@
 {
 }
 
+void mc::SwapperFactory::change_swapper_size(
+    std::vector<std::shared_ptr<mc::Buffer>>& list,
+    size_t const desired_size, size_t current_size, BufferProperties const& buffer_properties) const
+{
+    while (current_size < desired_size)
+    {
+        list.push_back(gr_allocator->alloc_buffer(buffer_properties));
+        current_size++;
+    }
+ 
+    while (current_size > desired_size)
+    {
+        if (list.empty())
+        {
+            BOOST_THROW_EXCEPTION(std::logic_error("SwapperFactory could not change algorithm"));
+        }
+        else
+        {
+            list.pop_back();
+            current_size--;
+        }
+    }
+}
+
 std::shared_ptr<mc::BufferSwapper> mc::SwapperFactory::create_swapper_reuse_buffers(
-    std::vector<std::shared_ptr<Buffer>>& list, size_t buffer_num, SwapperType type) const
+    BufferProperties const& buffer_properties, std::vector<std::shared_ptr<Buffer>>& list,
+    size_t buffer_num, SwapperType type) const
 {
     if (type == mc::SwapperType::synchronous)
     {
-        return std::make_shared<mc::BufferSwapperMulti>(list, buffer_num); 
+        change_swapper_size(list, synchronous_number_of_buffers, buffer_num, buffer_properties);
+        return std::make_shared<mc::BufferSwapperMulti>(list, synchronous_number_of_buffers); 
     }
     else
     {
-        return std::make_shared<mc::BufferSwapperSpin>(list, buffer_num);
+        change_swapper_size(list, spin_number_of_buffers, buffer_num, buffer_properties);
+        return std::make_shared<mc::BufferSwapperSpin>(list, spin_number_of_buffers);
     }
 }
 
@@ -68,20 +96,19 @@
 
     if (type == mc::SwapperType::synchronous)
     {
-        for(auto i=0; i< number_of_buffers; i++)
+        for(auto i=0u; i< synchronous_number_of_buffers; i++)
         {
             list.push_back(gr_allocator->alloc_buffer(requested_buffer_properties));
         }
-        new_swapper = std::make_shared<mc::BufferSwapperMulti>(list, number_of_buffers);
+        new_swapper = std::make_shared<mc::BufferSwapperMulti>(list, synchronous_number_of_buffers);
     }
     else
     {
-        int const async_buffer_count = 3; //async only can accept 3 buffers, so ignore constructor request
-        for(auto i=0; i < async_buffer_count; i++)
+        for(auto i=0u; i < spin_number_of_buffers; i++)
         {
             list.push_back(gr_allocator->alloc_buffer(requested_buffer_properties));
         }
-        new_swapper = std::make_shared<mc::BufferSwapperSpin>(list, async_buffer_count);
+        new_swapper = std::make_shared<mc::BufferSwapperSpin>(list, spin_number_of_buffers);
     }
 
     actual_buffer_properties = BufferProperties{

=== modified file 'src/server/compositor/switching_bundle.cpp'
--- src/server/compositor/switching_bundle.cpp	2013-06-15 11:09:16 +0000
+++ src/server/compositor/switching_bundle.cpp	2013-06-25 19:32:30 +0000
@@ -92,9 +92,9 @@
     swapper->end_responsibility(list, size);
 
     if (allow_dropping)
-        swapper = swapper_factory->create_swapper_reuse_buffers(list, size, mc::SwapperType::framedropping); 
+        swapper = swapper_factory->create_swapper_reuse_buffers(bundle_properties, list, size, mc::SwapperType::framedropping);
     else
-        swapper = swapper_factory->create_swapper_reuse_buffers(list, size, mc::SwapperType::synchronous); 
+        swapper = swapper_factory->create_swapper_reuse_buffers(bundle_properties, list, size, mc::SwapperType::synchronous);
 
     cv.notify_all();
 }

=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp	2013-06-25 05:42:49 +0000
+++ src/server/shell/surface.cpp	2013-06-25 19:32:30 +0000
@@ -189,6 +189,14 @@
     }
 }
 
+void msh::Surface::allow_framedropping(bool allow)
+{
+    if (auto const& s = surface.lock())
+    {
+        s->allow_framedropping(allow);
+    }
+}
+ 
 void msh::Surface::with_most_recent_buffer_do(
     std::function<void(mc::Buffer&)> const& exec)
 {
@@ -242,7 +250,7 @@
 int msh::Surface::configure(MirSurfaceAttrib attrib, int value)
 {
     int result = 0;
-
+    bool allow_dropping = false;
     /*
      * TODO: In future, query the shell implementation for the subset of
      *       attributes/types it implements.
@@ -261,6 +269,11 @@
             BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface state."));
         result = state();
         break;
+    case mir_surface_attrib_swapinterval:
+        allow_dropping = (value == 0);
+        allow_framedropping(allow_dropping);
+        result = value;
+        break;
     default:
         BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
                                                "attribute."));

=== modified file 'src/server/surfaces/surface.cpp'
--- src/server/surfaces/surface.cpp	2013-06-25 05:42:49 +0000
+++ src/server/surfaces/surface.cpp	2013-06-25 19:32:30 +0000
@@ -176,6 +176,11 @@
     return buffer_stream->secure_client_buffer();
 }
 
+void ms::Surface::allow_framedropping(bool allow)
+{
+    buffer_stream->allow_framedropping(allow);
+}
+
 std::shared_ptr<mc::Buffer> ms::Surface::compositor_buffer() const
 {
     return buffer_stream->lock_back_buffer();

=== modified file 'tests/integration-tests/CMakeLists.txt'
--- tests/integration-tests/CMakeLists.txt	2013-06-25 03:25:49 +0000
+++ tests/integration-tests/CMakeLists.txt	2013-06-25 19:32:30 +0000
@@ -7,6 +7,7 @@
   test_display_info.cpp
   test_display_server_main_loop_events.cpp
   test_surface_first_frame_sync.cpp
+  test_swapinterval.cpp
 )
 
 add_subdirectory(client/)

=== added file 'tests/integration-tests/test_swapinterval.cpp'
--- tests/integration-tests/test_swapinterval.cpp	1970-01-01 00:00:00 +0000
+++ tests/integration-tests/test_swapinterval.cpp	2013-06-25 19:32:30 +0000
@@ -0,0 +1,226 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir/geometry/rectangle.h"
+#include "mir/graphics/display.h"
+#include "mir/graphics/display_buffer.h"
+#include "mir/graphics/renderer.h"
+#include "mir/graphics/renderable.h"
+#include "mir/compositor/compositor.h"
+#include "mir/compositor/compositing_strategy.h"
+#include "mir/compositor/renderables.h"
+#include "mir/surfaces/buffer_stream.h"
+#include "mir/surfaces/buffer_stream_factory.h"
+
+#include "mir_test_framework/display_server_test_fixture.h"
+#include "mir_test_doubles/stub_buffer.h"
+
+#include "mir_toolkit/mir_client_library.h"
+
+#include <gtest/gtest.h>
+
+#include <thread>
+#include <unistd.h>
+#include <fcntl.h>
+
+namespace geom = mir::geometry;
+namespace mg = mir::graphics;
+namespace mc = mir::compositor;
+namespace mtf = mir_test_framework;
+namespace ms = mir::surfaces;
+namespace mtd = mir::test::doubles;
+
+namespace
+{
+char const* const mir_test_socket = mtf::test_socket_file().c_str();
+
+class CountingBufferStream : public ms::BufferStream
+{
+public:
+    CountingBufferStream(int render_operations_fd)
+     : render_operations_fd(render_operations_fd)
+    {
+    }
+
+    std::shared_ptr<mc::Buffer> secure_client_buffer() { return std::make_shared<mtd::StubBuffer>(); }
+    std::shared_ptr<mc::Buffer> lock_back_buffer() { return std::make_shared<mtd::StubBuffer>(); }
+    geom::PixelFormat get_stream_pixel_format() { return geom::PixelFormat::abgr_8888; }
+    geom::Size stream_size() { return geom::Size{}; }
+    void force_requests_to_complete() {}
+    void allow_framedropping(bool)
+    {
+        while (write(render_operations_fd, "a", 1) != 1) continue;
+    }
+
+private:
+    int render_operations_fd; 
+};
+
+class StubStreamFactory : public ms::BufferStreamFactory
+{
+public:
+    StubStreamFactory(int render_operations_fd)
+     : render_operations_fd(render_operations_fd)
+    {
+    }
+
+    std::shared_ptr<ms::BufferStream> create_buffer_stream(mc::BufferProperties const&)
+    {
+        return std::make_shared<CountingBufferStream>(render_operations_fd);
+    }
+private:
+    int render_operations_fd; 
+};
+
+}
+
+using SwapIntervalSignalTest = BespokeDisplayServerTestFixture;
+TEST_F(SwapIntervalSignalTest, swapinterval_test)
+{
+    static std::string const swapinterval_set{"swapinterval_set_952f3f10.tmp"};
+    static std::string const do_client_finish{"do_client_finish_952f3f10.tmp"};
+
+    std::remove(swapinterval_set.c_str());
+    std::remove(do_client_finish.c_str());
+
+    struct ServerConfig : TestingServerConfiguration
+    {
+        ServerConfig()
+        {
+            if (pipe(rendering_ops_pipe) != 0)
+            {
+                BOOST_THROW_EXCEPTION(
+                    std::runtime_error("Failed to create pipe"));
+            }
+
+            if (fcntl(rendering_ops_pipe[0], F_SETFL, O_NONBLOCK) != 0)
+            {
+                BOOST_THROW_EXCEPTION(
+                    std::runtime_error("Failed to make the read end of the pipe non-blocking"));
+            }
+        }
+
+        ~ServerConfig()
+        {
+            if (rendering_ops_pipe[0] >= 0)
+                close(rendering_ops_pipe[0]);
+            if (rendering_ops_pipe[1] >= 0)
+                close(rendering_ops_pipe[1]);
+        }
+
+        std::shared_ptr<ms::BufferStreamFactory> the_buffer_stream_factory() override
+        {
+            if (!stub_stream_factory)
+                stub_stream_factory = std::make_shared<StubStreamFactory>(rendering_ops_pipe[1]);
+            return stub_stream_factory;
+        }
+
+        int num_of_swapinterval_commands()
+        {
+            char c;
+            int ops{0};
+
+            while (read(rendering_ops_pipe[0], &c, 1) == 1)
+                ops++;
+
+            return ops;
+        }
+
+        int rendering_ops_pipe[2];
+        std::shared_ptr<StubStreamFactory> stub_stream_factory;
+    } server_config;
+
+    launch_server_process(server_config);
+
+    struct ClientConfig : TestingClientConfiguration
+    {
+        ClientConfig(std::string const& swapinterval_set,
+                     std::string const& do_client_finish)
+            : swapinterval_set{swapinterval_set},
+              do_client_finish{do_client_finish}
+        {
+        }
+
+        static void surface_callback(MirSurface*, void*)
+        {
+        }
+
+        void exec()
+        {
+            MirSurfaceParameters request_params =
+            {
+                __PRETTY_FUNCTION__,
+                640, 480,
+                mir_pixel_format_abgr_8888,
+                mir_buffer_usage_hardware
+            };
+
+            MirConnection* connection = mir_connect_sync(mir_test_socket, "testapp");
+            MirSurface* surface = mir_connection_create_surface_sync(connection, &request_params);
+
+            //1 is the default swapinterval
+            EXPECT_EQ(1, mir_surface_get_swapinterval(surface));
+
+            mir_wait_for(mir_surface_set_swapinterval(surface, 0));
+            EXPECT_EQ(0, mir_surface_get_swapinterval(surface));
+
+            mir_wait_for(mir_surface_set_swapinterval(surface, 1));
+            EXPECT_EQ(1, mir_surface_get_swapinterval(surface));
+
+            //swapinterval 2 not supported
+            EXPECT_EQ(NULL, mir_surface_set_swapinterval(surface, 2));
+            EXPECT_EQ(1, mir_surface_get_swapinterval(surface)); 
+
+            set_flag(swapinterval_set);
+            wait_for(do_client_finish);
+
+            mir_surface_release_sync(surface);
+            mir_connection_release(connection);
+        }
+
+        /* TODO: Extract this flag mechanism and make it reusable */
+        void set_flag(std::string const& flag_file)
+        {
+            close(open(flag_file.c_str(), O_CREAT, S_IWUSR | S_IRUSR));
+        }
+
+        void wait_for(std::string const& flag_file)
+        {
+            int fd = -1;
+            while ((fd = open(flag_file.c_str(), O_RDONLY, S_IWUSR | S_IRUSR)) == -1)
+            {
+                std::this_thread::sleep_for(std::chrono::milliseconds(1));
+            }
+            close(fd);
+        }
+
+        std::string const swapinterval_set;
+        std::string const do_client_finish;
+    } client_config{swapinterval_set, do_client_finish};
+
+    launch_client_process(client_config);
+
+    run_in_test_process([&]
+    {
+        client_config.wait_for(swapinterval_set);
+
+        EXPECT_EQ(2, server_config.num_of_swapinterval_commands());
+
+        client_config.set_flag(do_client_finish);
+    });
+}

=== modified file 'tests/unit-tests/compositor/test_buffer_stream.cpp'
--- tests/unit-tests/compositor/test_buffer_stream.cpp	2013-06-17 15:55:04 +0000
+++ tests/unit-tests/compositor/test_buffer_stream.cpp	2013-06-25 19:32:30 +0000
@@ -122,3 +122,12 @@
 
     buffer_stream.secure_client_buffer();
 }
+
+TEST_F(BufferStreamTest, allow_framedropping_command)
+{
+    EXPECT_CALL(*mock_bundle, allow_framedropping(true))
+        .Times(1);
+
+    mc::BufferStreamSurfaces buffer_stream(mock_bundle);
+    buffer_stream.allow_framedropping(true);
+}

=== modified file 'tests/unit-tests/compositor/test_swapper_factory.cpp'
--- tests/unit-tests/compositor/test_swapper_factory.cpp	2013-06-11 19:55:10 +0000
+++ tests/unit-tests/compositor/test_swapper_factory.cpp	2013-06-25 19:32:30 +0000
@@ -147,6 +147,7 @@
 {
     using namespace testing;
 
+    mc::BufferProperties properties;
     std::vector<std::shared_ptr<mc::Buffer>> list {};
     size_t size = 3;
 
@@ -154,19 +155,66 @@
         .Times(0);
 
     mc::SwapperFactory strategy(mock_buffer_allocator);
-    auto swapper = strategy.create_swapper_reuse_buffers(list, size, mc::SwapperType::framedropping);
+    auto swapper = strategy.create_swapper_reuse_buffers(properties, list, size, mc::SwapperType::framedropping);
 }
 
 TEST_F(SwapperFactoryTest, create_sync_reuse)
 {
     using namespace testing;
 
+    mc::BufferProperties properties;
     std::vector<std::shared_ptr<mc::Buffer>> list;
     size_t size = 3;
 
     EXPECT_CALL(*mock_buffer_allocator, alloc_buffer(properties))
         .Times(0);
 
+    mc::SwapperFactory strategy(mock_buffer_allocator, 3);
+    auto swapper = strategy.create_swapper_reuse_buffers(properties, list, size, mc::SwapperType::synchronous);
+}
+
+TEST_F(SwapperFactoryTest, reuse_drop_unneeded_buffer)
+{
+    using namespace testing;
+
+    mc::SwapperFactory strategy(mock_buffer_allocator, 2);
+
+    auto buffer = std::make_shared<mtd::StubBuffer>();
+    {
+        size_t size = 3;
+        std::vector<std::shared_ptr<mc::Buffer>> list{buffer};
+
+        auto swapper = strategy.create_swapper_reuse_buffers(
+            properties, list, size, mc::SwapperType::synchronous);
+    }
+    EXPECT_EQ(1, buffer.use_count());
+}
+
+TEST_F(SwapperFactoryTest, reuse_drop_unneeded_buffer_error)
+{
+    using namespace testing;
+
+    mc::SwapperFactory strategy(mock_buffer_allocator, 2);
+
+    size_t size = 3;
+    std::vector<std::shared_ptr<mc::Buffer>> list{};
+
+    EXPECT_THROW({
+        strategy.create_swapper_reuse_buffers(
+            properties, list, size, mc::SwapperType::synchronous);
+    }, std::logic_error);
+}
+
+TEST_F(SwapperFactoryTest, reuse_alloc_additional_buffer_for_framedropping)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_buffer_allocator, alloc_buffer(_))
+        .Times(1);
     mc::SwapperFactory strategy(mock_buffer_allocator);
-    auto swapper = strategy.create_swapper_reuse_buffers(list, size, mc::SwapperType::synchronous);
+
+    size_t size = 2;
+    std::vector<std::shared_ptr<mc::Buffer>> list{};
+    auto swapper = strategy.create_swapper_reuse_buffers(
+        properties, list, size, mc::SwapperType::framedropping);
 }

=== modified file 'tests/unit-tests/compositor/test_switching_bundle.cpp'
--- tests/unit-tests/compositor/test_switching_bundle.cpp	2013-06-13 08:59:25 +0000
+++ tests/unit-tests/compositor/test_switching_bundle.cpp	2013-06-25 19:32:30 +0000
@@ -90,7 +90,7 @@
         .WillOnce(Return(stub_buffer));
     EXPECT_CALL(*mock_secondary_swapper, client_release(stub_buffer))
         .Times(1);
-    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,_))
+    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,_,_))
         .Times(1)
         .WillOnce(Return(mock_secondary_swapper));
 
@@ -126,7 +126,7 @@
         .WillOnce(Return(stub_buffer));
     EXPECT_CALL(*mock_secondary_swapper, compositor_release(stub_buffer))
         .Times(1);
-    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,_))
+    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,_,_))
         .Times(1)
         .WillOnce(Return(mock_secondary_swapper));
 
@@ -147,7 +147,7 @@
         .Times(1);
     EXPECT_CALL(*mock_default_swapper, end_responsibility(_,_))
         .Times(1);
-    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,mc::SwapperType::framedropping))
+    EXPECT_CALL(*mock_swapper_factory, create_swapper_reuse_buffers(_,_,_,mc::SwapperType::framedropping))
         .Times(1)
         .WillOnce(Return(mock_secondary_swapper));
 

=== modified file 'tests/unit-tests/surfaces/test_surface.cpp'
--- tests/unit-tests/surfaces/test_surface.cpp	2013-06-25 05:42:49 +0000
+++ tests/unit-tests/surfaces/test_surface.cpp	2013-06-25 19:32:30 +0000
@@ -445,6 +445,18 @@
     surf.force_requests_to_complete();
 }
 
+TEST_F(SurfaceCreation, test_surface_allow_framedropping)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_buffer_stream, allow_framedropping(true))
+        .Times(1);
+
+    ms::Surface surf{surface_name, geom::Point(), mock_buffer_stream,
+        std::shared_ptr<mi::InputChannel>(), mock_change_cb};
+    surf.allow_framedropping(true);
+}
+
 TEST_F(SurfaceCreation, test_surface_next_buffer_does_not_set_valid_until_second_frame)
 {
     ms::Surface surf{surface_name, geom::Point(), mock_buffer_stream,