Re: [PATCH 5/5] d3dx11: Implemented D3DX11CompileFromFileW

[Matteo Bruni]

2016-09-13 22:11 GMT+02:00 Fabian Maurer <dark.shadow4(a)web.de>:
> Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
> ---
>  dlls/d3dx11_43/async.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/d3dx11_43/async.c b/dlls/d3dx11_43/async.c
> index 90c60fa..1e3042b 100644
> --- a/dlls/d3dx11_43/async.c
> +++ b/dlls/d3dx11_43/async.c
> @@ -1989,16 +1989,36 @@ HRESULT WINAPI D3DX11CompileFromFileA(const char *filename, const D3D10_SHADER_M
>      return E_NOTIMPL;
>  }
>
> +

Stray newline.
> HRESULT WINAPI D3DX11CompileFromFileW(const WCHAR *filename, const D3D10_SHADER_MACRO *defines,
>          ID3D10Include *include, const char *entry_point, const char *target, UINT sflags, UINT eflags,
>          ID3DX11ThreadPump *pump, ID3D10Blob **shader, ID3D10Blob **error_messages, HRESULT *hresult)
>  {
> -    FIXME("filename %s, defines %p, include %p, entry_point %s, target %s, sflags %#x, "
> +    void *file_buffer;
> +    HRESULT hr;
> +    DWORD file_size;
> +    LPSTR filename_converted;

Please use "char *" instead.
> +    UINT filename_converted_len;

Probably you can call it just "size".
> +
> +    TRACE("filename %s, defines %p, include %p, entry_point %s, target %s, sflags %#x, "
>              "eflags %#x, pump %p, shader %p, error_messages %p, hresult %p stub.\n",

The "stub" part should go away.
> debugstr_w(filename), defines, include, debugstr_a(entry_point), debugstr_a(target),
>              sflags, eflags, pump, shader, error_messages, hresult);
>
> -    return E_NOTIMPL;
> +    if (FAILED(hr = map_view_of_file(filename, &file_buffer, &file_size)))
> +        return hr;
> +
> +    filename_converted_len = WideCharToMultiByte(CP_ACP, 0, filename, -1, NULL, 0, NULL, NULL);
> +    filename_converted = HeapAlloc(GetProcessHeap(), 0, filename_converted_len);
> +    WideCharToMultiByte(CP_ACP, 0, filename, -1, filename_converted, filename_converted_len, NULL, NULL);
> +
> +    hr = D3DX11CompileFromMemory(file_buffer, file_size, filename_converted, defines, include, entry_point, target,
> +            sflags, eflags, pump, shader, error_messages, hresult);
> +
> +    HeapFree(GetProcessHeap(), 0, filename_converted);
> +    UnmapViewOfFile(file_buffer);
> +
> +    return hr;
>  }

This would be okay otherwise but, as mentioned for patch [4/5], maybe
it's a good idea to implement and make use of
D3DX11CreateAsyncFileLoaderW() here too.

[PATCH 5/5] d3dx11: Implemented D3DX11CompileFromFileW

[Fabian Maurer]

Thanks for your feedback.
> > +    UINT filename_converted_len;
>
> Probably you can call it just "size".
> Are you sure? I tried to make it less ambiguous, since there are two sizes 
> used.
> This would be okay otherwise but, as mentioned for patch [4/5], maybe
> it's a good idea to implement and make use of
> D3DX11CreateAsyncFileLoaderW() here too.

I fixed the other issues on my local branch and I'll try to get the async parts 
right.

[PATCH 5/5] d3dx11: Implemented D3DX11CompileFromFileW

[Matteo Bruni]

2016-09-20 1:56 GMT+02:00 Fabian Maurer <dark.shadow4(a)web.de>:
> Thanks for your feedback.
> > > +    UINT filename_converted_len;
> >
> > Probably you can call it just "size".
> > Are you sure? I tried to make it less ambiguous, since there are two sizes
> > used.

Right. Whatever is fine I guess, but using "_size" instead of "_len"
seems valuable for consistency.
> > This would be okay otherwise but, as mentioned for patch [4/5], maybe
> > it's a good idea to implement and make use of
> > D3DX11CreateAsyncFileLoaderW() here too.
>
> I fixed the other issues on my local branch and I'll try to get the async parts
> right.

Cool, looking forward to it.