[Mesa-dev] [PATCH] gallium/dri2: Avoid uneeded stride to pitch conversion

Axel Davy axel.davy at ens.fr
Tue Jan 5 08:19:53 PST 2016


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 ...

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

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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160105/9c9c529b/attachment-0001.html>


More information about the mesa-dev mailing list