[PATCH] d3dcompiler: Implement D3DReadFileToBlob().

[Jactry Zeng]

Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com>
---
 dlls/d3dcompiler_43/blob.c       |  54 +++++++++++++++-
 dlls/d3dcompiler_43/tests/blob.c | 106 +++++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+), 2 deletions(-)

diff --git a/dlls/d3dcompiler_43/blob.c b/dlls/d3dcompiler_43/blob.c
index f22dc7183d..6b1b20317f 100644
--- a/dlls/d3dcompiler_43/blob.c
+++ b/dlls/d3dcompiler_43/blob.c
@@ -466,9 +466,59 @@ HRESULT WINAPI D3DStripShader(const void *data, SIZE_T data_size, UINT flags, ID
 
 HRESULT WINAPI D3DReadFileToBlob(const WCHAR *filename, ID3DBlob **contents)
 {
-    FIXME("filename %s, contents %p\n", debugstr_w(filename), contents);
+    struct d3dcompiler_blob *object;
+    HANDLE file;
+    SIZE_T data_size;
+    DWORD read_size;
+    HRESULT hr;
 
-    return E_NOTIMPL;
+    TRACE("filename %s, contents %p\n", debugstr_w(filename), contents);
+
+    file = CreateFileW(filename, GENERIC_READ, FILE_SHARE_READ, NULL,
+                       OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (file == INVALID_HANDLE_VALUE)
+        return HRESULT_FROM_WIN32(GetLastError());
+
+    data_size = GetFileSize(file, NULL);
+    if (data_size == INVALID_FILE_SIZE)
+    {
+        CloseHandle(file);
+        return HRESULT_FROM_WIN32(GetLastError());
+    }
+
+    object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object));
+    if (!object)
+    {
+        CloseHandle(file);
+        return E_OUTOFMEMORY;
+    }
+
+    hr = d3dcompiler_blob_init(object, data_size);
+    if (FAILED(hr))
+    {
+        WARN("Failed to initialize blob, hr %#x.\n", hr);
+        CloseHandle(file);
+        HeapFree(GetProcessHeap(), 0, object);
+        return hr;
+    }
+
+    if (!ReadFile(file, object->data, data_size, &read_size, NULL) ||
+        (read_size != data_size))
+    {
+        WARN("Failed to read file contents.\n");
+        CloseHandle(file);
+        HeapFree(GetProcessHeap(), 0, object->data);
+        HeapFree(GetProcessHeap(), 0, object);
+        return E_FAIL;
+    }
+    CloseHandle(file);
+    object->size = read_size;
+
+    *contents = &object->ID3DBlob_iface;
+
+    TRACE("Returning ID3DBlob %p.\n", *contents);
+
+    return S_OK;
 }
 
 HRESULT WINAPI D3DWriteBlobToFile(ID3DBlob* blob, const WCHAR *filename, BOOL overwrite)
diff --git a/dlls/d3dcompiler_43/tests/blob.c b/dlls/d3dcompiler_43/tests/blob.c
index 443ed2754c..6a9fa5de8b 100644
--- a/dlls/d3dcompiler_43/tests/blob.c
+++ b/dlls/d3dcompiler_43/tests/blob.c
@@ -35,6 +35,7 @@
 
 static HRESULT (WINAPI *pD3DCreateBlob)(SIZE_T, ID3DBlob **);
 static HRESULT (WINAPI *pD3DGetBlobPart)(const void *, SIZE_T, D3D_BLOB_PART, UINT, ID3DBlob **);
+static HRESULT (WINAPI *pD3DReadFileToBlob)(const WCHAR *, ID3DBlob **);
 static HRESULT (WINAPI *pD3DStripShader)(const void *, SIZE_T, UINT, ID3DBlob **);
 
 #define MAKE_TAG(ch0, ch1, ch2, ch3) \
@@ -752,6 +753,103 @@ static BOOL load_d3dcompiler(void)
     return TRUE;
 }
 
+static BOOL load_d3dcompiler_47(void)
+{
+    HMODULE module;
+
+    if (!(module = LoadLibraryA("d3dcompiler_47.dll"))) return FALSE;
+
+    pD3DReadFileToBlob = (void*)GetProcAddress(module, "D3DReadFileToBlob");
+    return TRUE;
+}
+
+static void create_cso_file(LPCWSTR pathW, void *data, DWORD data_size)
+{
+    HANDLE file;
+    DWORD written;
+
+    file = CreateFileW(pathW, GENERIC_READ | GENERIC_WRITE, 0,
+                       NULL, CREATE_ALWAYS, 0, 0);
+    ok(file != INVALID_HANDLE_VALUE, "File creation failed, at %s, error 0x%08x.\n",
+       wine_dbgstr_w(pathW), GetLastError());
+
+    if (data)
+    {
+        WriteFile(file, data, data_size, &written, NULL);
+        ok(written == data_size, "Couldn't write .cso file.\n" );
+    }
+    CloseHandle(file);
+}
+
+/* .cso file compiled by fxc.exe.
+   HLSL source:
+   ```
+   struct PSInput
+   {
+       float4 value : SV_POSITION;
+   };
+
+   PSInput main(float4 position : POSITION)
+   {
+       PSInput result;
+       result.value = position;
+       return result;
+   }
+   ```
+ */
+static byte test_cso_data[] =
+{
+    0x00,0x02,0xfe,0xff,0xfe,0xff,0x14,0x00,0x43,0x54,0x41,0x42,0x1c,0x00,0x00,0x00,0x23,0x00,
+    0x00,0x00,0x00,0x02,0xfe,0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
+    0x1c,0x00,0x00,0x00,0x76,0x73,0x5f,0x32,0x5f,0x30,0x00,0x4d,0x69,0x63,0x72,0x6f,0x73,0x6f,
+    0x66,0x74,0x20,0x28,0x52,0x29,0x20,0x48,0x4c,0x53,0x4c,0x20,0x53,0x68,0x61,0x64,0x65,0x72,
+    0x20,0x43,0x6f,0x6d,0x70,0x69,0x6c,0x65,0x72,0x20,0x31,0x30,0x2e,0x31,0x00,0xab,0x1f,0x00,
+    0x00,0x02,0x00,0x00,0x00,0x80,0x00,0x00,0x0f,0x90,0x01,0x00,0x00,0x02,0x00,0x00,0x0f,0xc0,
+    0x00,0x00,0xe4,0x90,0xff,0xff,0x00,0x00
+};
+
+static void test_D3DReadFileToBlob(void)
+{
+    ID3DBlob *blob = NULL;
+    HRESULT hr;
+    static const WCHAR filenameW[] = {'t','e','s','t','.','c','s','o',0};
+    byte *data_enter;
+    SIZE_T data_size;
+
+    hr = pD3DReadFileToBlob(filenameW, NULL);
+    ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
+       "D3DReadFileToBlob returned: 0x%08x.\n", hr);
+
+    hr = pD3DReadFileToBlob(filenameW, &blob);
+    ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
+       "D3DReadFileToBlob returned: 0x%08x.\n", hr);
+
+    /* Crash on Windows
+    create_cso_file(filenameW, test_cso_data, sizeof(test_cso_data));
+    pD3DReadFileToBlob(filenameW, NULL);
+    DeleteFileW(filenameW);
+    */
+
+    create_cso_file(filenameW, NULL, 0);
+    hr = pD3DReadFileToBlob(filenameW, &blob);
+    ok(hr == S_OK, "D3DReadFileToBlob failed: 0x%08x.\n", hr);
+    data_size = ID3D10Blob_GetBufferSize(blob);
+    ok(data_size == 0, "got wrong data size: %lu, expected %u.\n", data_size, 0);
+    DeleteFileW(filenameW);
+    ID3D10Blob_Release(blob);
+
+    create_cso_file(filenameW, test_cso_data, sizeof(test_cso_data));
+    hr = pD3DReadFileToBlob(filenameW, &blob);
+    ok(hr == S_OK, "D3DReadFileToBlob failed: 0x%08x.\n", hr);
+    data_size = ID3D10Blob_GetBufferSize(blob);
+    ok(data_size == sizeof(test_cso_data), "got wrong data size: %lu, expected %u.\n",
+       data_size, sizeof(test_cso_data));
+    data_enter = ID3D10Blob_GetBufferPointer(blob);
+    ok(!memcmp(data_enter, test_cso_data, sizeof(test_cso_data)), "got wrong data.\n");
+    DeleteFileW(filenameW);
+    ID3D10Blob_Release(blob);
+}
+
 START_TEST(blob)
 {
     if (!load_d3dcompiler())
@@ -763,4 +861,12 @@ START_TEST(blob)
     test_create_blob();
     test_get_blob_part();
     test_get_blob_part2();
+
+    if (!load_d3dcompiler_47())
+    {
+        win_skip("Could not load d3dcompiler_47.dll\n");
+        return;
+    }
+
+    test_D3DReadFileToBlob();
 }
-- 
2.20.1

[Matteo Bruni]

On Fri, Apr 26, 2019 at 12:16 PM Jactry Zeng <jzeng(a)codeweavers.com> wrote:
> Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com>
> ---
>  dlls/d3dcompiler_43/blob.c       |  54 +++++++++++++++-
>  dlls/d3dcompiler_43/tests/blob.c | 106 +++++++++++++++++++++++++++++++
>  2 files changed, 158 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/d3dcompiler_43/blob.c b/dlls/d3dcompiler_43/blob.c
> index f22dc7183d..6b1b20317f 100644
> --- a/dlls/d3dcompiler_43/blob.c
> +++ b/dlls/d3dcompiler_43/blob.c
> @@ -466,9 +466,59 @@ HRESULT WINAPI D3DStripShader(const void *data, SIZE_T data_size, UINT flags, ID
>
> HRESULT WINAPI D3DReadFileToBlob(const WCHAR *filename, ID3DBlob **contents)
>  {
> -    FIXME("filename %s, contents %p\n", debugstr_w(filename), contents);
> +    struct d3dcompiler_blob *object;
> +    HANDLE file;
> +    SIZE_T data_size;
> +    DWORD read_size;
> +    HRESULT hr;

Nitpicking, but please sort declarations in reverse Christmas tree
order (i.e. longest line to shortest)
> -    return E_NOTIMPL;
> +    TRACE("filename %s, contents %p\n", debugstr_w(filename), contents);

Period at the end of the message.
> +
> +    file = CreateFileW(filename, GENERIC_READ, FILE_SHARE_READ, NULL,
> +                       OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);

Please use 8 space indentation for line continuations.
> +    if (file == INVALID_HANDLE_VALUE)
> +        return HRESULT_FROM_WIN32(GetLastError());
> +
> +    data_size = GetFileSize(file, NULL);
> +    if (data_size == INVALID_FILE_SIZE)
> +    {
> +        CloseHandle(file);
> +        return HRESULT_FROM_WIN32(GetLastError());
> +    }
> +
> +    object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object));

Please use the heap_alloc_zero() / heap_free() helpers.
> +    if (!object)
> +    {
> +        CloseHandle(file);
> +        return E_OUTOFMEMORY;
> +    }
> +
> +    hr = d3dcompiler_blob_init(object, data_size);
> +    if (FAILED(hr))
> +    {
> +        WARN("Failed to initialize blob, hr %#x.\n", hr);
> +        CloseHandle(file);
> +        HeapFree(GetProcessHeap(), 0, object);
> +        return hr;
> +    }

More nitpicking: you can put these two assignments inside the
respective if conditions.
> +
> +    if (!ReadFile(file, object->data, data_size, &read_size, NULL) ||
> +        (read_size != data_size))

Please put the operator at the start of the line.
> +static void create_cso_file(LPCWSTR pathW, void *data, DWORD data_size)

const WCHAR *filename

There isn't anything specific to .cso files in this function, I'd just
call it create_file() or something like that.
> +{
> +    HANDLE file;
> +    DWORD written;
> +
> +    file = CreateFileW(pathW, GENERIC_READ | GENERIC_WRITE, 0,
> +                       NULL, CREATE_ALWAYS, 0, 0);

It doesn't really matter but you don't need GENERIC_READ.
> +    ok(file != INVALID_HANDLE_VALUE, "File creation failed, at %s, error 0x%08x.\n",
> +       wine_dbgstr_w(pathW), GetLastError());

I don't know that I like failing the test if for whatever reason we
can't create the file. I guess it's okay, although I'd find it
slightly preferable to win_skip instead.
> +/* .cso file compiled by fxc.exe.
> +   HLSL source:
> +   ```
> +   struct PSInput
> +   {
> +       float4 value : SV_POSITION;
> +   };
> +
> +   PSInput main(float4 position : POSITION)
> +   {
> +       PSInput result;
> +       result.value = position;
> +       return result;
> +   }
> +   ```
> + */

Probably better to use the same style as the d3d11 tests (i.e. inside
an #if 0 - #endif).
> +static byte test_cso_data[] =

You want BYTE here (or even better DWORD, although you need to update
the definition too in that case).
> +static void test_D3DReadFileToBlob(void)
> +{
> +    ID3DBlob *blob = NULL;
> +    HRESULT hr;
> +    static const WCHAR filenameW[] = {'t','e','s','t','.','c','s','o',0};
> +    byte *data_enter;

Same here. You can also call it just "data".
> +    SIZE_T data_size;
> +
> +    hr = pD3DReadFileToBlob(filenameW, NULL);
> +    ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
> +       "D3DReadFileToBlob returned: 0x%08x.\n", hr);

This crashes if you happen to have a "test.cso" file around. Which I
had from a previous run of the test (which in turn happened because I
forgot to rebuild d3dcompiler_47 before running the test).
Since you don't need to use a specific file name, probably better to
just have it generated automatically by GetTempFileName() or similar.

Also, we generally use %#x for HRESULT traces.
> +    hr = pD3DReadFileToBlob(filenameW, &blob);
> +    ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
> +       "D3DReadFileToBlob returned: 0x%08x.\n", hr);
> +
> +    /* Crash on Windows
> +    create_cso_file(filenameW, test_cso_data, sizeof(test_cso_data));
> +    pD3DReadFileToBlob(filenameW, NULL);
> +    DeleteFileW(filenameW);
> +    */

I'd prefer that to be put inside an if (0) {} instead, so that it is
still compiled.
> +    create_cso_file(filenameW, NULL, 0);

Please create the test files in the Windows temporary files directory.
> +    hr = pD3DReadFileToBlob(filenameW, &blob);
> +    ok(hr == S_OK, "D3DReadFileToBlob failed: 0x%08x.\n", hr);
> +    data_size = ID3D10Blob_GetBufferSize(blob);
> +    ok(data_size == 0, "got wrong data size: %lu, expected %u.\n", data_size, 0);

Please avoid tracing sizeof() results. Also !data_size is preferred.