Re: [PATCH 04/11] ddraw: Create the second texture for system memory bindable surfaces.

[Henri Verbeet]

On Mon, 8 Mar 2021 at 11:20, Paul Gofman  wrote:
> On 3/5/21 21:36, Henri Verbeet wrote:
> > On Wed, 3 Mar 2021 at 23:36, Paul Gofman  wrote:
> > > +static void ddraw_surface_create_draw_texture(struct ddraw_surface *surface)
> > > +{
> > > - DDSURFACEDESC2 *desc = &surface->surface_desc;
> > > - struct wined3d_resource *draw_texture_resource;
> > > - struct wined3d_resource_desc wined3d_desc;
> > > - unsigned int i, layer_count, level_count;
> > > - struct wined3d_texture *draw_texture;
> > > - struct ddraw_surface *parent;
> > > - unsigned int bind_flags;
> > > - HRESULT hr;
> > > -
> > > - if (!(desc->ddsCaps.dwCaps & DDSCAPS_SYSTEMMEMORY))
> > > -
> > >    return;
> > > -
> >
> > Does that check do the right thing? Note that we add
> > DDSCAPS_SYSTEMMEMORY for managed textures, but these still have
> > WINED3D_RESOURCE_ACCESS_GPU set.
>
> As far as I could see we add DDSCAPS_SYSTEMMEMORY for managed texture
> only if we detected that wined3d does not support the required bind
> flags. So it is not immediately clear to me why would we want to treat
> that case differently here, shouldn't we just have draw_texture the same
> way as for system memory texture without _TEXTUREMANAGE?

I think the block you're referring to is the following?

if (!(desc->ddsCaps.dwCaps & (DDSCAPS_VIDEOMEMORY | DDSCAPS_SYSTEMMEMORY)))
    {
        if (!(desc->ddsCaps.dwCaps2 & (DDSCAPS2_TEXTUREMANAGE |
DDSCAPS2_D3DTEXTUREMANAGE)))
        {
            ...
            if (... hr = wined3d_check_device_format(...)))
            {
                ...
            }
            else
            {
                desc->ddsCaps.dwCaps |= DDSCAPS_SYSTEMMEMORY;
                sysmem_fallback = TRUE;
            }
            ...
        }
        ...
    }

but note that that's for unmanaged textures.

The block I'm referring to is a bit further down:

if (desc->ddsCaps.dwCaps & DDSCAPS_SYSTEMMEMORY)
    {
        ...
    }
    else
    {
        ...
        if (desc->ddsCaps.dwCaps2 & (DDSCAPS2_TEXTUREMANAGE |
DDSCAPS2_D3DTEXTUREMANAGE))
        {
            ...
            wined3d_desc.access = WINED3D_RESOURCE_ACCESS_GPU |
WINED3D_RESOURCE_ACCESS_CPU
                | WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W;
            /* Managed textures have the system memory flag set. */
            desc->ddsCaps.dwCaps |= DDSCAPS_SYSTEMMEMORY;
        }
        ...
    }

I.e., managed textures get DDSCAPS_SYSTEMMEMORY, but also both
WINED3D_RESOURCE_ACCESS_GPU and WINED3D_RESOURCE_ACCESS_CPU. It may be
more robust to check whether the existing texture has
WINED3D_RESOURCE_ACCESS_GPU in ddraw_surface_create_draw_texture(),
since ultimately that's what we care about anyway.