Hi Michal,
thanks for the patch. That piglit test actually fails on radeonsi as well.
On 28.03.2017 22:39, Michal Srb wrote:
st_finalize_texture always accesses image at face 0, but it may not be set if
we are working with cubemap that had other face set.
This fixes crash in piglit
same-attachment-glFramebufferTexture2D-GL_DEPTH_STENCIL_ATTACHMENT.
Please make sure commit messages are wrapped to <75 characters.
Also:
Cc: mesa-sta...@lists.freedesktop.org
---
Hi, this is my attempt to fix crash in piglit test
same-attachment-glFramebufferTexture2D-GL_DEPTH_STENCIL_ATTACHMENT ran with
LIBGL_ALWAYS_INDIRECT=1.
I am not sure if it is the right approach. From what I found online rendering
into a face of a cube texture that doesn't have all faces set would be invalid,
but the test passes with other drivers, so maybe it's ok.
This makes it pass with software rendering as well.
I actually don't see anything in the spec that would require texture
completeness. That makes sense, since rendering into one image of a
texture doesn't imply using sampler state. So allowing the test to pass
is good.
The flip-side is that this means calling st_finalize_texture at all may
not be the right thing to do in the FBO code (except perhaps as an
opportunistic optimization). After all, we could have a messed up
situation where there are incompatible mip level in a texture, and we
render to one of them anyway.
Cleaning that up would be quite involved. I think this fix is fine for
now, since it does improve the situation:
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
Let's see if there are any other opinions...
Cheers,
Nicolai
src/gallium/state_trackers/dri/dri2.c | 2 +-
src/mesa/state_tracker/st_atom_image.c | 2 +-
src/mesa/state_tracker/st_atom_texture.c | 2 +-
src/mesa/state_tracker/st_cb_fbo.c | 2 +-
src/mesa/state_tracker/st_cb_texture.c | 5 +++--
src/mesa/state_tracker/st_cb_texture.h | 3 ++-
src/mesa/state_tracker/st_gen_mipmap.c | 2 +-
7 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/gallium/state_trackers/dri/dri2.c
b/src/gallium/state_trackers/dri/dri2.c
index b50e096..ed6004f 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -1808,7 +1808,7 @@ dri2_interop_export_object(__DRIcontext *_ctx,
return MESA_GLINTEROP_INVALID_MIP_LEVEL;
}
- if (!st_finalize_texture(ctx, st->pipe, obj)) {
+ if (!st_finalize_texture(ctx, st->pipe, obj, 0)) {
mtx_unlock(&ctx->Shared->Mutex);
return MESA_GLINTEROP_OUT_OF_RESOURCES;
}
diff --git a/src/mesa/state_tracker/st_atom_image.c
b/src/mesa/state_tracker/st_atom_image.c
index 5dd2cd6..4101552 100644
--- a/src/mesa/state_tracker/st_atom_image.c
+++ b/src/mesa/state_tracker/st_atom_image.c
@@ -64,7 +64,7 @@ st_bind_images(struct st_context *st, struct gl_program *prog,
struct pipe_image_view *img = &images[i];
if (!_mesa_is_image_unit_valid(st->ctx, u) ||
- !st_finalize_texture(st->ctx, st->pipe, u->TexObj) ||
+ !st_finalize_texture(st->ctx, st->pipe, u->TexObj, 0) ||
!stObj->pt) {
memset(img, 0, sizeof(*img));
continue;
diff --git a/src/mesa/state_tracker/st_atom_texture.c
b/src/mesa/state_tracker/st_atom_texture.c
index 92023e0..5b481ec 100644
--- a/src/mesa/state_tracker/st_atom_texture.c
+++ b/src/mesa/state_tracker/st_atom_texture.c
@@ -73,7 +73,7 @@ update_single_texture(struct st_context *st,
}
stObj = st_texture_object(texObj);
- retval = st_finalize_texture(ctx, st->pipe, texObj);
+ retval = st_finalize_texture(ctx, st->pipe, texObj, 0);
if (!retval) {
/* out of mem */
return GL_FALSE;
diff --git a/src/mesa/state_tracker/st_cb_fbo.c
b/src/mesa/state_tracker/st_cb_fbo.c
index 78433bf..dce4239 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -488,7 +488,7 @@ st_render_texture(struct gl_context *ctx,
struct st_renderbuffer *strb = st_renderbuffer(rb);
struct pipe_resource *pt;
- if (!st_finalize_texture(ctx, pipe, att->Texture))
+ if (!st_finalize_texture(ctx, pipe, att->Texture, att->CubeMapFace))
return;
pt = st_get_texobj_resource(att->Texture);
diff --git a/src/mesa/state_tracker/st_cb_texture.c
b/src/mesa/state_tracker/st_cb_texture.c
index bc6f108..1b486d7 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -2434,7 +2434,8 @@ copy_image_data_to_texture(struct st_context *st,
GLboolean
st_finalize_texture(struct gl_context *ctx,
struct pipe_context *pipe,
- struct gl_texture_object *tObj)
+ struct gl_texture_object *tObj,
+ GLuint cubeMapFace)
{
struct st_context *st = st_context(ctx);
struct st_texture_object *stObj = st_texture_object(tObj);
@@ -2478,7 +2479,7 @@ st_finalize_texture(struct gl_context *ctx,
}
- firstImage = st_texture_image_const(_mesa_base_tex_image(&stObj->base));
+ firstImage =
st_texture_image_const(stObj->base.Image[cubeMapFace][stObj->base.BaseLevel]);
assert(firstImage);
/* If both firstImage and stObj point to a texture which can contain
diff --git a/src/mesa/state_tracker/st_cb_texture.h
b/src/mesa/state_tracker/st_cb_texture.h
index 415d59f..f647b16 100644
--- a/src/mesa/state_tracker/st_cb_texture.h
+++ b/src/mesa/state_tracker/st_cb_texture.h
@@ -47,7 +47,8 @@ st_get_blit_mask(GLenum srcFormat, GLenum dstFormat);
extern GLboolean
st_finalize_texture(struct gl_context *ctx,
struct pipe_context *pipe,
- struct gl_texture_object *tObj);
+ struct gl_texture_object *tObj,
+ GLuint cubeMapFace);
extern void
diff --git a/src/mesa/state_tracker/st_gen_mipmap.c
b/src/mesa/state_tracker/st_gen_mipmap.c
index 10af11e..16b914a 100644
--- a/src/mesa/state_tracker/st_gen_mipmap.c
+++ b/src/mesa/state_tracker/st_gen_mipmap.c
@@ -125,7 +125,7 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
*
* After this, we'll have all mipmap levels in one resource.
*/
- st_finalize_texture(ctx, st->pipe, texObj);
+ st_finalize_texture(ctx, st->pipe, texObj, 0);
}
pt = stObj->pt;
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
