[Spice-devel] [PATCH spice-streaming-agent v4 1/2] Move out the cursor-updating code into it's own class

Frediano Ziglio fziglio at redhat.com
Mon Jul 9 09:18:13 UTC 2018


> 
> Wraps the code in a functor which can then be used in std::thread. Keeps
> the call to std::thread::detach() for now, as the thread actually cannot
> be properly joined. It spends most of the time blocking on XNextEvent()
> and there is no way of interrupting it.
> 
> It seems XNextEvent() is acutally using a socket, so a proper solution
> might be to integrate this into the loop of the main process.
> 
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>

Acked-by: Frediano Ziglio <fziglio at redhat.com>

This part of the code is getting maybe too "importance".
Not all settings potentially will use this part of the code.
This part is useful to support client mouse if the plugin is not
able to capture the cursor shape but on other cases (like for instance
if the capture came with the cursor) is useless and should be
disabled. Not sure if this patch helps or create more issues but
I don't think introduce much regression on this.

Frediano

> ---
>  src/Makefile.am               |   2 +
>  src/cursor-updater.cpp        | 106 ++++++++++++++++++++++++++++++++++
>  src/cursor-updater.hpp        |  34 +++++++++++
>  src/spice-streaming-agent.cpp |  84 +--------------------------
>  4 files changed, 145 insertions(+), 81 deletions(-)
>  create mode 100644 src/cursor-updater.cpp
>  create mode 100644 src/cursor-updater.hpp
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index fead680..40544ba 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -52,6 +52,8 @@ spice_streaming_agent_SOURCES = \
>  	spice-streaming-agent.cpp \
>  	concrete-agent.cpp \
>  	concrete-agent.hpp \
> +	cursor-updater.cpp \
> +	cursor-updater.hpp \
>  	error.cpp \
>  	error.hpp \
>  	frame-log.cpp \
> diff --git a/src/cursor-updater.cpp b/src/cursor-updater.cpp
> new file mode 100644
> index 0000000..8f65e83
> --- /dev/null
> +++ b/src/cursor-updater.cpp
> @@ -0,0 +1,106 @@
> +/* A class that monitors X11 cursor changes and sends the cursor image over
> the
> + * streaming virtio port.
> + *
> + * \copyright
> + * Copyright 2016-2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include "cursor-updater.hpp"
> +
> +#include "error.hpp"
> +
> +#include <spice/stream-device.h>
> +#include <spice/enums.h>
> +
> +#include <cstring>
> +#include <functional>
> +#include <memory>
> +#include <X11/extensions/Xfixes.h>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +namespace {
> +
> +void send_cursor(StreamPort &stream_port, unsigned width, unsigned height,
> int hotspot_x, int hotspot_y,
> +                 std::function<void(uint32_t *)> fill_cursor)
> +{
> +    if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH || height >=
> STREAM_MSG_CURSOR_SET_MAX_HEIGHT) {
> +        return;
> +    }
> +
> +    size_t cursor_size =
> +        sizeof(StreamDevHeader) + sizeof(StreamMsgCursorSet) +
> +        width * height * sizeof(uint32_t);
> +    std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
> +
> +    StreamDevHeader
> &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
> +    memset(&dev_hdr, 0, sizeof(dev_hdr));
> +    dev_hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> +    dev_hdr.type = STREAM_TYPE_CURSOR_SET;
> +    dev_hdr.size = cursor_size - sizeof(StreamDevHeader);
> +
> +    StreamMsgCursorSet &cursor_msg(*reinterpret_cast<StreamMsgCursorSet
> *>(msg.get() + sizeof(StreamDevHeader)));
> +    memset(&cursor_msg, 0, sizeof(cursor_msg));
> +
> +    cursor_msg.type = SPICE_CURSOR_TYPE_ALPHA;
> +    cursor_msg.width = width;
> +    cursor_msg.height = height;
> +    cursor_msg.hot_spot_x = hotspot_x;
> +    cursor_msg.hot_spot_y = hotspot_y;
> +
> +    uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
> +    fill_cursor(pixels);
> +
> +    std::lock_guard<std::mutex> guard(stream_port.mutex);
> +    stream_port.write(msg.get(), cursor_size);
> +}
> +
> +} // namespace
> +
> +CursorUpdater::CursorUpdater(StreamPort *stream_port) :
> stream_port(stream_port)
> +{
> +    display = XOpenDisplay(nullptr);
> +    if (display == nullptr) {
> +        throw Error("Failed to open X display");
> +    }
> +
> +    int error_base;
> +    if (!XFixesQueryExtension(display, &xfixes_event_base, &error_base)) {
> +        throw Error("XFixesQueryExtension failed");
> +    }
> +
> +    XFixesSelectCursorInput(display, DefaultRootWindow(display),
> XFixesDisplayCursorNotifyMask);
> +}
> +
> +[[noreturn]] void CursorUpdater::operator()()
> +{
> +    unsigned long last_serial = 0;
> +
> +    while (1) {
> +        XEvent event;
> +        XNextEvent(display, &event);
> +        if (event.type != xfixes_event_base + 1) {
> +            continue;
> +        }
> +
> +        XFixesCursorImage *cursor = XFixesGetCursorImage(display);
> +        if (!cursor) {
> +            continue;
> +        }
> +
> +        if (cursor->cursor_serial == last_serial) {
> +            continue;
> +        }
> +
> +        last_serial = cursor->cursor_serial;
> +        auto fill_cursor = [cursor](uint32_t *pixels) {
> +            for (unsigned i = 0; i < cursor->width * cursor->height; ++i)
> +                pixels[i] = cursor->pixels[i];
> +        };
> +        send_cursor(*stream_port, cursor->width, cursor->height,
> cursor->xhot, cursor->yhot, fill_cursor);
> +    }
> +}
> +
> +}} // namespace spice::streaming_agent
> diff --git a/src/cursor-updater.hpp b/src/cursor-updater.hpp
> new file mode 100644
> index 0000000..d5f00af
> --- /dev/null
> +++ b/src/cursor-updater.hpp
> @@ -0,0 +1,34 @@
> +/* A class that monitors X11 cursor changes and sends the cursor image over
> the
> + * streaming virtio port.
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#ifndef SPICE_STREAMING_AGENT_CURSOR_UPDATER_HPP
> +#define SPICE_STREAMING_AGENT_CURSOR_UPDATER_HPP
> +
> +#include "stream-port.hpp"
> +
> +#include <X11/Xlib.h>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +class CursorUpdater
> +{
> +public:
> +    CursorUpdater(StreamPort *stream_port);
> +
> +    void operator()();
> +
> +private:
> +    StreamPort *stream_port;
> +    Display *display;  // the X11 display
> +    int xfixes_event_base;  // event number for the XFixes events
> +};
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_CURSOR_UPDATER_HPP
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 2e93049..4103720 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -6,6 +6,7 @@
>  
>  #include "concrete-agent.hpp"
>  #include "mjpeg-fallback.hpp"
> +#include "cursor-updater.hpp"
>  #include "frame-log.hpp"
>  #include "stream-port.hpp"
>  #include "error.hpp"
> @@ -35,9 +36,6 @@
>  #include <thread>
>  #include <vector>
>  #include <string>
> -#include <functional>
> -#include <X11/Xlib.h>
> -#include <X11/extensions/Xfixes.h>
>  
>  using namespace spice::streaming_agent;
>  
> @@ -253,70 +251,6 @@ static void usage(const char *progname)
>      exit(1);
>  }
>  
> -static void
> -send_cursor(StreamPort &stream_port, unsigned width, unsigned height, int
> hotspot_x, int hotspot_y,
> -            std::function<void(uint32_t *)> fill_cursor)
> -{
> -    if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH || height >=
> STREAM_MSG_CURSOR_SET_MAX_HEIGHT) {
> -        return;
> -    }
> -
> -    size_t cursor_size =
> -        sizeof(StreamDevHeader) + sizeof(StreamMsgCursorSet) +
> -        width * height * sizeof(uint32_t);
> -    std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
> -
> -    StreamDevHeader
> &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
> -    memset(&dev_hdr, 0, sizeof(dev_hdr));
> -    dev_hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
> -    dev_hdr.type = STREAM_TYPE_CURSOR_SET;
> -    dev_hdr.size = cursor_size - sizeof(StreamDevHeader);
> -
> -    StreamMsgCursorSet &cursor_msg(*reinterpret_cast<StreamMsgCursorSet
> *>(msg.get() + sizeof(StreamDevHeader)));
> -    memset(&cursor_msg, 0, sizeof(cursor_msg));
> -
> -    cursor_msg.type = SPICE_CURSOR_TYPE_ALPHA;
> -    cursor_msg.width = width;
> -    cursor_msg.height = height;
> -    cursor_msg.hot_spot_x = hotspot_x;
> -    cursor_msg.hot_spot_y = hotspot_y;
> -
> -    uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
> -    fill_cursor(pixels);
> -
> -    std::lock_guard<std::mutex> guard(stream_port.mutex);
> -    stream_port.write(msg.get(), cursor_size);
> -}
> -
> -static void cursor_changes(StreamPort *stream_port, Display *display, int
> event_base)
> -{
> -    unsigned long last_serial = 0;
> -
> -    while (1) {
> -        XEvent event;
> -        XNextEvent(display, &event);
> -        if (event.type != event_base + 1) {
> -            continue;
> -        }
> -
> -        XFixesCursorImage *cursor = XFixesGetCursorImage(display);
> -        if (!cursor) {
> -            continue;
> -        }
> -
> -        if (cursor->cursor_serial == last_serial) {
> -            continue;
> -        }
> -
> -        last_serial = cursor->cursor_serial;
> -        auto fill_cursor = [cursor](uint32_t *pixels) {
> -            for (unsigned i = 0; i < cursor->width * cursor->height; ++i)
> -                pixels[i] = cursor->pixels[i];
> -        };
> -        send_cursor(*stream_port, cursor->width, cursor->height,
> cursor->xhot, cursor->yhot, fill_cursor);
> -    }
> -}
> -
>  static void
>  do_capture(StreamPort &stream_port, FrameLog &frame_log)
>  {
> @@ -474,22 +408,10 @@ int main(int argc, char* argv[])
>          }
>          old_args.clear();
>  
> -        Display *display = XOpenDisplay(NULL);
> -        if (display == NULL) {
> -            throw Error("Failed to open X display");
> -        }
> -
> -        int event_base, error_base;
> -        if (!XFixesQueryExtension(display, &event_base, &error_base)) {
> -            throw Error("XFixesQueryExtension failed");
> -        }
> -        Window rootwindow = DefaultRootWindow(display);
> -        XFixesSelectCursorInput(display, rootwindow,
> XFixesDisplayCursorNotifyMask);
> -
>          StreamPort stream_port(stream_port_name);
>  
> -        std::thread cursor_th(cursor_changes, &stream_port, display,
> event_base);
> -        cursor_th.detach();
> +        std::thread cursor_updater{CursorUpdater(&stream_port)};
> +        cursor_updater.detach();
>  
>          do_capture(stream_port, frame_log);
>      }


More information about the Spice-devel mailing list