On 21.03.2017 17:51, 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.
Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
---
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.
This description might as well go into the commit message IMO.
Do you have test cases for these errors? If so, best to mention it in
the commit message as well.
---
src/mesa/state_tracker/st_cb_eglimage.c | 53 +++++++++++++++++++++++++++++++--
src/mesa/state_tracker/st_manager.c | 27 ++++++-----------
src/mesa/state_tracker/st_manager.h | 4 ++-
3 files changed, 63 insertions(+), 21 deletions(-)
diff --git a/src/mesa/state_tracker/st_cb_eglimage.c
b/src/mesa/state_tracker/st_cb_eglimage.c
index c425154..6810e24 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 <o...@lunarg.com>
*/
+#include "main/errors.h"
#include "main/texobj.h"
#include "main/teximage.h"
#include "util/u_inlines.h"
@@ -74,10 +75,34 @@ st_egl_image_target_renderbuffer_storage(struct gl_context
*ctx,
GLeglImageOES image_handle)
{
struct st_context *st = st_context(ctx);
+ struct st_manager *smapi =
+ (struct st_manager *) st->iface.st_context_private;
struct st_renderbuffer *strb = st_renderbuffer(rb);
+ struct pipe_screen *screen = st->pipe->screen;
+ struct st_egl_image stimg;
struct pipe_surface *ps;
- ps = st_manager_get_egl_image_surface(st, (void *) image_handle);
+ if (!smapi || !smapi->get_egl_image) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
"glEGLImageTargetRenderbufferStorage");
+ return NULL;
+ }
This error should be caught by the caller in mesa/main.
+
+ memset(&stimg, 0, sizeof(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,
"glEGLImageTargetRenderbufferStorage");
+ 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");
+ return NULL;
+ }
+
+ ps = st_manager_get_egl_image_surface(st, &stimg);
if (ps) {
strb->Base.Width = ps->width;
strb->Base.Height = ps->height;
@@ -160,9 +185,33 @@ st_egl_image_target_texture_2d(struct gl_context *ctx,
GLenum target,
GLeglImageOES image_handle)
{
struct st_context *st = st_context(ctx);
+ struct st_manager *smapi =
+ (struct st_manager *) st->iface.st_context_private;
+ struct pipe_screen *screen = st->pipe->screen;
+ struct st_egl_image stimg;
struct pipe_surface *ps;
- ps = st_manager_get_egl_image_surface(st, (void *) image_handle);
+ if (!smapi || !smapi->get_egl_image) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glEGLImageTargetTexture2D");
+ return NULL;
+ }
Same as above: should be caught by the caller.
+
+ memset(&stimg, 0, sizeof(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, "glEGLImageTargetTexture2D");
+ return NULL;
+ }
+
+ if (!screen->is_format_supported(screen, stimg.format, PIPE_TEXTURE_2D,
+ stimg.texture->nr_samples,
+ PIPE_BIND_SAMPLER_VIEW)) {
+ /* unable to specify a texture object using the specified EGL image */
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glEGLImageTargetTexture2D");
+ return NULL;
+ }
+
+ ps = st_manager_get_egl_image_surface(st, &stimg);
Since you're pretty much duplicating code here, have you considered
adding a helper function that does all the above? Actually, I think
st_manager_get_egl_image_surface should then be removed in favor of the
helper function in st_cb_eglimage.c.
if (ps) {
st_bind_surface(ctx, target, texObj, texImage, ps);
pipe_surface_reference(&ps, NULL);
diff --git a/src/mesa/state_tracker/st_manager.c
b/src/mesa/state_tracker/st_manager.c
index dad408a..8f16da1 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -862,27 +862,18 @@ st_manager_flush_frontbuffer(struct st_context *st)
* FIXME: I think this should operate on resources, not surfaces
*/
struct pipe_surface *
-st_manager_get_egl_image_surface(struct st_context *st, void *eglimg)
+st_manager_get_egl_image_surface(struct st_context *st,
+ struct st_egl_image *stimg)
{
- struct st_manager *smapi =
- (struct st_manager *) st->iface.st_context_private;
- struct st_egl_image stimg;
struct pipe_surface *ps, surf_tmpl;
- if (!smapi || !smapi->get_egl_image)
- return NULL;
-
- memset(&stimg, 0, sizeof(stimg));
- if (!smapi->get_egl_image(smapi, eglimg, &stimg))
- return NULL;
-
- u_surface_default_template(&surf_tmpl, stimg.texture);
- surf_tmpl.format = stimg.format;
- surf_tmpl.u.tex.level = stimg.level;
- surf_tmpl.u.tex.first_layer = stimg.layer;
- surf_tmpl.u.tex.last_layer = stimg.layer;
- ps = st->pipe->create_surface(st->pipe, stimg.texture, &surf_tmpl);
- pipe_resource_reference(&stimg.texture, NULL);
+ u_surface_default_template(&surf_tmpl, stimg->texture);
+ surf_tmpl.format = stimg->format;
+ surf_tmpl.u.tex.level = stimg->level;
+ surf_tmpl.u.tex.first_layer = stimg->layer;
+ surf_tmpl.u.tex.last_layer = stimg->layer;
+ ps = st->pipe->create_surface(st->pipe, stimg->texture, &surf_tmpl);
+ pipe_resource_reference(&stimg->texture, NULL);
Maybe moot if this function is removed anyway, but: the dereference
should be moved into the caller, so that referencing/dereferencing
logically happens in the same place.
Cheers,
Nicolai
return ps;
}
diff --git a/src/mesa/state_tracker/st_manager.h
b/src/mesa/state_tracker/st_manager.h
index bbb9b0f..8490c56 100644
--- a/src/mesa/state_tracker/st_manager.h
+++ b/src/mesa/state_tracker/st_manager.h
@@ -33,9 +33,11 @@
#include "pipe/p_compiler.h"
struct st_context;
+struct st_egl_image;
struct pipe_surface *
-st_manager_get_egl_image_surface(struct st_context *st, void *eglimg);
+st_manager_get_egl_image_surface(struct st_context *st,
+ struct st_egl_image *stimg);
void
st_manager_flush_frontbuffer(struct st_context *st);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
