Re: [PATCH 4/5] d3dx11: Implemented D3DX11CreateShaderResourceViewFromFileW

[Nikolay Sivov]

On 13.09.2016 23:11, Fabian Maurer wrote:
> +HRESULT WINAPI D3DX11CreateShaderResourceViewFromFileW(ID3D11Device *iface,
> +        WCHAR *filename, D3DX11_IMAGE_LOAD_INFO *load_info_original, ID3DX11ThreadPump *pump, ID3D11ShaderResourceView **view, HRESULT *hresult)
> +{
> +    D3DX11_IMAGE_LOAD_INFO load_info;
> +    D3D11_SHADER_RESOURCE_VIEW_DESC desc_view;
> +    D3D11_TEXTURE2D_DESC desc_texture;
> +    D3D11_SUBRESOURCE_DATA data_texture;
> +    ID3D11Texture2D *texture;
> +    enum wined3d_format_id dst_format;
> +    UINT dst_pitch;
> +    D3DXIMAGE_INFO image_info;
> +    void *file_buffer = NULL;
> +    DWORD file_size;
> +    void *image_data = NULL;
> +    RECT rectSize;
> +    HRESULT hr;
> +
> +    TRACE("iface %p, filename %s, load_info_original %p, pump %p, view %p, hresult %p.\n",
> +            iface, debugstr_w(filename), load_info_original, pump, view, hresult);
> +
> +    if (pump)
> +        FIXME("Unimplemented ID3DX11ThreadPump handling.\n");

Personally I think this function should use
D3DX11CreateAsyncFileLoaderW() and
D3DX11CreateAsyncShaderResourceViewProcessor() internally, for both
async and synchronous cases. This will make support for a thread pump
trivial, and will consolidate things like map_view_of_file() in one place.

[PATCH 4/5] d3dx11: Implemented D3DX11CreateShaderResourceViewFromFileW

[Matteo Bruni]

2016-09-13 22:27 GMT+02:00 Nikolay Sivov <bunglehead(a)gmail.com>:
> On 13.09.2016 23:11, Fabian Maurer wrote:
> > +HRESULT WINAPI D3DX11CreateShaderResourceViewFromFileW(ID3D11Device *iface,
> > +        WCHAR *filename, D3DX11_IMAGE_LOAD_INFO *load_info_original, ID3DX11ThreadPump *pump, ID3D11ShaderResourceView **view, HRESULT *hresult)
> > +{
> > +    D3DX11_IMAGE_LOAD_INFO load_info;
> > +    D3D11_SHADER_RESOURCE_VIEW_DESC desc_view;
> > +    D3D11_TEXTURE2D_DESC desc_texture;
> > +    D3D11_SUBRESOURCE_DATA data_texture;
> > +    ID3D11Texture2D *texture;
> > +    enum wined3d_format_id dst_format;
> > +    UINT dst_pitch;
> > +    D3DXIMAGE_INFO image_info;
> > +    void *file_buffer = NULL;
> > +    DWORD file_size;
> > +    void *image_data = NULL;
> > +    RECT rectSize;
> > +    HRESULT hr;
> > +
> > +    TRACE("iface %p, filename %s, load_info_original %p, pump %p, view %p, hresult %p.\n",
> > +            iface, debugstr_w(filename), load_info_original, pump, view, hresult);
> > +
> > +    if (pump)
> > +        FIXME("Unimplemented ID3DX11ThreadPump handling.\n");
>
> Personally I think this function should use
> D3DX11CreateAsyncFileLoaderW() and
> D3DX11CreateAsyncShaderResourceViewProcessor() internally, for both
> async and synchronous cases. This will make support for a thread pump
> trivial, and will consolidate things like map_view_of_file() in one place.

Yeah, it would be nice. The async and sync cases should be able to
share most of the code, with the only difference being that in the
async case the function will call AddWorkItem on the thread pump
object while in the sync case it will call the data loader / data
processor methods itself.

While it might be an option to ignore the thread pump case in the
initial implementation, it seems clearly better to structure things to
use data loader + data processor objects from the start.