Re: [PATCH 1/9] d2d1: ID2D1Factory1 test

[Nikolay Sivov]

On 21.11.2017 11:41, Lucian Poston wrote:
> https://bugs.winehq.org/show_bug.cgi?id=44052
> Signed-off-by: Lucian Poston
>
> dlls/d2d1/tests/Makefile.in |   2 +-
>  dlls/d2d1/tests/d2d1.c      | 145 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 146 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/d2d1/tests/Makefile.in b/dlls/d2d1/tests/Makefile.in
> index 91ede7888a..5eeb815e07 100644
> --- a/dlls/d2d1/tests/Makefile.in
> +++ b/dlls/d2d1/tests/Makefile.in
> @@ -1,5 +1,5 @@
>  TESTDLL   = d2d1.dll
> -IMPORTS   = d2d1 d3d10_1 dwrite dxguid uuid user32 advapi32 ole32 gdi32
> +IMPORTS   = d2d1 d3d10_1 d3d11 dwrite dxguid uuid user32 advapi32 ole32 gdi32
>
> C_SRCS = \
>    d2d1.c
> diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c
> index a0f22128fa..9c7c875b74 100644
> --- a/dlls/d2d1/tests/d2d1.c
> +++ b/dlls/d2d1/tests/d2d1.c
> @@ -20,6 +20,8 @@
>  #include <limits.h>
>  #include <math.h>
>  #include "d2d1.h"
> +#include "d2d1_1.h"

This can't work, because you're introducing this header in next patch.
The idea is that patch can depend only on previous not following patches
in series.
> +#include "d3d11.h"
>  #include "wincrypt.h"
>  #include "wine/test.h"
>  #include "initguid.h"
> @@ -609,6 +611,37 @@ static BOOL compare_figure(IDXGISurface *surface, unsigned int x, unsigned int y
>      return diff <= max_diff;
>  }
>
> +static ID3D11Device *create_d11device(ID3D11DeviceContext **outContext)
> +{
> - ID3D11Device *device;
> - ID3D11DeviceContext *context;
> - D3D_FEATURE_LEVEL feature_level = D3D_FEATURE_LEVEL_10_0;
> -
> - if (SUCCEEDED(D3D11CreateDevice(NULL, D3D_DRIVER_TYPE_HARDWARE, NULL,
> -
>        D3D11_CREATE_DEVICE_BGRA_SUPPORT, &feature_level,
> -
>        1, D3D11_SDK_VERSION, &device, NULL, &context)))
> - {
> -
>    if (outContext) *outContext = context;
> -
>    return device;
> - }
> - if (SUCCEEDED(D3D11CreateDevice(NULL, D3D_DRIVER_TYPE_WARP, NULL,
> -
>        D3D11_CREATE_DEVICE_BGRA_SUPPORT, &feature_level,
> -
>        1, D3D11_SDK_VERSION, &device, NULL, &context)))
> - {
> -
>    if (outContext) *outContext = context;
> -
>    return device;
> - }
> - if (SUCCEEDED(D3D11CreateDevice(NULL, D3D_DRIVER_TYPE_REFERENCE, NULL,
> -
>        D3D11_CREATE_DEVICE_BGRA_SUPPORT, &feature_level,
> -
>        1, D3D11_SDK_VERSION, &device, NULL, &context)))
> - {
> -
>    if (outContext) *outContext = context;
> -
>    return device;
> - }
> -
> - return NULL;
>
> +}
> -
>
> static ID3D10Device1 *create_device(void)

Why create_device() helper is not enough?
> +static void test_draw_text_layout_with_ID2D1Factory1(void)

Test function name does not reflect what's being tested.
> - D2D1_COLOR_F c;
> - D2D1_RECT_F r;
> - c.r = .5; c.g = .5; c.b = .5; c.a = .5;
> - r.top = 10; r.left = 10; r.bottom = 90; r.right = 90;

Please use existing helpers to initialize this.
> -
> - hr = D2D1CreateFactory(D2D1_FACTORY_TYPE_SINGLE_THREADED,
> -
>        &IID_ID2D1Factory1, NULL, (void**)&factory);
> - ok(SUCCEEDED(hr), "Failed to create factory, hr %#x.\n", hr);

We probably want to handle a case when newer interface is not supported.