[Spice-devel] [PATCH 4/6] LZ4: Fix the row alignment when it is not on a 32bit boundary

[Christophe Fergeau]

On Mon, Feb 02, 2015 at 10:11:32AM +0100, Javier Celaya wrote:
> Hello
> 
> El Viernes, 30 de enero de 2015 11:36:08 Christophe Fergeau escribió:
> > On Wed, Jan 28, 2015 at 11:49:47AM +0100, Javier Celaya wrote:
> > > Fix the row alignment for 16/24 bpp images when it is not in a 32bit
> > > boundary. This is needed for 16bpp images when the width is an odd
> > > number, and for the future support of 24bpp images.
> > > ---
> > > @@ -620,6 +623,21 @@ static pixman_image_t *canvas_get_lz4(CanvasBase
> > > *canvas, SpiceImage *image, int> 
> > >          data += enc_size;
> > >      
> > >      } while (data < data_end);
> > > 
> > > +    if (stride_abs > stride_encoded) {
> > > +        // Fix the row alignment
> > > +        int row;
> > > +        uint32_t *dest_aligned, *dest_misaligned;
> > > +        dest = (uint8_t *)pixman_image_get_data(surface);
> > > +        if (!top_down) {
> > > +            dest -= (stride_abs * (height - 1));
> > > +        }
> > > +        for (row = height - 1; row > 0; --row) {
> > > +            dest_aligned = (uint32_t *)(dest + stride_abs*row);
> > > +            dest_misaligned = (uint32_t*)(dest + stride_encoded*row);
> > > +            memmove(dest_aligned, dest_misaligned, stride_encoded);
> > > +        }
> > > +    }
> > > +
> > 
> > Looking at common/canvas_utils.c:surface_create, we don't seem to align
> > rows when top_down is TRUE, it seems this code will need to be more
> > careful in this case :(
> > 
> > Christophe
> 
> IMHO, rows should be aligned whether the image is top-down or bottom-up. In 
> common/canvas_utils.c:surface_create, pixman_image_create_bits is called with 
> an aligned stride for bottom-up images. Pixman checks it in pixman-bits-
> image.c, line 996, in function create_bits_image_internal, which is called by 
> pixman_image_create_bits:
> 
> http://cgit.freedesktop.org/pixman/tree/pixman/pixman-bits-image.c#n996
> 
> But for top-down images, pixman_image_create_bits is called with a null buffer 
> and a zero stride. Then, pixman allocates the buffer and computes an aligned 
> stride by itself, in function create_bits:
> 
> http://cgit.freedesktop.org/pixman/tree/pixman/pixman-bits-image.c#n898

Yup, I didn't look deep enough in the code, I missed the rounding done
by pixman :)

Patch looks good then.

Christophe

> 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150202/7886948c/attachment.sig>