[Mesa-dev] [PATCH] gallium/dri2: Avoid uneeded stride to pitch conversion
Nicolas Dufresne
nicolas.dufresne at collabora.com
Tue Jan 5 09:00:27 PST 2016
Le mardi 05 janvier 2016 à 17:19 +0100, Axel Davy a écrit :
> Hi Nicolas,
>
> I believe this patch doesn't fully solve the problem and could
> introduce some bugs.
>
> For example you change dri2_create_image_from_name pitch argument to
> stride,
> whereas this function is used as is to implement createImageFromName
> from __DRIimageExtensionRec,
> which takes a pitch. That said for this very function I'm not sure it
> is used correctly by the other parts of mesa,
> for example in dri2_create_image_mesa_drm_buffer it seems to pass a
> stride ...
That was not intentional, I didn't expect to change anything in the
interface. I'll fix this and resubmit.
>
> It is a good idea to clean the pitch/stride confusions that can occur
> in the code, but you should fix
> all occurences, not just some subset. I mean you'll have to change
> __DRIimageExtensionRec spec,
> adapt all users and implementers.
>
> I guess piglit can be used to check for regressions
I'll look into this. There is no obvious confusing
in __DRIimageExtensionRec, so nothing to fix for now. There might be
confusing elsewhere in dri2.c (just grep for pitch = stride), though
I'm trying to focus on fixing importation of RGB565 dmabuf first, as
it's made very visible within GStreamer now, but was concerned not to
add extra conversion here.
thanks,
Nicolas
>
> Yours,
>
> Axel
>
> On 05/01/2016 16:58, Nicolas Dufresne wrote:
> > Le me know if this patch needs an update.
> >
> > cheers,
> > Nicolas
> >
> > Le jeudi 24 décembre 2015 à 15:15 -0500, Nicolas Dufresne a écrit :
> > > In order to convert from stride to pitch, few functions were
> > > diving
> > > by 4
> > > the stride. This is not valid for RGB565 and this conversion was
> > > not
> > > needed anyway in this context.
> > >
> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > > ---
> > > src/gallium/state_trackers/dri/dri2.c | 33 ++++++++++++---------
> > > ----
> > > --------
> > > 1 file changed, 12 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/src/gallium/state_trackers/dri/dri2.c
> > > b/src/gallium/state_trackers/dri/dri2.c
> > > index a11a6cb..9f07a45 100644
> > > --- a/src/gallium/state_trackers/dri/dri2.c
> > > +++ b/src/gallium/state_trackers/dri/dri2.c
> > > @@ -708,7 +708,7 @@ dri2_lookup_egl_image(struct dri_screen
> > > *screen,
> > > void *handle)
> > > static __DRIimage *
> > > dri2_create_image_from_winsys(__DRIscreen *_screen,
> > > int width, int height, int format,
> > > - struct winsys_handle *whandle, int
> > > pitch,
> > > + struct winsys_handle *whandle, int
> > > stride,
> > > void *loaderPrivate)
> > > {
> > > struct dri_screen *screen = dri_screen(_screen);
> > > @@ -753,7 +753,7 @@ dri2_create_image_from_winsys(__DRIscreen
> > > *_screen,
> > > templ.depth0 = 1;
> > > templ.array_size = 1;
> > >
> > > - whandle->stride = pitch * util_format_get_blocksize(pf);
> > > + whandle->stride = stride;
> > >
> > > img->texture = screen->base.screen-
> > > >resource_from_handle(screen-
> > > > base.screen,
> > > &templ, whandle);
> > > @@ -773,7 +773,7 @@ dri2_create_image_from_winsys(__DRIscreen
> > > *_screen,
> > > static __DRIimage *
> > > dri2_create_image_from_name(__DRIscreen *_screen,
> > > int width, int height, int format,
> > > - int name, int pitch, void
> > > *loaderPrivate)
> > > + int name, int stride, void
> > > *loaderPrivate)
> > > {
> > > struct winsys_handle whandle;
> > >
> > > @@ -782,13 +782,13 @@ dri2_create_image_from_name(__DRIscreen
> > > *_screen,
> > > whandle.handle = name;
> > >
> > > return dri2_create_image_from_winsys(_screen, width, height,
> > > format,
> > > - &whandle, pitch,
> > > loaderPrivate);
> > > + &whandle, stride,
> > > loaderPrivate);
> > > }
> > >
> > > static __DRIimage *
> > > dri2_create_image_from_fd(__DRIscreen *_screen,
> > > int width, int height, int format,
> > > - int fd, int pitch, void
> > > *loaderPrivate)
> > > + int fd, int stride, void
> > > *loaderPrivate)
> > > {
> > > struct winsys_handle whandle;
> > >
> > > @@ -800,7 +800,7 @@ dri2_create_image_from_fd(__DRIscreen
> > > *_screen,
> > > whandle.handle = (unsigned)fd;
> > >
> > > return dri2_create_image_from_winsys(_screen, width, height,
> > > format,
> > > - &whandle, pitch,
> > > loaderPrivate);
> > > + &whandle, stride,
> > > loaderPrivate);
> > > }
> > >
> > > static __DRIimage *
> > > @@ -986,7 +986,7 @@ dri2_from_names(__DRIscreen *screen, int
> > > width,
> > > int height, int format,
> > > void *loaderPrivate)
> > > {
> > > __DRIimage *img;
> > > - int stride, dri_components;
> > > + int dri_components;
> > >
> > > if (num_names != 1)
> > > return NULL;
> > > @@ -997,11 +997,8 @@ dri2_from_names(__DRIscreen *screen, int
> > > width,
> > > int height, int format,
> > > if (format == -1)
> > > return NULL;
> > >
> > > - /* Strides are in bytes not pixels. */
> > > - stride = strides[0] /4;
> > > -
> > > img = dri2_create_image_from_name(screen, width, height,
> > > format,
> > > - names[0], stride,
> > > loaderPrivate);
> > > + names[0], strides[0],
> > > loaderPrivate);
> > > if (img == NULL)
> > > return NULL;
> > >
> > > @@ -1101,7 +1098,7 @@ dri2_from_fds(__DRIscreen *screen, int
> > > width,
> > > int height, int fourcc,
> > > void *loaderPrivate)
> > > {
> > > __DRIimage *img;
> > > - int format, stride, dri_components;
> > > + int format, dri_components;
> > >
> > > if (num_fds != 1)
> > > return NULL;
> > > @@ -1112,11 +1109,8 @@ dri2_from_fds(__DRIscreen *screen, int
> > > width,
> > > int height, int fourcc,
> > > if (format == -1)
> > > return NULL;
> > >
> > > - /* Strides are in bytes not pixels. */
> > > - stride = strides[0] /4;
> > > -
> > > img = dri2_create_image_from_fd(screen, width, height,
> > > format,
> > > - fds[0], stride,
> > > loaderPrivate);
> > > + fds[0], strides[0],
> > > loaderPrivate);
> > > if (img == NULL)
> > > return NULL;
> > >
> > > @@ -1137,7 +1131,7 @@ dri2_from_dma_bufs(__DRIscreen *screen,
> > > void *loaderPrivate)
> > > {
> > > __DRIimage *img;
> > > - int format, stride, dri_components;
> > > + int format, dri_components;
> > >
> > > if (num_fds != 1 || offsets[0] != 0) {
> > > *error = __DRI_IMAGE_ERROR_BAD_MATCH;
> > > @@ -1150,11 +1144,8 @@ dri2_from_dma_bufs(__DRIscreen *screen,
> > > return NULL;
> > > }
> > >
> > > - /* Strides are in bytes not pixels. */
> > > - stride = strides[0] /4;
> > > -
> > > img = dri2_create_image_from_fd(screen, width, height,
> > > format,
> > > - fds[0], stride,
> > > loaderPrivate);
> > > + fds[0], strides[0],
> > > loaderPrivate);
> > > if (img == NULL) {
> > > *error = __DRI_IMAGE_ERROR_BAD_ALLOC;
> > > return NULL;
> > >
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160105/dba47b54/attachment.sig>
More information about the mesa-dev
mailing list