[Mesa-dev] [PATCH 2/2] st/mesa: EGLImageTarget* error handling

[Nicolai Hähnle]

On 27.03.2017 12:11, Philipp Zabel wrote:
> Stop trying to specify texture or renderbuffer objects for unsupported
> EGL images. Generate the error codes specified in the OES_EGL_image
> extension.
>
> EGLImageTargetTexture2D and EGLImageTargetRenderbuffer would call
> the pipe driver's create_surface callback without ever checking that
> the given EGL image is actually compatible with the chosen target
> texture or renderbuffer. This patch adds a call to the pipe driver's
> is_format_supported callback and generates an INVALID_OPERATION error
> for unsupported EGL images. If the EGL image handle does not describe
> a valid EGL image, an INVALID_VALUE error is generated.
>
> Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> ---
>  src/mesa/state_tracker/st_cb_eglimage.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_eglimage.c b/src/mesa/state_tracker/st_cb_eglimage.c
> index e1fc9bff75..30b177731e 100644
> --- a/src/mesa/state_tracker/st_cb_eglimage.c
> +++ b/src/mesa/state_tracker/st_cb_eglimage.c
> @@ -25,6 +25,7 @@
>   *    Chia-I Wu <olv at lunarg.com>
>   */
>
> +#include "main/errors.h"
>  #include "main/texobj.h"
>  #include "main/teximage.h"
>  #include "util/u_inlines.h"
> @@ -44,9 +45,11 @@
>   * FIXME: I think this should operate on resources, not surfaces
>   */
>  static struct pipe_surface *
> -st_egl_image_get_surface(struct gl_context *ctx, GLeglImageOES image_handle)
> +st_egl_image_get_surface(struct gl_context *ctx, GLeglImageOES image_handle,
> +                         unsigned usage, const char *error)
>  {
>     struct st_context *st = st_context(ctx);
> +   struct pipe_screen *screen = st->pipe->screen;
>     struct st_manager *smapi =
>        (struct st_manager *) st->iface.st_context_private;
>     struct st_egl_image stimg;
> @@ -56,8 +59,19 @@ st_egl_image_get_surface(struct gl_context *ctx, GLeglImageOES image_handle)
>        return NULL;
>
>     memset(&stimg, 0, sizeof(stimg));
> -   if (!smapi->get_egl_image(smapi, (void *) image_handle, &stimg))
> +   if (!smapi->get_egl_image(smapi, (void *) image_handle, &stimg)) {
> +      /* image_handle does not refer to a valid EGL image object */
> +      _mesa_error(ctx, GL_INVALID_VALUE, error);
>        return NULL;
> +   }
> +
> +   if (!screen->is_format_supported(screen, stimg.format, PIPE_TEXTURE_2D,
> +                                    stimg.texture->nr_samples,
> +                                    PIPE_BIND_RENDER_TARGET)) {
> +      /* unable to specify a texture object using the specified EGL image */
> +      _mesa_error(ctx, GL_INVALID_OPERATION, "glEGLImageTargetRenderbufferStorage");

This block still needs to be changed to use the usage and error 
parameters instead of hard-coded values. With that fixed, the series is

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


> +      return NULL;
> +   }
>
>     u_surface_default_template(&surf_tmpl, stimg.texture);
>     surf_tmpl.format = stimg.format;
> @@ -108,7 +122,8 @@ st_egl_image_target_renderbuffer_storage(struct gl_context *ctx,
>     struct st_renderbuffer *strb = st_renderbuffer(rb);
>     struct pipe_surface *ps;
>
> -   ps = st_egl_image_get_surface(ctx, image_handle);
> +   ps = st_egl_image_get_surface(ctx, image_handle, PIPE_BIND_RENDER_TARGET,
> +				 "glEGLImageTargetRenderbufferStorage");
>     if (ps) {
>        strb->Base.Width = ps->width;
>        strb->Base.Height = ps->height;
> @@ -192,7 +207,8 @@ st_egl_image_target_texture_2d(struct gl_context *ctx, GLenum target,
>  {
>     struct pipe_surface *ps;
>
> -   ps = st_egl_image_get_surface(ctx, image_handle);
> +   ps = st_egl_image_get_surface(ctx, image_handle, PIPE_BIND_SAMPLER_VIEW,
> +				 "glEGLImageTargetTexture2D");
>     if (ps) {
>        st_bind_surface(ctx, target, texObj, texImage, ps);
>        pipe_surface_reference(&ps, NULL);
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.