[PATCH] d3dx10: Parially implement async data loaders interfaces

[Nikolay Sivov]

On 07.11.2016 0:23, Matteo Bruni wrote:
> 2016-11-06 19:45 GMT+01:00 Nikolay Sivov <nsivov at codeweavers.com>:
>> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
>> ---
>>  dlls/d3dx10_43/async.c        | 296 ++++++++++++++++++++++++++++++++++++++++--
>>  dlls/d3dx10_43/tests/d3dx10.c |  36 ++---
>>  2 files changed, 306 insertions(+), 26 deletions(-)
>>
>> diff --git a/dlls/d3dx10_43/async.c b/dlls/d3dx10_43/async.c
>> index f49eb92..a9cbde5 100644
>> --- a/dlls/d3dx10_43/async.c
>> +++ b/dlls/d3dx10_43/async.c
> 
>> +/* Memory data loader. */
>> +static HRESULT WINAPI memorydataloader_Load(ID3DX10DataLoader *loader)
>> +{
>> +    TRACE("loader %p\n", loader);
> 
> We usually put a period at the end of trace messages.
> By the way, these comments don't add much IMO, it's pretty clear where
> each interface implementation starts and ends.

Ok.

> 
>> +static HRESULT WINAPI memorydataloader_Decompress(ID3DX10DataLoader *iface, void **data, SIZE_T *size)
>> +{
>> +    struct asyncdataloader *loader = impl_from_ID3DX10DataLoader(iface);
>> +
>> +    FIXME("loader %p, data %p, size %p semi-stub\n", loader, data, size);
>> +
>> +    *data = loader->data;
>> +    *size = loader->size;
>> +
>> +    return S_OK;
>> +}
> 
> Why the fixme? What's still missing here?

My understanding is that it should detect certain types of compression
and actually decompress if needed.

> 
>> +static HRESULT WINAPI filedataloader_Load(ID3DX10DataLoader *iface)
>> +{
>> +    struct asyncdataloader *loader = impl_from_ID3DX10DataLoader(iface);
>> +    HANDLE mapping;
>> +
>> +    TRACE("loader %p\n", loader);
>> +
>> +    if (loader->u.file == INVALID_HANDLE_VALUE)
>> +        return D3D10_ERROR_FILE_NOT_FOUND;
>> +
>> +    if (loader->data)
>> +        return S_OK;
>> +
>> +    GetFileSize(loader->u.file, (DWORD*)&loader->size);
>> +    mapping = CreateFileMappingW(loader->u.file, NULL, PAGE_READONLY, 0, 0, NULL);
>> +    if (!mapping)
>> +        return E_FAIL;
>> +
>> +    loader->data = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, 0);
>> +    CloseHandle(mapping);
>> +    if (!loader->data)
>> +        return E_FAIL;
>> +
>> +    return S_OK;
>> +}
> 
> Creating and keeping a file mapping open might not be correct. What
> happens on Windows if e.g. you try to delete the file after Load() but
> before Decompress()? The issue might also be between Decompress() and
> Destroy().

Sure, that should be easy enough to test.

> 
> I guess I'd like some more tests for the file and the resource loaders...
> 
>> +    if (!(rsrc = FindResourceA(module, resource, (LPSTR)RT_RCDATA)))
> 
> Please avoid LPtypes and casting away const, here and below.
> 

Ok.