[Spice-devel] [RFC PATCH spice 0.8 18/19] client: display channel migration
Alon Levy
alevy at redhat.com
Tue Sep 20 09:12:01 PDT 2011
On Mon, Sep 19, 2011 at 12:47:11PM +0300, Yonit Halperin wrote:
>
Some nitpicks. One question on the added touch_context at the end.
> Signed-off-by: Yonit Halperin <yhalperi at redhat.com>
> ---
> client/display_channel.cpp | 158 ++++++++++++++++++++++++++++++++++++++++----
> client/display_channel.h | 12 +++-
> client/red_gl_canvas.cpp | 1 +
> 3 files changed, 155 insertions(+), 16 deletions(-)
>
> diff --git a/client/display_channel.cpp b/client/display_channel.cpp
> index a757183..aa4b52b 100644
> --- a/client/display_channel.cpp
> +++ b/client/display_channel.cpp
> @@ -84,6 +84,28 @@ private:
> DisplayChannel& _channel;
> };
>
> +class DestroyAllSurfacesEvent: public SyncEvent {
> +public:
> + DestroyAllSurfacesEvent(DisplayChannel& channel, bool include_primary = true)
> + : _channel(channel)
> + , _include_primary(include_primary)
> + {
> + }
> +
> + virtual void do_response(AbstractProcessLoop& events_loop)
> + {
> + if (_include_primary) {
> + _channel.do_destroy_all_surfaces();
> + } else {
> + _channel.do_destroy_off_screen_surfaces();
> + }
> + }
> +
> +private:
> + DisplayChannel& _channel;
> + bool _include_primary;
> +};
> +
> class CreateSurfaceEvent: public SyncEvent {
> public:
> CreateSurfaceEvent(DisplayChannel& channel, int surface_id, int width, int height,
> @@ -385,7 +407,6 @@ bool VideoStream::is_time_to_display(uint32_t now, uint32_t frame_time)
> void VideoStream::maintenance()
> {
> uint32_t mm_time = _client.get_mm_time();
> -
Should leave this line.
> remove_dead_frames(mm_time);
> if (!_update_mark && !_update_time && _frames_head != _frames_tail) {
> VideoFrame* tail = &_frames[frame_slot(_frames_tail)];
> @@ -412,6 +433,7 @@ void VideoStream::maintenance()
> }
> free_frame(_frames_tail++);
> }
> +
Should not add this line.
> }
>
> uint32_t VideoStream::handle_timer_update(uint32_t now)
> @@ -544,6 +566,21 @@ void ResetTimer::response(AbstractProcessLoop& events_loop)
> _client.deactivate_interval_timer(this);
> }
>
> +#define MIGRATION_PRIMARY_SURFACE_TIMEOUT (1000 * 5)
> +
> +class MigPrimarySurfaceTimer: public Timer {
> +public:
> + virtual void response(AbstractProcessLoop& events_loop)
> + {
> + DisplayChannel *channel = static_cast<DisplayChannel*>(events_loop.get_owner());
> + if (channel->_mig_wait_primary) {
> + channel->destroy_primary_surface();
> + channel->_mig_wait_primary = false;
> + }
> + channel->get_process_loop().deactivate_interval_timer(this);
Whitespace.
> + }
> +};
> +
> class DisplayHandler: public MessageHandlerImp<DisplayChannel, SPICE_CHANNEL_DISPLAY> {
> public:
> DisplayHandler(DisplayChannel& channel)
> @@ -618,11 +655,11 @@ DisplayChannel::~DisplayChannel()
> screen()->set_update_interrupt_trigger(NULL);
> }
>
> - //destroy_canvas(); fixme destroy all
> - destroy_strams();
> + destroy_streams();
> + do_destroy_all_surfaces();
> }
>
> -void DisplayChannel::destroy_strams()
> +void DisplayChannel::destroy_streams()
> {
> Lock lock(_streams_lock);
> for (unsigned int i = 0; i < _streams.size(); i++) {
> @@ -1021,6 +1058,75 @@ void DisplayChannel::on_disconnect()
> (*sync_event)->wait();
> }
>
> +void DisplayChannel::do_destroy_all_surfaces()
> +{
> + SurfacesCache::iterator s_iter;
> +
> + for (s_iter = _surfaces_cache.begin(); s_iter != _surfaces_cache.end(); s_iter++) {
> + delete (*s_iter).second;
> + }
> + _surfaces_cache.clear();
> +}
> +
> +void DisplayChannel::do_destroy_off_screen_surfaces()
> +{
> + SurfacesCache::iterator s_iter;
> + Canvas *primary_canvas = NULL;
> +
> + for (s_iter = _surfaces_cache.begin(); s_iter != _surfaces_cache.end(); s_iter++) {
> + if (s_iter->first == 0) {
> + primary_canvas = s_iter->second;
> + } else {
> + delete s_iter->second;
> + }
> + }
> + _surfaces_cache.clear();
> + if (primary_canvas) {
> + _surfaces_cache[0] = primary_canvas;
> + }
> +}
> +
> +void DisplayChannel::destroy_all_surfaces()
> +{
> + AutoRef<DestroyAllSurfacesEvent> destroy_event(new DestroyAllSurfacesEvent(*this));
> +
> + get_client().push_event(*destroy_event);
> + (*destroy_event)->wait();
> + if (!(*destroy_event)->success()) {
> + THROW("destroy all surfaces failed");
> + }
> +}
> +
> +void DisplayChannel::destroy_off_screen_surfaces()
> +{
> + AutoRef<DestroyAllSurfacesEvent> destroy_event(new DestroyAllSurfacesEvent(*this, false));
> +
> + get_client().push_event(*destroy_event);
> + (*destroy_event)->wait();
> + if (!(*destroy_event)->success()) {
> + THROW("destroy all surfaces failed");
> + }
> +}
> +
> +void DisplayChannel::on_disconnect_mig_src()
> +{
> + _palette_cache.clear();
> + destroy_streams();
> + if (screen()) {
> + screen()->set_update_interrupt_trigger(NULL);
> + }
> + _update_mark = 0;
> + _next_timer_time = 0;
> + get_client().deactivate_interval_timer(*_streams_timer);
> + destroy_off_screen_surfaces();
> + // Not clrearing the primary surface till we receive a new one (or a timeout).
typo, clearing.
> + if (_surfaces_cache.exist(0)) {
> + AutoRef<MigPrimarySurfaceTimer> mig_timer(new MigPrimarySurfaceTimer());
> + get_process_loop().activate_interval_timer(*mig_timer, MIGRATION_PRIMARY_SURFACE_TIMEOUT);
> + _mig_wait_primary = true;
> + }
> +}
> +
> bool DisplayChannel::create_sw_canvas(int surface_id, int width, int height, uint32_t format)
> {
> try {
> @@ -1355,26 +1461,50 @@ void DisplayChannel::handle_stream_destroy(RedPeer::InMessage* message)
>
> void DisplayChannel::handle_stream_destroy_all(RedPeer::InMessage* message)
> {
> - destroy_strams();
> + destroy_streams();
> }
>
> void DisplayChannel::create_primary_surface(int width, int height, uint32_t format)
> {
> + bool do_create_primary = true;
> #ifdef USE_OGL
> - Canvas *canvas;
> + Canvas *canvas;
> #endif
> - _mark = false;
> - attach_to_screen(get_client().get_application(), get_id());
> - clear_area();
> + _mark = false;
> +
> + /*
> + * trying to avoid artifacts when the display hasn't changed much
> + * between the disconnection from the migration src and the
> + * connection to the target.
> + */
> + if (_mig_wait_primary) {
> + ASSERT(_surfaces_cache.exist(0));
> + if (_x_res != width || _y_res != height || format != format) {
> + LOG_INFO("destroy the primary surface of the mig src session");
> + destroy_primary_surface();
> + } else {
> + LOG_INFO("keep the primary surface of the mig src session");
> + _surfaces_cache[0]->clear();
> + clear_area();
> + do_create_primary = false;
> + }
> + }
whitespace at the end of the line.
>
> - AutoRef<CreatePrimarySurfaceEvent> event(new CreatePrimarySurfaceEvent(*this, width, height,
> + if (do_create_primary) {
> + LOG_INFO("");
> + attach_to_screen(get_client().get_application(), get_id());
> + clear_area();
> + AutoRef<CreatePrimarySurfaceEvent> event(new CreatePrimarySurfaceEvent(*this, width, height,
> format));
> - get_client().push_event(*event);
> - (*event)->wait();
> - if (!(*event)->success()) {
> - THROW("Create primary surface failed");
> + get_client().push_event(*event);
> + (*event)->wait();
> + if (!(*event)->success()) {
> + THROW("Create primary surface failed");
> + }
> }
>
> + _mig_wait_primary = false;
> +
> _x_res = width;
> _y_res = height;
> _format = format;
> diff --git a/client/display_channel.h b/client/display_channel.h
> index cdad5ff..755bf25 100644
> --- a/client/display_channel.h
> +++ b/client/display_channel.h
> @@ -114,6 +114,7 @@ public:
> protected:
> virtual void on_connect();
> virtual void on_disconnect();
> + virtual void on_disconnect_mig_src();
>
> private:
> void set_draw_handlers();
> @@ -129,13 +130,17 @@ private:
> void destroy_canvas(int surface_id);
> void create_canvas(int surface_id, const std::vector<int>& canvas_type, int width, int height,
> uint32_t format);
> - void destroy_strams();
> + void destroy_streams();
> void update_cursor();
>
> void create_primary_surface(int width, int height, uint32_t format);
> void create_surface(int surface_id, int width, int height, uint32_t format);
> void destroy_primary_surface();
> void destroy_surface(int surface_id);
> + void destroy_all_surfaces();
> + void do_destroy_all_surfaces();
> + void destroy_off_screen_surfaces();
> + void do_destroy_off_screen_surfaces();
>
> void handle_mode(RedPeer::InMessage* message);
> void handle_mark(RedPeer::InMessage* message);
> @@ -217,11 +222,13 @@ private:
> #endif
> InterruptUpdate _interrupt_update;
>
> + bool _mig_wait_primary;
> friend class SetModeEvent;
> friend class CreatePrimarySurfaceEvent;
> - friend class DestroyPrimarySurfaceEvent;
> + friend class DestroyPrimarySurfaceEvent;
Whitespace at the end of the line.
> friend class CreateSurfaceEvent;
> friend class DestroySurfaceEvent;
> + friend class DestroyAllSurfacesEvent;
> friend class ActivateTimerEvent;
> friend class VideoStream;
> friend class StreamsTrigger;
> @@ -229,6 +236,7 @@ private:
> friend class StreamsTimer;
> friend class AttachChannelsEvent;
> friend class DetachChannelsEvent;
> + friend class MigPrimarySurfaceTimer;
> };
>
> #endif
> diff --git a/client/red_gl_canvas.cpp b/client/red_gl_canvas.cpp
> index e2bff7f..78bf272 100644
> --- a/client/red_gl_canvas.cpp
> +++ b/client/red_gl_canvas.cpp
> @@ -49,6 +49,7 @@ GCanvas::GCanvas(int width, int height, uint32_t format, RedWindow *win,
>
> GCanvas::~GCanvas()
> {
> + touch_context();
Huh?
> gl_canvas_set_textures_lost (_canvas, (int)_textures_lost);
> _canvas->ops->destroy(_canvas);
> _canvas = NULL;
> --
> 1.7.4.4
>
More information about the Spice-devel
mailing list