Re: [PATCH 09/10] d3dx9: Use intermediate surfaces for loading from unmappable source surfaces in D3DXLoadSurfaceFromSurface().

[Henri Verbeet]

On Mon, 19 Nov 2018 at 21:52, Matteo Bruni  wrote:
> On Sun, Nov 18, 2018 at 7:40 PM Henri Verbeet  wrote:
> > @@ -1937,23 +1939,37 @@ HRESULT WINAPI D3DXLoadSurfaceFromSurface(IDirect3DSurface9 *dst_surface,
> >      if (!dst_surface || !src_surface)
> >          return D3DERR_INVALIDCALL;
> > - IDirect3DSurface9_GetDesc(src_surface, &SrcDesc);
> > - IDirect3DSurface9_GetDesc(src_surface, &src_desc);
> > - if (src_desc.Pool == D3DPOOL_DEFAULT && !(src_desc.Usage & D3DUSAGE_DYNAMIC))
> > - {
> > -
> >    IDirect3DSurface9_GetDevice(src_surface, &device);
> > -
> >    if (FAILED(IDirect3DDevice9_CreateRenderTarget(device, src_desc.Width, src_desc.Height,
> > -
> >            src_desc.Format, D3DMULTISAMPLE_NONE, 0, TRUE, &surface, NULL))
> > -
> >            || FAILED(IDirect3DDevice9_StretchRect(device, src_surface, NULL, surface, NULL, D3DTEXF_NONE)))
> > -
> >        surface = src_surface;
> > -
> >    IDirect3DDevice9_Release(device);
> > - }
>
> Doesn't this leak the staging RT if StretchRect() fails?

Right, it would. I'll fix that.
> I'm also unsure whether StretchRect() covers all the cases of our
> interest. After staring at the StretchRect() tests + docs for a while,
> my current understanding is that it should always work for us, we're
> probably doing an extra copy for off-screen plain surfaces but that's
> no big deal.

Well, there's one case we really care about, and that's regular
non-dynamic default pool textures. StretchRect() is the only way to
get the data out of those, GetRenderTargetData() isn't an option. It's
true that for e.g. render targets and offscreen plain surfaces there
are more efficient options, but I'd consider those optimisations. Note
though that after the next patch in this series, this is essentially a
fall-back path for when we can't StretchRect() directly between the
source and destination.