[Spice-devel] [PATCH spice-streaming-agent v4 1/2] Move out the cursor-updating code into it's own class
Lukáš Hrázký
lhrazky at redhat.com
Mon Jul 9 10:31:35 UTC 2018
On Mon, 2018-07-09 at 05:18 -0400, Frediano Ziglio wrote:
> >
> > 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.
I'm not so sure about this, from my limited experience client-side
cursor is much smoother and more responsive than server-side (or rather
guest-side in this case).
> Not sure if this patch helps or create more issues but
> I don't think introduce much regression on this.
Any issues on your mind? In my opinion this patch makes it easier, if
anything, to enable this conditionally.
Cheers,
Lukas
> 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