Re: [Mesa-dev] [PATCH] st: Add cubeMapFace parameter to st_finalize_texture.

Nicolai Hähnle Sat, 01 Apr 2017 00:06:13 -0700

Pushed.

On 29.03.2017 21:58, Marek Olšák wrote:

I'm OK with this patch.


Marek

On Wed, Mar 29, 2017 at 12:57 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:

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



--
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

Re: [Mesa-dev] [PATCH] st: Add cubeMapFace parameter to st_finalize_texture.

Reply via email to