[Spice-devel] [spice-gtk] Use cursor->data instead of row data

[Frediano Ziglio]

> 
> On Wed, 2017-05-31 at 06:13 -0400, Frediano Ziglio wrote:
> > > 
> > > Hi,
> > > 
> > > 'raw' data
> > > 
> > 
> > typo, thanks
> > 
> > > On Wed, 2017-05-31 at 09:43 +0100, Frediano Ziglio wrote:
> > > > data and cursor->data contains the same data (see the memcpy
> > > > above) but data is not aligned to uint32_t and require also
> > > > additional casts. Using cursor->data avoid these issues.
> > > > 
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > ---
> > > >  src/channel-cursor.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/channel-cursor.c b/src/channel-cursor.c
> > > > index cddba03..2610660 100644
> > > > --- a/src/channel-cursor.c
> > > > +++ b/src/channel-cursor.c
> > > > @@ -422,7 +422,7 @@ static display_cursor
> > > > *set_cursor(SpiceChannel
> > > > *channel, SpiceCursor *scursor)
> > > >          memcpy(cursor->data, data, size);
> > > >          for (i = 0; i < hdr->width * hdr->height; i++) {
> > > >              pix_mask = get_pix_mask(data, size, i);
> > > > -            if (pix_mask && *((guint32*)data + i) == 0xffffff)
> > > > {
> > > > +            if (pix_mask && cursor->data[i] == 0xffffff) {
> > > >                  cursor->data[i] = get_pix_hack(i, hdr->width);
> > > >              } else {
> > > >                  cursor->data[i] |= (pix_mask ? 0 : 0xff000000);
> > > 
> > > Is it needed for the other cases (COLOR16 and COLOR4)?
> > > looking at these... their "cursor->data[i] |= ..." does not make
> > > much
> > > sense
> > > 
> > > Thanks,
> > > Pavel
> > > 
> > > 
> > 
> > Looks like COLOR16 is quite broken. The pix_mask =
> > get_pix_mask(data, size, i);
> > call for COLOR16 uses a size which is computed using 32 bit instead
> > of 16 bit so
> > will point to invalid mask. Don't know how to test however. I
> > suspect some
> > 16 bit Windows client. And yes, the |= is also useless, however
> > cursor->data is
> > initialized with g_malloc0 so is basically an assignment (but is
> > confusing).
> > 
> > COLOR4 is completely a different case.
> > 
> > Beside that I think COLOR16 and COLOR4 are a bit OT considering this
> > patch.
> 
> it is a bit ot
> 
> I was referring to :
>             pix = *((guint16*)data + i);
>             if (pix_mask && pix == 0x7fff) {
> and
>             pix = *((uint32_t*)(data + size) + idx);
>             if (pix_mask && pix == 0xffffff) {
> 
> it is not much different from
>             if (pix_mask && *((guint32*)data + i) == 0xffffff) {
> 
> Pavel
> 

cursor->data and data are 2 areas of memory, in the COLOR32 you can
use cursor->data instead of data as you have the memcpy, in the
COLOR16 case you don't have the memcpy.

Frediano