On 04.11.2016 09:47, Christian König wrote:
Am 03.11.2016 um 22:06 schrieb Nicolai Hähnle:
From: Nicolai Hähnle <nicolai.haeh...@amd.com>

A latent bug in VDPAU interop was exposed by commit
e5cc84dd43be066c1dd418e32f5ad258e31a150a.

Before that commit, the st_vdpau code created samplers with
first_layer == last_layer == 1 that the general texture handling code
would immediately delete and re-create, because the layer does not match
the information in the GL texture object.

This was correct behavior, because the imported resource because the
imported resource is supposed to have the correct offset already applied.

After that commit, the state tracker assumes that an existing sampler is
correct at all times.  Existing samplers are supposed to be deleted when
they may become invalid, and they will be created on-demand.  This meant
that the sampler with first_layer == last_layer == 1 stuck around,
leading
to rendering artefacts (on radeonsi), command stream failures (on
r600), and
assertions (in debug builds).

This patch fixes the problem by simply not creating a sampler at all in
st_vdpau_map_surface and relying on the generic texture code to do the
right
thing.

A big NAK on this, cause this way GL clearly samples from the wrong
layer for the classic interop case.

What we probably should rather do is setting index=0 on line 202 when
the DMA-buf import succeeded, cause resources imported using the DMA-buf
method just have one layer.

Then I'm confused about how this was ever supposed to work. Take a look at st_get_texture_sampler_view_from_stobj, in particular the problematic commit e5cc84dd43be066c1dd418e32f5ad258e31a150a.

Note that before the change, there was:

   if (*sv) {
      if (check_sampler_swizzle(st, stObj, *sv, glsl_version) ||
          (format != (*sv)->format) ||
          gl_target_to_pipe(stObj->base.Target) != (*sv)->target ||
stObj->base.MinLevel + stObj->base.BaseLevel != (*sv)->u.tex.first_level ||
          last_level(stObj) != (*sv)->u.tex.last_level ||
          stObj->base.MinLayer != (*sv)->u.tex.first_layer ||
          last_layer(stObj) != (*sv)->u.tex.last_layer) {
         pipe_sampler_view_reference(sv, NULL);
      }
   }

   if (!*sv) {
      *sv = st_create_texture_sampler_view_from_stobj(st, stObj,
format, glsl_version);

In particular, note the stObj->base.MinLayer != (*sv)->u.tex.first_layer part. Sure, the st_vdpau code would create a sampler with first_layer == 1, but that sampler would almost immediately get replaced by one with first_layer == 0...

So at least I don't see how the patch I sent could regress anything (even in combination with Brian's earlier commit).

But yes, it's quite possible that the other path was simply broken before without anybody noticing it. How can I test exercising the non-DMABUF path?

Cheers,
Nicolai



Regards,
Christian.


Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98512
Cc: 13.0 <mesa-sta...@lists.freedesktop.org>
---
  src/mesa/state_tracker/st_vdpau.c | 12 ------------
  1 file changed, 12 deletions(-)

diff --git a/src/mesa/state_tracker/st_vdpau.c
b/src/mesa/state_tracker/st_vdpau.c
index 7912057..770f0ff 100644
--- a/src/mesa/state_tracker/st_vdpau.c
+++ b/src/mesa/state_tracker/st_vdpau.c
@@ -182,21 +182,20 @@ static void
  st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum
access,
                       GLboolean output, struct gl_texture_object
*texObj,
                       struct gl_texture_image *texImage,
                       const void *vdpSurface, GLuint index)
  {
     struct st_context *st = st_context(ctx);
     struct st_texture_object *stObj = st_texture_object(texObj);
     struct st_texture_image *stImage = st_texture_image(texImage);
       struct pipe_resource *res;
-   struct pipe_sampler_view templ, **sampler_view;
     mesa_format texFormat;
       if (output) {
        res = st_vdpau_output_surface_dma_buf(ctx, vdpSurface);
          if (!res)
           res = st_vdpau_output_surface_gallium(ctx, vdpSurface);
       } else {
        res = st_vdpau_video_surface_dma_buf(ctx, vdpSurface, index);
@@ -226,31 +225,20 @@ st_vdpau_map_surface(struct gl_context *ctx,
GLenum target, GLenum access,
     texFormat = st_pipe_format_to_mesa_format(res->format);
       _mesa_init_teximage_fields(ctx, texImage,
                                res->width0, res->height0, 1, 0, GL_RGBA,
                                texFormat);
       pipe_resource_reference(&stObj->pt, res);
     st_texture_release_all_sampler_views(st, stObj);
     pipe_resource_reference(&stImage->pt, res);
  -   u_sampler_view_default_template(&templ, res, res->format);
-   templ.u.tex.first_layer = index & 1;
-   templ.u.tex.last_layer = index & 1;
-   templ.swizzle_r = GET_SWZ(stObj->base._Swizzle, 0);
-   templ.swizzle_g = GET_SWZ(stObj->base._Swizzle, 1);
-   templ.swizzle_b = GET_SWZ(stObj->base._Swizzle, 2);
-   templ.swizzle_a = GET_SWZ(stObj->base._Swizzle, 3);
-
-   sampler_view = st_texture_get_sampler_view(st, stObj);
-   *sampler_view = st->pipe->create_sampler_view(st->pipe, res, &templ);
-
     stObj->surface_format = res->format;
       _mesa_dirty_texobj(ctx, texObj);
     pipe_resource_reference(&res, NULL);
  }
    static void
  st_vdpau_unmap_surface(struct gl_context *ctx, GLenum target, GLenum
access,
                         GLboolean output, struct gl_texture_object
*texObj,
                         struct gl_texture_image *texImage,


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to