[Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
Some dri drivers will pass multiple bits in buffer_mask parameter
to droid_image_get_buffer(), more than the actual supported buffer
type combination. For such case, will go through all the bits, and
will not return error when unsupported buffer is requested, only
return error when the allocation for supported buffer failed.
Signed-off-by: Liu Zhiquan
Signed-off-by: Long, Zhifang
---
src/egl/drivers/dri2/platform_android.c | 209 +++-
1 file changed, 126 insertions(+), 83 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index 373e2c0..c70423d 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -392,13 +392,13 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *surf)
}
if (dri2_surf->dri_image_back) {
- _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__,
__LINE__);
+ _eglLog(_EGL_DEBUG, "destroy dri_image_back");
dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
dri2_surf->dri_image_back = NULL;
}
if (dri2_surf->dri_image_front) {
- _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
__LINE__);
+ _eglLog(_EGL_DEBUG, "destroy dri_image_front");
dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
dri2_surf->dri_image_front = NULL;
}
@@ -434,81 +434,98 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
}
static int
-get_back_bo(struct dri2_egl_surface *dri2_surf)
+get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
{
struct dri2_egl_display *dri2_dpy =
dri2_egl_display(dri2_surf->base.Resource.Display);
- int fourcc, pitch;
- int offset = 0, fd;
- if (dri2_surf->dri_image_back)
+ if (dri2_surf->dri_image_front)
+ {
+ _eglLog(_EGL_WARNING, "dri2_image_front allocated !");
return 0;
-
- if (!dri2_surf->buffer)
- return -1;
-
- fd = get_native_buffer_fd(dri2_surf->buffer);
- if (fd < 0) {
- _eglLog(_EGL_WARNING, "Could not get native buffer FD");
- return -1;
}
- fourcc = get_fourcc(dri2_surf->buffer->format);
-
- pitch = dri2_surf->buffer->stride *
- get_format_bpp(dri2_surf->buffer->format);
-
- if (fourcc == -1 || pitch == 0) {
- _eglLog(_EGL_WARNING, "Invalid buffer fourcc(%x) or pitch(%d)",
- fourcc, pitch);
- return -1;
+ if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
+ /* According current EGL spec, front buffer rendering
+ * for window surface is not supported now.
+ * and mesa doesn't have the implemetation of this case.
+ * Add warnning message, but not treat it as error.
+ */
+ _eglLog(_EGL_DEBUG, "front buffer for window surface is not supported
now !");
+
+ } else if (dri2_surf->base.Type == EGL_PBUFFER_BIT) {
+
+ dri2_surf->dri_image_front =
+ dri2_dpy->image->createImage(dri2_dpy->dri_screen,
+ dri2_surf->base.Width,
+ dri2_surf->base.Height,
+ format,
+ 0,
+ dri2_surf);
+ if (!dri2_surf->dri_image_front)
+ {
+ _eglLog(_EGL_WARNING, "dri2_image_front allocation failed !");
+ return -1;
+ }
+ } else {
+ _eglLog(_EGL_WARNING, "pixmap is not supported now !");
}
- dri2_surf->dri_image_back =
- dri2_dpy->image->createImageFromFds(dri2_dpy->dri_screen,
- dri2_surf->base.Width,
- dri2_surf->base.Height,
- fourcc,
- &fd,
- 1,
- &pitch,
- &offset,
- dri2_surf);
- if (!dri2_surf->dri_image_back)
- return -1;
-
return 0;
}
static int
-droid_image_get_buffers(__DRIdrawable *driDrawable,
- unsigned int format,
- uint32_t *stamp,
- void *loaderPrivate,
- uint32_t buffer_mask,
- struct __DRIimageList *images)
+get_back_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
{
- struct dri2_egl_surface *dri2_surf = loaderPrivate;
struct dri2_egl_display *dri2_dpy =
dri2_egl_display(dri2_surf->base.Resource.Display);
+ int fourcc, pitch;
+ int offset = 0, fd;
- images->image_mask = 0;
- images->front = NULL;
- images->back = NULL;
-
- if (update_buffers(dri2_surf) < 0)
+ if (dri2_surf->dri_image_back) {
+ _eglLog(_EGL_WARNING, "dri_image_back allocated !");
return 0;
+ }
- if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
-
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
On 25 November 2016 at 03:58, Liu Zhiquan wrote:
> Some dri drivers will pass multiple bits in buffer_mask parameter
> to droid_image_get_buffer(), more than the actual supported buffer
> type combination. For such case, will go through all the bits, and
> will not return error when unsupported buffer is requested, only
> return error when the allocation for supported buffer failed.
>
> Signed-off-by: Liu Zhiquan
> Signed-off-by: Long, Zhifang
> ---
> src/egl/drivers/dri2/platform_android.c | 209
> +++-
> 1 file changed, 126 insertions(+), 83 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index 373e2c0..c70423d 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -392,13 +392,13 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> }
>
> if (dri2_surf->dri_image_back) {
> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__,
> __LINE__);
> + _eglLog(_EGL_DEBUG, "destroy dri_image_back");
>dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>dri2_surf->dri_image_back = NULL;
> }
>
> if (dri2_surf->dri_image_front) {
> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
> __LINE__);
> + _eglLog(_EGL_DEBUG, "destroy dri_image_front");
>dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>dri2_surf->dri_image_front = NULL;
> }
Unrelated changes ?
> @@ -434,81 +434,98 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> }
>
> static int
> -get_back_bo(struct dri2_egl_surface *dri2_surf)
> +get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
> {
While I see Tomasz concern I'm wondering if we cannot make this
__DRIimage *
get_bo(..., uint32_t buffer_mask)
{
...
}
droid_image_get_buffers(...)
{
struct dri2_egl_surface *dri2_surf = loaderPrivate;
images->image_mask = 0;
images->front = NULL;
images->back = NULL;
if (update_buffers(dri2_surf) < 0)
return 0;
if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
images->front = get_front_bo(... buffer_mask);
if (images->front < 0) {
_eglError(EGL_BAD_PARAMETER, "droid_get_front_bo");
return 0;
}
...
}
if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
images->back = get_front_bo(... buffer_mask);
if (images->back < 0)
This way things would be reasonably simple and reusable.
Hit I'm thinking that we want to unify much/most of the platform_foo.c
code... one of these days.
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
On Mon, Nov 28, 2016 at 10:35 PM, Emil Velikov wrote:
> On 25 November 2016 at 03:58, Liu Zhiquan wrote:
>> Some dri drivers will pass multiple bits in buffer_mask parameter
>> to droid_image_get_buffer(), more than the actual supported buffer
>> type combination. For such case, will go through all the bits, and
>> will not return error when unsupported buffer is requested, only
>> return error when the allocation for supported buffer failed.
>>
>> Signed-off-by: Liu Zhiquan
>> Signed-off-by: Long, Zhifang
>> ---
>> src/egl/drivers/dri2/platform_android.c | 209
>> +++-
>> 1 file changed, 126 insertions(+), 83 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c
>> b/src/egl/drivers/dri2/platform_android.c
>> index 373e2c0..c70423d 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -392,13 +392,13 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
>> *disp, _EGLSurface *surf)
>> }
>>
>> if (dri2_surf->dri_image_back) {
>> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__,
>> __LINE__);
>> + _eglLog(_EGL_DEBUG, "destroy dri_image_back");
>>dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>>dri2_surf->dri_image_back = NULL;
>> }
>>
>> if (dri2_surf->dri_image_front) {
>> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
>> __LINE__);
>> + _eglLog(_EGL_DEBUG, "destroy dri_image_front");
>>dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>>dri2_surf->dri_image_front = NULL;
>> }
> Unrelated changes ?
>
>
>> @@ -434,81 +434,98 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
>> }
>>
>> static int
>> -get_back_bo(struct dri2_egl_surface *dri2_surf)
>> +get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
>> {
> While I see Tomasz concern I'm wondering if we cannot make this
>
> __DRIimage *
> get_bo(..., uint32_t buffer_mask)
> {
> ...
> }
>
Hmm, could you point me to the common code we would get by keeping
these two functions together? From what I see, both cases have
significantly different handling and that's why I asked for splitting
them. It's visible clearly in my original patch which ended up with
much less code than the one that was merged (and this one is trying to
fix).
>
> droid_image_get_buffers(...)
> {
>struct dri2_egl_surface *dri2_surf = loaderPrivate;
>
>images->image_mask = 0;
>images->front = NULL;
>images->back = NULL;
>
>if (update_buffers(dri2_surf) < 0)
> return 0;
>
>if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> images->front = get_front_bo(... buffer_mask);
> if (images->front < 0) {
> _eglError(EGL_BAD_PARAMETER, "droid_get_front_bo");
> return 0;
> }
> ...
>}
>if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
> images->back = get_front_bo(... buffer_mask);
> if (images->back < 0)
This code is actually where the duplication is (doing the same for
each of the bits but dispatching different allocation/dequeue code),
but we can't do much since "images" is not an array, but a struct.
>
>
>
> This way things would be reasonably simple and reusable.
> Hit I'm thinking that we want to unify much/most of the platform_foo.c
> code... one of these days.
Hmm, pbuffer handling should be mostly the same for all (I'm guessing
that X11 might be doing some funky stuff, though...). I was wondering
why we even need to have any pbuffer code in platform backends.
(Pixmaps and windows are a different, completely platform-specific
thing, though...)
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
On 28 November 2016 at 17:46, Tomasz Figa wrote:
> On Mon, Nov 28, 2016 at 10:35 PM, Emil Velikov
> wrote:
>> On 25 November 2016 at 03:58, Liu Zhiquan wrote:
>>> Some dri drivers will pass multiple bits in buffer_mask parameter
>>> to droid_image_get_buffer(), more than the actual supported buffer
>>> type combination. For such case, will go through all the bits, and
>>> will not return error when unsupported buffer is requested, only
>>> return error when the allocation for supported buffer failed.
>>>
>>> Signed-off-by: Liu Zhiquan
>>> Signed-off-by: Long, Zhifang
>>> ---
>>> src/egl/drivers/dri2/platform_android.c | 209
>>> +++-
>>> 1 file changed, 126 insertions(+), 83 deletions(-)
>>>
>>> diff --git a/src/egl/drivers/dri2/platform_android.c
>>> b/src/egl/drivers/dri2/platform_android.c
>>> index 373e2c0..c70423d 100644
>>> --- a/src/egl/drivers/dri2/platform_android.c
>>> +++ b/src/egl/drivers/dri2/platform_android.c
>>> @@ -392,13 +392,13 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
>>> *disp, _EGLSurface *surf)
>>> }
>>>
>>> if (dri2_surf->dri_image_back) {
>>> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__,
>>> __LINE__);
>>> + _eglLog(_EGL_DEBUG, "destroy dri_image_back");
>>>dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>>>dri2_surf->dri_image_back = NULL;
>>> }
>>>
>>> if (dri2_surf->dri_image_front) {
>>> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
>>> __LINE__);
>>> + _eglLog(_EGL_DEBUG, "destroy dri_image_front");
>>>dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>>>dri2_surf->dri_image_front = NULL;
>>> }
>> Unrelated changes ?
>>
>>
>>> @@ -434,81 +434,98 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
>>> }
>>>
>>> static int
>>> -get_back_bo(struct dri2_egl_surface *dri2_surf)
>>> +get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
>>> {
>> While I see Tomasz concern I'm wondering if we cannot make this
>>
>> __DRIimage *
>> get_bo(..., uint32_t buffer_mask)
>> {
>> ...
>> }
>>
>
> Hmm, could you point me to the common code we would get by keeping
> these two functions together? From what I see, both cases have
> significantly different handling and that's why I asked for splitting
> them. It's visible clearly in my original patch which ended up with
> much less code than the one that was merged (and this one is trying to
> fix).
>
>>
>> droid_image_get_buffers(...)
>> {
>>struct dri2_egl_surface *dri2_surf = loaderPrivate;
>>
>>images->image_mask = 0;
>>images->front = NULL;
>>images->back = NULL;
>>
>>if (update_buffers(dri2_surf) < 0)
>> return 0;
>>
>>if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
>> images->front = get_front_bo(... buffer_mask);
>> if (images->front < 0) {
>> _eglError(EGL_BAD_PARAMETER, "droid_get_front_bo");
>> return 0;
>> }
>> ...
>>}
>>if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
>> images->back = get_front_bo(... buffer_mask);
>> if (images->back < 0)
>
> This code is actually where the duplication is (doing the same for
> each of the bits but dispatching different allocation/dequeue code),
> but we can't do much since "images" is not an array, but a struct.
>
Having a closer look - something have gone short circuited on my end.
Please ignore my comments above.
>>
>>
>>
>> This way things would be reasonably simple and reusable.
>> Hit I'm thinking that we want to unify much/most of the platform_foo.c
>> code... one of these days.
>
> Hmm, pbuffer handling should be mostly the same for all (I'm guessing
> that X11 might be doing some funky stuff, though...). I was wondering
> why we even need to have any pbuffer code in platform backends.
> (Pixmaps and windows are a different, completely platform-specific
> thing, though...)
>
Roughly what I'm wondering as well ;-)
Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
> >>> if (dri2_surf->dri_image_front) {
> >>> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
> __LINE__);
> >>> + _eglLog(_EGL_DEBUG, "destroy dri_image_front");
> >>>dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
> >>>dri2_surf->dri_image_front = NULL;
> >>> }
> >> Unrelated changes ?
Refer to mesa code, uniform coding(log) style.
These logs were added in pbuffer implementation patch, so I changed them in
this enhance patch.
> >> This way things would be reasonably simple and reusable.
> >> Hit I'm thinking that we want to unify much/most of the
> >> platform_foo.c code... one of these days.
> >
> > Hmm, pbuffer handling should be mostly the same for all (I'm
> > guessing that X11 might be doing some funky stuff, though...). I was
> > wondering why we even need to have any pbuffer code in platform backends.
> > (Pixmaps and windows are a different, completely platform-specific
> > thing, though...)
> >
> Roughly what I'm wondering as well ;-)
This patch mainly resolve multiple bits requirement of pbuffer implemental.
If it's correct, could you merge it ?
Best Regards,
Zhiquan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
Hi,
On Fri, Nov 25, 2016 at 12:58 PM, Liu Zhiquan wrote:
> Some dri drivers will pass multiple bits in buffer_mask parameter
> to droid_image_get_buffer(), more than the actual supported buffer
> type combination. For such case, will go through all the bits, and
> will not return error when unsupported buffer is requested, only
> return error when the allocation for supported buffer failed.
Please see my comments inline.
>
> Signed-off-by: Liu Zhiquan
> Signed-off-by: Long, Zhifang
> ---
> src/egl/drivers/dri2/platform_android.c | 209
> +++-
> 1 file changed, 126 insertions(+), 83 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index 373e2c0..c70423d 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -392,13 +392,13 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> }
>
> if (dri2_surf->dri_image_back) {
> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__,
> __LINE__);
> + _eglLog(_EGL_DEBUG, "destroy dri_image_back");
>dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>dri2_surf->dri_image_back = NULL;
> }
>
> if (dri2_surf->dri_image_front) {
> - _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__,
> __LINE__);
> + _eglLog(_EGL_DEBUG, "destroy dri_image_front");
Patch description mentions only a change to handle multiple buffer
bits. Any other changes should be sent in separate patches.
>dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>dri2_surf->dri_image_front = NULL;
> }
> @@ -434,81 +434,98 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> }
>
> static int
> -get_back_bo(struct dri2_egl_surface *dri2_surf)
> +get_front_bo(struct dri2_egl_surface *dri2_surf, unsigned int format)
> {
> struct dri2_egl_display *dri2_dpy =
>dri2_egl_display(dri2_surf->base.Resource.Display);
> - int fourcc, pitch;
> - int offset = 0, fd;
>
> - if (dri2_surf->dri_image_back)
> + if (dri2_surf->dri_image_front)
> + {
style: This file seems to follow the convention of opening brace at
the same line as the statement.
> + _eglLog(_EGL_WARNING, "dri2_image_front allocated !");
This is a normal case, there is no need to print anything here.
>return 0;
> -
> - if (!dri2_surf->buffer)
> - return -1;
> -
> - fd = get_native_buffer_fd(dri2_surf->buffer);
> - if (fd < 0) {
> - _eglLog(_EGL_WARNING, "Could not get native buffer FD");
> - return -1;
> }
>
> - fourcc = get_fourcc(dri2_surf->buffer->format);
> -
> - pitch = dri2_surf->buffer->stride *
> - get_format_bpp(dri2_surf->buffer->format);
> -
> - if (fourcc == -1 || pitch == 0) {
> - _eglLog(_EGL_WARNING, "Invalid buffer fourcc(%x) or pitch(%d)",
> - fourcc, pitch);
> - return -1;
> + if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> + /* According current EGL spec, front buffer rendering
> + * for window surface is not supported now.
> + * and mesa doesn't have the implemetation of this case.
typo: s/implemetation/implementation/
> + * Add warnning message, but not treat it as error.
typo: s/warnning/warning/
> + */
> + _eglLog(_EGL_DEBUG, "front buffer for window surface is not supported
> now !");
nit: No need for exclamation mark. Also the message could be a bit
more informational, e.g.
"DRI driver requested unsupported front buffer for window surface"
> +
We can just return 0 here, no need to fall through to the end of the function.
> + } else if (dri2_surf->base.Type == EGL_PBUFFER_BIT) {
We won't be called with anything else than window or pbuffer bit here,
because we don't advertise pixmap support and createPixmapSurface is
stubbed out. For better coding style (less indentation) it's enough to
just return 0 in the if above and then move the code below out of the
conditional block completely.
> +
> + dri2_surf->dri_image_front =
> + dri2_dpy->image->createImage(dri2_dpy->dri_screen,
> + dri2_surf->base.Width,
> + dri2_surf->base.Height,
> + format,
> + 0,
> + dri2_surf);
> + if (!dri2_surf->dri_image_front)
> + {
Style: Brace should be on the same linen as if statement.
> + _eglLog(_EGL_WARNING, "dri2_image_front allocation failed !");
No need for exclamation mark.
> + return -1;
> + }
> + } else {
> + _eglLog(_EGL_WARNING, "pixmap is not supported now !");
> }
This else block is not needed, as I explained above.
>
> - dri2_surf->dri_image_back =
> - dri2_dpy->image->createImageFromFds(dri2_dpy->dri_screen,
> -
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
Hi Tomasz,
Thanks for the review. I will change these logs and code styles errors in v2
version.
> > + } else if (dri2_surf->base.Type == EGL_PBUFFER_BIT) {
>
> We won't be called with anything else than window or pbuffer bit here,
> because we don't advertise pixmap support and createPixmapSurface is
> stubbed out. For better coding style (less indentation) it's enough to
> just return 0 in the if above and then move the code below out of the
> conditional block completely.
The original code (if...else if...)is very clearly to describe what will happen
for each type(window, pbuffer, pixmap).
It's good for readability.
> > + return -1;
> > + }
> > + } else {
> > + _eglLog(_EGL_WARNING, "pixmap is not supported now !");
> > }
>
> This else block is not needed, as I explained above.
I suggest to remove this pixmap log, and add follow description in else.
/*pixmap is not supported now, add the implementation of pixmap here in the
future.*/
Best Regards,
Zhiquan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
On Mon, Dec 5, 2016 at 10:20 PM, Liu, Zhiquan wrote:
> Hi Tomasz,
>
> Thanks for the review. I will change these logs and code styles errors in v2
> version.
>
>> > + } else if (dri2_surf->base.Type == EGL_PBUFFER_BIT) {
>>
>> We won't be called with anything else than window or pbuffer bit here,
>> because we don't advertise pixmap support and createPixmapSurface is
>> stubbed out. For better coding style (less indentation) it's enough to
>> just return 0 in the if above and then move the code below out of the
>> conditional block completely.
> The original code (if...else if...)is very clearly to describe what will
> happen for each type(window, pbuffer, pixmap).
> It's good for readability.
>
>> > + return -1;
>> > + }
>> > + } else {
>> > + _eglLog(_EGL_WARNING, "pixmap is not supported now !");
>> > }
>>
>> This else block is not needed, as I explained above.
> I suggest to remove this pixmap log, and add follow description in else.
> /*pixmap is not supported now, add the implementation of pixmap here in the
> future.*/
This is just speculating about future. Odds are that pixmap support
won't show up in this backend at all, so for now it would be just
adding unnecessary lines of code. Let's stick to the "keep it simple
s." principle.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation
I upload v2 version, please review. https://patchwork.freedesktop.org/patch/125787/ Best Regards, Zhiquan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
