Merge lp:~alan-griffiths/mir/fix-1308133 into lp:mir

[Alan Griffiths]

scene: use the cursor stream for a surface correctly by extracting frames for "composition", not "snapshotting" the last composited frame.
(LP: #1308133)

scene: use the cursor stream for a surface correctly by extracting frames for "composition", not "snapshotting" the last composited frame.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2977
http://jenkins.qa.ubuntu.com/job/mir-ci/5061/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3087
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4122
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/80/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1212
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1212/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/80/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4123
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4123/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6802
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23766
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/81/console

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Daniel van Vugt (vanvugt)]

Oh wow. Such a silly mistake.

[Daniel van Vugt (vanvugt)]

Confirmed; this fixes the missing cursor in Xmir. In fact this fixes it better than the workaround I have in Xmir.

review: Approve

=== modified file 'examples/animated_cursor_demo_client.c'
--- examples/animated_cursor_demo_client.c	2015-02-25 22:10:35 +0000
+++ examples/animated_cursor_demo_client.c	2015-09-29 16:26:37 +0000
@@ -52,7 +52,9 @@
 {
     MirBufferStream* stream = mir_connection_create_buffer_stream_sync(connection,
       24, 24, mir_pixel_format_argb_8888, mir_buffer_usage_software);
-    
+
+    animate_cursor(stream);
+
     MirCursorConfiguration* conf = mir_cursor_configuration_from_buffer_stream(stream, 0, 0);
     mir_wait_for(mir_surface_configure_cursor(surface, conf));
     mir_cursor_configuration_destroy(conf);

=== modified file 'src/server/graphics/nested/mir_client_host_connection.cpp'
--- src/server/graphics/nested/mir_client_host_connection.cpp	2015-09-25 16:23:50 +0000
+++ src/server/graphics/nested/mir_client_host_connection.cpp	2015-09-29 16:26:37 +0000
@@ -130,12 +130,6 @@
         {
             cursor_hotspot = image.hotspot();
 
-            // push an extra frame for host to display correctly
-            // TODO remove this workaround for lp:1308133
-            mir_buffer_stream_get_graphics_region(cursor, &g);
-            memcpy(g.vaddr, image.as_argb_8888(), pixels_size);
-            mir_buffer_stream_swap_buffers_sync(cursor);
-
             auto conf = mir_cursor_configuration_from_buffer_stream(
                 cursor, cursor_hotspot.dx.as_int(), cursor_hotspot.dy.as_int());
 

=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp	2015-07-27 07:24:50 +0000
+++ src/server/scene/basic_surface.cpp	2015-09-29 16:26:37 +0000
@@ -124,6 +124,73 @@
         { observer->renamed(name); });
 }
 
+struct ms::CursorStreamImageAdapter
+{
+    CursorStreamImageAdapter(ms::BasicSurface &surface)
+        : surface(surface),
+          observer{std::make_shared<FramePostObserver>(this)}
+    {
+    }
+
+    ~CursorStreamImageAdapter()
+    {
+        reset();
+    }
+
+    void reset()
+    {
+        if (stream)
+        {
+            stream->remove_observer(observer);
+            stream.reset();
+        }
+    }
+
+    void update(std::shared_ptr<mf::BufferStream> const& new_stream, geom::Displacement const& new_hotspot)
+    {
+        if (new_stream == stream && new_hotspot == hotspot)
+        {
+            return;
+        }
+        else if (new_stream != stream)
+        {
+            if (stream) stream->remove_observer(observer);
+
+            stream = std::dynamic_pointer_cast<mc::BufferStream>(new_stream);
+            stream->add_observer(observer);
+        }
+
+        hotspot = new_hotspot;
+        post_cursor_image_from_current_buffer();
+    }
+
+private:
+    struct FramePostObserver : public ms::NullSurfaceObserver
+    {
+        FramePostObserver(CursorStreamImageAdapter const* self)
+            : self(self)
+        {
+        }
+
+        void frame_posted(int /* available */)
+        {
+            self->post_cursor_image_from_current_buffer();
+        }
+
+        CursorStreamImageAdapter const* const self;
+    };
+
+    void post_cursor_image_from_current_buffer() const
+    {
+        surface.set_cursor_from_buffer(*stream->lock_compositor_buffer(this), hotspot);
+    }
+
+    ms::BasicSurface& surface;
+    std::shared_ptr<FramePostObserver> const observer;
+
+    std::shared_ptr<mc::BufferStream> stream;
+    geom::Displacement hotspot;
+};
 
 ms::BasicSurface::BasicSurface(
     std::string const& name,
@@ -149,6 +216,7 @@
     report(report),
     parent_(parent),
     layers({StreamInfo{buffer_stream, {0,0}}}),
+    cursor_stream_adapter{std::make_unique<ms::CursorStreamImageAdapter>(*this)},
     input_validator([this](MirEvent const& ev) { this->input_sender->send_event(ev, server_input_channel); })
 {
     report->surface_created(this, surface_name);
@@ -540,7 +608,7 @@
 {
     {
         std::unique_lock<std::mutex> lock(guard);
-        cursor_stream_adapter.reset();
+        cursor_stream_adapter->reset();
         
         cursor_image_ = image;
     }
@@ -556,20 +624,6 @@
 
 namespace
 {
-struct FramePostObserver : public ms::NullSurfaceObserver
-{
-    FramePostObserver(std::function<void()> const& exec)
-        : exec(exec)
-    {
-    }
-
-    void frame_posted(int /* available */)
-    {
-        exec();
-    }
-    std::function<void()> const exec;
-};
-
 struct CursorImageFromBuffer : public mg::CursorImage
 {
     CursorImageFromBuffer(mg::Buffer &buffer, geom::Displacement const& hotspot)
@@ -608,45 +662,6 @@
 };
 }
 
-namespace mir
-{
-namespace scene
-{
-struct CursorStreamImageAdapter
-{
-    CursorStreamImageAdapter(ms::BasicSurface &surface, std::shared_ptr<mf::BufferStream> const& stream,
-                             geom::Displacement const& hotspot)
-        : surface(surface),
-          stream(stream),
-          observer{std::make_shared<FramePostObserver>(
-            [this](){ post_cursor_image_from_current_buffer(); })},
-          hotspot(hotspot)
-    {
-        stream->add_observer(observer);
-    }
-
-    ~CursorStreamImageAdapter()
-    {
-        stream->remove_observer(observer);
-    }
-
-    void post_cursor_image_from_current_buffer()
-    {
-        stream->with_most_recent_buffer_do([&](mg::Buffer &buffer)
-            {
-                surface.set_cursor_from_buffer(buffer, hotspot);
-            });
-    }
-
-    ms::BasicSurface &surface;
-
-    std::shared_ptr<mf::BufferStream> const stream;
-    std::shared_ptr<FramePostObserver> observer;
-    geom::Displacement const hotspot;
-}; 
-}
-}
-
 void ms::BasicSurface::set_cursor_from_buffer(mg::Buffer& buffer, geom::Displacement const& hotspot)
 {
     auto image = std::make_shared<CursorImageFromBuffer>(buffer, hotspot);
@@ -667,12 +682,7 @@
 void ms::BasicSurface::set_cursor_stream(std::shared_ptr<mf::BufferStream> const& stream,
                                          geom::Displacement const& hotspot)
 {
-    std::unique_lock<std::mutex> lock(guard);
-
-    cursor_stream_adapter = std::make_unique<ms::CursorStreamImageAdapter>(*this, stream, hotspot);
-    stream->with_most_recent_buffer_do([this, &hotspot](mg::Buffer& buffer) {
-        cursor_image_ = std::make_shared<CursorImageFromBuffer>(buffer, hotspot); 
-    });
+    cursor_stream_adapter->update(stream, hotspot);
 }
 
 void ms::BasicSurface::request_client_surface_close()

=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h	2015-07-23 02:39:20 +0000
+++ src/server/scene/basic_surface.h	2015-09-29 16:26:37 +0000
@@ -184,7 +184,7 @@
     MirSurfaceVisibility visibility_ = mir_surface_visibility_exposed;
     MirOrientationMode pref_orientation_mode = mir_orientation_mode_any;
 
-    std::unique_ptr<CursorStreamImageAdapter> cursor_stream_adapter;
+    std::unique_ptr<CursorStreamImageAdapter> const cursor_stream_adapter;
 
     input::Validator input_validator;
 };