On Mon, 2018-07-09 at 05:21 -0400, Frediano Ziglio wrote:
> >
> > Pass the pointer to X cursor struct directly instead of using a lambda
> > to fill in the pixels.
> >
> > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > ---
> > src/cursor-updater.cpp | 28 ++++++++++++++--------------
> > 1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/cursor-updater.cpp b/src/cursor-updater.cpp
> > index 8f65e83..5901172 100644
> > --- a/src/cursor-updater.cpp
> > +++ b/src/cursor-updater.cpp
> > @@ -23,16 +23,17 @@ 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)
> > +void send_cursor(StreamPort &stream_port, XFixesCursorImage *cursor)
> > {
> > - if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH || height >=
> > STREAM_MSG_CURSOR_SET_MAX_HEIGHT) {
> > + if (cursor->width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
> > + cursor->height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT)
> > + {
>
> style
Ok, so you left me guessing what the issue is, I suppose you want me to
do this?
if (cursor->width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
cursor->height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT) {
return;
}
Sure... but I'll not spare you the rant :)
This is much worse on the eyes and makes little sense to me, especially
along with the brace rules on functions and classes.
(I'm done :))
> > return;
> > }
> >
> > size_t cursor_size =
> > sizeof(StreamDevHeader) + sizeof(StreamMsgCursorSet) +
> > - width * height * sizeof(uint32_t);
> > + cursor->width * cursor->height * sizeof(uint32_t);
> > std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
> >
> > StreamDevHeader
> > &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
> > @@ -45,13 +46,15 @@ void send_cursor(StreamPort &stream_port, unsigned width,
> > unsigned height, int h
> > 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;
> > + cursor_msg.width = cursor->width;
> > + cursor_msg.height = cursor->height;
> > + cursor_msg.hot_spot_x = cursor->xhot;
> > + cursor_msg.hot_spot_y = cursor->yhot;
> >
> > uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
> > - fill_cursor(pixels);
> > + for (unsigned i = 0; i < cursor->width * cursor->height; ++i) {
> > + pixels[i] = cursor->pixels[i];
> > + }
> >
> > std::lock_guard<std::mutex> guard(stream_port.mutex);
> > stream_port.write(msg.get(), cursor_size);
> > @@ -95,11 +98,8 @@ CursorUpdater::CursorUpdater(StreamPort *stream_port) :
> > stream_port(stream_port)
> > }
> >
> > 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);
> > +
> > + send_cursor(*stream_port, cursor);
> > }
> > }
> >
>
> Why bounding send_cursor to a specific (X11) implementation?
We have no other implementation and the function is local, so it can be
made generic when and if needed, no real reason to make the code more
complex now.
If there was an elegant way to make the send_cursor method independent,
I'd use it, but I don't see one. I don't like the lambda much and I
remember c3d had an issue with it too, though I forgot what it was
exactly. This is a preparation patch for encapsulating the sending of
the messages which is mostly based on his work.
> Would not be better to add another overloaded send_cursor instead?
Not sure what you mean by overloaded send_cursor?
> I don't thing cursor can be nullptr, in this case I would use a reference
> instead, better constant, I don't see why we should change it.
Sure, I just used a pointer because that's what I had, and yeah, it
should be const.
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel