Re: [PATCH 1/2] wined3d: Make (wined3d_)surface_depth_blt_fbo handle locations other than SFLAG_INTEXTURE.

[Henri Verbeet]

On 17 February 2013 18:10, Matteo Bruni  wrote:
> @@ -6168,18 +6172,10 @@ HRESULT surface_load_location(struct wined3d_surface *surface, DWORD location, c
>  if (surface->resource.usage & WINED3DUSAGE_DEPTHSTENCIL)
>  {
> -
>    if (location == SFLAG_INTEXTURE)
> -
>    {
> -
>        struct wined3d_context *context = context_acquire(device, NULL);
> -
>        surface_load_ds_location(surface, context, location);
> -
>        context_release(context);
> -
>        return WINED3D_OK;
> -
>    }
> -
>    else
> -
>    {
> -
>        FIXME("Unimplemented location %s for depth/stencil buffers.\n", debug_surflocation(location));
> -
>        return WINED3DERR_INVALIDCALL;
> -
>    }
> -
>    struct wined3d_context *context = context_acquire(device, NULL);
> -
>    surface_load_ds_location(surface, context, location);
> -
>    context_release(context);
> -
>    return WINED3D_OK;
> }

This is probably wrong (and the original code is probably questionable
too), surface_load_ds_location() is only really meant for onscreen <->
offscreen blits. The original code is meant for the case where
AlwaysOffscreen is disabled and a depth stencil is used as texture.
The implication there is that the only possible locations are
SFLAG_INTEXTURE and SFLAG_INDRAWABLE, it doesn't really work with
multisampling. Fortunately you can't directly texture from
multisampled depth stencil surfaces either.

In principle we could make surface_load_ds_location() the generic
function for depth / stencil location transfers, but currently it
isn't. If you're hitting it with the RESZ code it probably only works
by accident.

[PATCH 1/2] wined3d: Make (wined3d_)surface_depth_blt_fbo handle locations other than SFLAG_INTEXTURE.

[Matteo Bruni]

2013/2/17 Henri Verbeet :
> On 17 February 2013 18:10, Matteo Bruni  wrote:
> > @@ -6168,18 +6172,10 @@ HRESULT surface_load_location(struct wined3d_surface *surface, DWORD location, c
> >  if (surface->resource.usage & WINED3DUSAGE_DEPTHSTENCIL)
> >  {
> > -
> >    if (location == SFLAG_INTEXTURE)
> > -
> >    {
> > -
> >        struct wined3d_context *context = context_acquire(device, NULL);
> > -
> >        surface_load_ds_location(surface, context, location);
> > -
> >        context_release(context);
> > -
> >        return WINED3D_OK;
> > -
> >    }
> > -
> >    else
> > -
> >    {
> > -
> >        FIXME("Unimplemented location %s for depth/stencil buffers.\n", debug_surflocation(location));
> > -
> >        return WINED3DERR_INVALIDCALL;
> > -
> >    }
> > -
> >    struct wined3d_context *context = context_acquire(device, NULL);
> > -
> >    surface_load_ds_location(surface, context, location);
> > -
> >    context_release(context);
> > -
> >    return WINED3D_OK;
> > }
>
> This is probably wrong (and the original code is probably questionable
> too), surface_load_ds_location() is only really meant for onscreen <->
> offscreen blits. The original code is meant for the case where
> AlwaysOffscreen is disabled and a depth stencil is used as texture.
> The implication there is that the only possible locations are
> SFLAG_INTEXTURE and SFLAG_INDRAWABLE, it doesn't really work with
> multisampling. Fortunately you can't directly texture from
> multisampled depth stencil surfaces either.
>
> In principle we could make surface_load_ds_location() the generic
> function for depth / stencil location transfers, but currently it
> isn't. If you're hitting it with the RESZ code it probably only works
> by accident.

I don't think I actually need any call to surface_load_ds_location() for RESZ.

What happens is that surface_depth_blt_fbo() wants to make sure the
depth buffer is current. It does that by calling
surface_load_location() with SFLAG_INRB_MULTISAMPLE location, if the
depth buffer is multisampled, but in that case the depth buffer is up
to date already. You're right that it doesn't make sense to call
surface_load_ds_location() (in its current form, anyway) at all here,
so what about this instead?

[PATCH 1/2] wined3d: Make (wined3d_)surface_depth_blt_fbo handle locations other than SFLAG_INTEXTURE.

[Henri Verbeet]

On 18 February 2013 01:24, Matteo Bruni  wrote:
> to date already. You're right that it doesn't make sense to call
> surface_load_ds_location() (in its current form, anyway) at all here,
> so what about this instead?

Yeah, I think that makes more sense.