On 6 July 2017 at 15:45, Eric Engestrom <eric.engest...@imgtec.com> wrote:
> On Friday, 2017-06-30 12:15:19 +0100, Emil Velikov wrote:
>> From: Emil Velikov <emil.veli...@collabora.com>
>>
>> Drop the (duplicate) top-level check in dri2_create_image_khr() and add
>> the respective checks in dri2_create_image_khr_{texture,renderbuffer}
>>
>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> ---
>> src/egl/drivers/dri2/egl_dri2.c | 36 +++++++++++++++++++++---------------
>> 1 file changed, 21 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.c
>> b/src/egl/drivers/dri2/egl_dri2.c
>> index a641d774523..78a6d5f2219 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.c
>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>> @@ -1682,6 +1682,11 @@ dri2_create_image_khr_renderbuffer(_EGLDisplay *disp,
>> _EGLContext *ctx,
>> return EGL_NO_IMAGE_KHR;
>> }
>>
>> + if (!disp->Extensions.KHR_gl_renderbuffer_image) {
>> + _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>> + return EGL_NO_IMAGE_KHR;
>> + }
>> +
>> dri_image =
>> dri2_dpy->image->createImageFromRenderbuffer(dri2_ctx->dri_context,
>> renderbuffer, NULL);
>> @@ -1820,30 +1825,38 @@ dri2_create_image_khr_texture(_EGLDisplay *disp,
>> _EGLContext *ctx,
>>
>> switch (target) {
>> case EGL_GL_TEXTURE_2D_KHR:
>> + if (!disp->Extensions.KHR_gl_texture_2D_image) {
>> + _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>> + return EGL_NO_IMAGE_KHR;
>> + }
>> depth = 0;
>> gl_target = GL_TEXTURE_2D;
>> break;
>> case EGL_GL_TEXTURE_3D_KHR:
>> - if (disp->Extensions.KHR_gl_texture_3D_image) {
>> - depth = attrs.GLTextureZOffset;
>> - gl_target = GL_TEXTURE_3D;
>> - break;
>> - }
>> - else {
>> + if (!disp->Extensions.KHR_gl_texture_3D_image) {
>> _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>> return EGL_NO_IMAGE_KHR;
>> }
>> +
>> + depth = attrs.GLTextureZOffset;
>> + gl_target = GL_TEXTURE_3D;
>> + break;
>> case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_X_KHR:
>> case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_X_KHR:
>> case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_Y_KHR:
>> case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_KHR:
>> case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_Z_KHR:
>> case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_KHR:
>> + if (!disp->Extensions.KHR_gl_texture_cubemap_image) {
>> + _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>> + return EGL_NO_IMAGE_KHR;
>> + }
>> +
>> depth = target - EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_X_KHR;
>> gl_target = GL_TEXTURE_CUBE_MAP;
>> break;
>> default:
>> - _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>> + assert(!"Unexpected target in dri2_create_image_khr_texture()");
>
> I think the _eglError() should stay.
>
This function is called (by dri2_create_image_khr) when the target is
either TEXTURE_2D/3D or any of the 6 TEXTURE_CUBE_MAP.
Hence the default case is unreachable. I've opted for an assert, since
we'd want to quickly get feedback if we get there.
> My comment on patch 1/10 clearly shows that I didn't read the whole
> series before replying: you can ignore it :]
>
> Series is:
> Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>
>
> Btw, I really like the solution in patch 3/10, I had this split on my
> todo-list for a while, but I hadn't thought of a good way to avoid an
> `if`-nesting-chain; your solution looks better than what I had in mind :)
>
Glad to hear you like it. There's still a bit of ugly in there, but
it's should be easier to follow than the previous 200+ lines switch
statement ;-)
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
