[PATCH v4 0/2] MR979: d3d9: Implement d3dx_effect_SetRawValue
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46012 Signed-off-by: Patrick Hibbs <hibbsncc1701(a)gmail.com> -- v4: d3dx9/tests: Implement tests for d3dx_effect_SetRawValue(). d3dx9: Implement d3dx_effect_SetRawValue https://gitlab.winehq.org/wine/wine/-/merge_requests/979
From: Patrick Hibbs <hibbsncc1701(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46012 Signed-off-by: Patrick Hibbs <hibbsncc1701(a)gmail.com> --- V2: Fix prefix in commit subject. --- dlls/d3dx9_36/effect.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 7719edd4f25..f82d69d1acd 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -4442,10 +4442,26 @@ static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevic static HRESULT WINAPI d3dx_effect_SetRawValue(ID3DXEffect *iface, D3DXHANDLE parameter, const void *data, UINT byte_offset, UINT bytes) { - FIXME("iface %p, parameter %p, data %p, byte_offset %u, bytes %u stub!\n", + struct d3dx_effect *src = impl_from_ID3DXEffect( iface ); + struct d3dx_parameter *param = get_valid_parameter( src, parameter ); + void *raw = NULL; + + TRACE("iface %p, parameter %p, data %p, byte_offset %u, bytes %u.\n", iface, parameter, data, byte_offset, bytes); - return E_NOTIMPL; + if (!src) + { + WARN("Invalid variable.\n"); + return E_FAIL; + } + + if (!data || !param || !(raw = param_get_data_and_dirtify( src, param, + byte_offset + bytes, TRUE ))) + return D3DERR_INVALIDCALL; + + memcpy( raw, data, bytes ); + + return S_OK; } #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/979
From: Patrick Hibbs <hibbsncc1701(a)gmail.com> Signed-off-by: Patrick Hibbs <hibbsncc1701(a)gmail.com> --- v2: Actually use a real datatype instead of an enum when calling sizeof(). --- dlls/d3dx9_36/tests/effect.c | 599 +++++++++++++++++++++++++++++++++++ 1 file changed, 599 insertions(+) diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c index 12af35cb8d8..dd503bd6cf6 100644 --- a/dlls/d3dx9_36/tests/effect.c +++ b/dlls/d3dx9_36/tests/effect.c @@ -8703,6 +8703,604 @@ static void test_effect_parameter_block(void) DestroyWindow(window); } +/* + * fxc.exe /Tfx_2_0 + */ +#if 0 +bool b = false; +bool4 b4 = {false,false,false,false}; +bool4x4 b44 = {{false,false,false,false}, {false,false,false,false}, + {false,false,false,false}, {false,false,false,false}}; +int i = 0; +int4 i4 = {0,0,0,0}; +int4x4 i44 = {{4411, 4421, 4431, 4441}, {4412, 4422, 4432, 4442}, + {4413, 4423, 4433, 4443}, {4414, 4424, 4434, 4444}}; +float f = 0.0; +float4 f4 = {4.1, 4.2, 4.3, 4.4}; +float4x4 f44 = {{4.1, 4.2, 4.3, 4.4}, {4.5, 4.6, 4.7, 4.8}, + {4.9, 4.10, 4.11, 4.12}, {4.13, 4.14, 4.15, 4.16}}; +string s = "blank"; +string s_2[2] = {"blank1", "blank2"}; +texture tex; +texture2D tex2d; +texture2D tex3d; +Vertexshader v; +Vertexshader v_2[2]; +Pixelshader p; +Pixelshader p_2[2]; +technique t { pass p { } }; +#endif +static const BYTE test_set_raw_value_blob[] = +{ + 1, 9, 255, 254, 136, 3, + 0, 0, 0, 0, 0, 0, + 1, 0, 0, 0, 0, 0, + 0, 0, 36, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, + 1, 0, 0, 0, 0, 0, + 0, 0, 2, 0, 0, 0, + 98, 0, 0, 0, 1, 0, + 0, 0, 1, 0, 0, 0, + 88, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 4, 0, 0, 0, 1, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 3, 0, 0, 0, 98, 52, + 0, 0, 1, 0, 0, 0, + 2, 0, 0, 0, 188, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 4, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 98, 52, 52, 0, + 2, 0, 0, 0, 0, 0, + 0, 0, 228, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, + 1, 0, 0, 0, 0, 0, + 0, 0, 2, 0, 0, 0, + 105, 0, 0, 0, 2, 0, + 0, 0, 1, 0, 0, 0, + 24, 1, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 4, 0, 0, 0, 1, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 3, 0, 0, 0, 105, 52, + 0, 0, 2, 0, 0, 0, + 2, 0, 0, 0, 124, 1, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 4, 0, 0, 0, + 59, 17, 0, 0, 69, 17, + 0, 0, 79, 17, 0, 0, + 89, 17, 0, 0, 60, 17, + 0, 0, 70, 17, 0, 0, + 80, 17, 0, 0, 90, 17, + 0, 0, 61, 17, 0, 0, + 71, 17, 0, 0, 81, 17, + 0, 0, 91, 17, 0, 0, + 62, 17, 0, 0, 72, 17, + 0, 0, 82, 17, 0, 0, + 92, 17, 0, 0, 4, 0, + 0, 0, 105, 52, 52, 0, + 3, 0, 0, 0, 0, 0, + 0, 0, 164, 1, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, + 1, 0, 0, 0, 0, 0, + 0, 0, 2, 0, 0, 0, + 102, 0, 0, 0, 3, 0, + 0, 0, 1, 0, 0, 0, + 216, 1, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 4, 0, 0, 0, 1, 0, + 0, 0, 51, 51, 131, 64, + 102, 102, 134, 64, 154, 153, + 137, 64, 205, 204, 140, 64, + 3, 0, 0, 0, 102, 52, + 0, 0, 3, 0, 0, 0, + 2, 0, 0, 0, 60, 2, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 4, 0, 0, 0, + 51, 51, 131, 64, 102, 102, + 134, 64, 154, 153, 137, 64, + 205, 204, 140, 64, 0, 0, + 144, 64, 51, 51, 147, 64, + 102, 102, 150, 64, 154, 153, + 153, 64, 205, 204, 156, 64, + 51, 51, 131, 64, 31, 133, + 131, 64, 10, 215, 131, 64, + 246, 40, 132, 64, 225, 122, + 132, 64, 205, 204, 132, 64, + 184, 30, 133, 64, 4, 0, + 0, 0, 102, 52, 52, 0, + 4, 0, 0, 0, 4, 0, + 0, 0, 92, 2, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, + 2, 0, 0, 0, 115, 0, + 0, 0, 4, 0, 0, 0, + 4, 0, 0, 0, 128, 2, + 0, 0, 0, 0, 0, 0, + 2, 0, 0, 0, 2, 0, + 0, 0, 3, 0, 0, 0, + 4, 0, 0, 0, 115, 95, + 50, 0, 5, 0, 0, 0, + 4, 0, 0, 0, 160, 2, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 4, 0, 0, 0, + 116, 101, 120, 0, 7, 0, + 0, 0, 4, 0, 0, 0, + 192, 2, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 5, 0, 0, 0, 6, 0, + 0, 0, 116, 101, 120, 50, + 100, 0, 0, 0, 7, 0, + 0, 0, 4, 0, 0, 0, + 228, 2, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 6, 0, 0, 0, 6, 0, + 0, 0, 116, 101, 120, 51, + 100, 0, 0, 0, 16, 0, + 0, 0, 4, 0, 0, 0, + 8, 3, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 7, 0, 0, 0, 2, 0, + 0, 0, 118, 0, 0, 0, + 16, 0, 0, 0, 4, 0, + 0, 0, 44, 3, 0, 0, + 0, 0, 0, 0, 2, 0, + 0, 0, 8, 0, 0, 0, + 9, 0, 0, 0, 4, 0, + 0, 0, 118, 95, 50, 0, + 15, 0, 0, 0, 4, 0, + 0, 0, 76, 3, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 10, 0, 0, 0, + 2, 0, 0, 0, 112, 0, + 0, 0, 15, 0, 0, 0, + 4, 0, 0, 0, 112, 3, + 0, 0, 0, 0, 0, 0, + 2, 0, 0, 0, 11, 0, + 0, 0, 12, 0, 0, 0, + 4, 0, 0, 0, 112, 95, + 50, 0, 2, 0, 0, 0, + 112, 0, 0, 0, 2, 0, + 0, 0, 116, 0, 0, 0, + 18, 0, 0, 0, 1, 0, + 0, 0, 7, 0, 0, 0, + 13, 0, 0, 0, 4, 0, + 0, 0, 32, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 44, 0, 0, 0, + 72, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 96, 0, 0, 0, 124, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 196, 0, + 0, 0, 224, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 236, 0, 0, 0, + 8, 1, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 32, 1, 0, 0, 60, 1, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 132, 1, + 0, 0, 160, 1, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 172, 1, 0, 0, + 200, 1, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 224, 1, 0, 0, 252, 1, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 68, 2, + 0, 0, 88, 2, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 100, 2, 0, 0, + 120, 2, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 136, 2, 0, 0, 156, 2, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 168, 2, + 0, 0, 188, 2, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 204, 2, 0, 0, + 224, 2, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 240, 2, 0, 0, 4, 3, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 16, 3, + 0, 0, 36, 3, 0, 0, + 0, 0, 0, 0, 0, 0, + 0, 0, 52, 3, 0, 0, + 72, 3, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, + 84, 3, 0, 0, 104, 3, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 128, 3, + 0, 0, 0, 0, 0, 0, + 1, 0, 0, 0, 120, 3, + 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 12, 0, + 0, 0, 0, 0, 0, 0, + 11, 0, 0, 0, 0, 0, + 0, 0, 12, 0, 0, 0, + 0, 0, 0, 0, 10, 0, + 0, 0, 0, 0, 0, 0, + 8, 0, 0, 0, 0, 0, + 0, 0, 9, 0, 0, 0, + 0, 0, 0, 0, 7, 0, + 0, 0, 0, 0, 0, 0, + 6, 0, 0, 0, 0, 0, + 0, 0, 5, 0, 0, 0, + 0, 0, 0, 0, 4, 0, + 0, 0, 0, 0, 0, 0, + 2, 0, 0, 0, 7, 0, + 0, 0, 98, 108, 97, 110, + 107, 49, 0, 0, 3, 0, + 0, 0, 7, 0, 0, 0, + 98, 108, 97, 110, 107, 50, + 0, 0, 1, 0, 0, 0, + 6, 0, 0, 0, 98, 108, + 97, 110, 107, 0, 0, 0 +}; + +struct test_set_raw_data_value +{ + D3DXPARAMETER_TYPE param_type; + unsigned int expected_data_len; + unsigned int param_data_len; + unsigned int offset; + HRESULT expected_result; + const void *expected_data; + const char *param_name; + const void *param_data; +}; + +static const int SET_RAWDATA_BOOL44_A[16] = { 1 }; +static const int SET_RAWDATA_BOOL4_BA[16] = { 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; +static const int SET_RAWDATA_BOOL44_BA[16] = { 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; +static const int SET_RAWDATA_INT44_A[16] = { 2 }; +static const int SET_RAWDATA_INT44_B[16] = { 3 }; +static const int SET_RAWDATA_INT4_BA[16] = { (3 << 8) | 3, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 }; +static const int SET_RAWDATA_INT4_BBA[16] = { (3 << 8) | 3, 2, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 }; +static const int SET_RAWDATA_INT44_BA[16] = { (3 << 8) | 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; +static const int SET_RAWDATA_INT44_BBA[16] = { (3 << 8) | 3, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; +static const float SET_RAWDATA_FLOAT44_A[16] = { 1.0f }; +static const float SET_RAWDATA_FLOAT44_B[16] = { 1.2f }; +/* Note: The values below need to have index 0 updated with a value (1.0f) with + * an off-by-one byte alignment copied to them. + * See test_effect_setrawvalue_init_floats() below. + * + * E.x. arr[0] = (1.0f & 0x00FFFFFF) | 1.2f; + */ +static float SET_RAWDATA_FLOAT4_BA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, + 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f }; +static float SET_RAWDATA_FLOAT4_BBA[16] = { 1.2f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, + 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f }; +static float SET_RAWDATA_FLOAT44_BA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, + 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; +static float SET_RAWDATA_FLOAT44_BBA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, + 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; + +struct test_set_raw_data_value test_set_raw_data_values[] = +{ + /* All of these cause C0000005 on win10. + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), 0, D3DERR_INVALIDCALL, NULL, "b", NULL }, + { D3DXPT_BOOL, sizeof( BOOL ) * 4, sizeof( BOOL ) * 4, 0, D3DERR_INVALIDCALL, NULL, "b4", NULL }, + { D3DXPT_BOOL, sizeof( BOOL ) * 16, sizeof( BOOL ) * 16, 0, D3DERR_INVALIDCALL, NULL, "b44", NULL }, + { D3DXPT_INT, sizeof( DWORD ), sizeof( DWORD ), 0, D3DERR_INVALIDCALL, NULL, "i", NULL }, + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ) * 4, 0, D3DERR_INVALIDCALL, NULL, "i4", NULL }, + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ) * 16, 0, D3DERR_INVALIDCALL, NULL, "i44", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ), sizeof( DWORD ), 0, D3DERR_INVALIDCALL, NULL, "f", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ) * 4, sizeof( DWORD ) * 4, 0, D3DERR_INVALIDCALL, NULL, "f4", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ) * 16, sizeof( DWORD ) * 16, 0, D3DERR_INVALIDCALL, NULL, "f44", NULL }, + { D3DXPT_STRING, sizeof( "test" ), sizeof( "test" ), 0, D3DERR_INVALIDCALL, "test", "s_2", NULL }, + */ + + /* Boolean. Test 0x0 */ + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_BOOL44_A }, + { D3DXPT_BOOL, sizeof( BOOL ), 0, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), sizeof( BOOL ), S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + + /* Boolean vector. Test 0x4 */ + { D3DXPT_BOOL, sizeof( BOOL ) * 4, sizeof( BOOL ) * 4, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b4", &SET_RAWDATA_BOOL44_A }, + { D3DXPT_BOOL, sizeof( BOOL ) * 4, 0, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b4", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 4, sizeof( BOOL ) * 4, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b4", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 4, sizeof( BOOL ) * 4, 1, S_OK, &SET_RAWDATA_BOOL44_A, "b4", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 4, sizeof( BOOL ) * 4, sizeof( BOOL ), S_OK, &SET_RAWDATA_BOOL4_BA, "b4", &SET_RAWDATA_INT44_B }, + + /* Boolean matrix. Test 0x9 */ + { D3DXPT_BOOL, sizeof( BOOL ) * 16, sizeof( DWORD ) * 16, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b44", &SET_RAWDATA_BOOL44_A }, + { D3DXPT_BOOL, sizeof( BOOL ) * 16, 0, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b44", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 16, sizeof( BOOL ) * 16, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b44", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 16, sizeof( BOOL ) * 16, 1, S_OK, &SET_RAWDATA_BOOL44_A, "b44", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ) * 16, sizeof( BOOL ) * 16, sizeof( BOOL ), S_OK, &SET_RAWDATA_BOOL44_BA, "b44", &SET_RAWDATA_INT44_B }, + + /* Integer. Test 0xE */ + { D3DXPT_INT, sizeof( DWORD ), sizeof( DWORD ), 0, S_OK, &SET_RAWDATA_INT44_A, "i", &SET_RAWDATA_INT44_A }, + { D3DXPT_INT, sizeof( DWORD ), 0, 0, S_OK, &SET_RAWDATA_INT44_A, "i", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ), sizeof( DWORD ), sizeof( DWORD ), S_OK, &SET_RAWDATA_INT44_A, "i", &SET_RAWDATA_INT44_B }, + + /* Integer vector. Test 0x11 */ + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ) * 4, 0, S_OK, &SET_RAWDATA_INT44_A, "i4", &SET_RAWDATA_INT44_A }, + { D3DXPT_INT, sizeof( DWORD ) * 4, 0, 0, S_OK, &SET_RAWDATA_INT44_A, "i4", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ), 0, S_OK, &SET_RAWDATA_INT44_B, "i4", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ), 1, S_OK, &SET_RAWDATA_INT4_BA, "i4", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ), sizeof( DWORD ), S_OK, &SET_RAWDATA_INT4_BBA, "i4", &SET_RAWDATA_INT44_A }, + + /* Integer matrix. Test 0x16 */ + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ) * 16, 0, S_OK, &SET_RAWDATA_INT44_A, "i44", &SET_RAWDATA_INT44_A }, + { D3DXPT_INT, sizeof( DWORD ) * 16, 0, 0, S_OK, &SET_RAWDATA_INT44_A, "i44", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ), 0, S_OK, &SET_RAWDATA_INT44_B, "i44", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ), 1, S_OK, &SET_RAWDATA_INT44_BA, "i44", &SET_RAWDATA_INT44_B }, + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ), sizeof( DWORD ), S_OK, &SET_RAWDATA_INT44_BBA, "i44", &SET_RAWDATA_INT44_A }, + + /* Float. Test 0x1B */ + { D3DXPT_FLOAT, sizeof( FLOAT ), sizeof( FLOAT ), 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ), 0, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ), sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT44_A, "f", &SET_RAWDATA_FLOAT44_B }, + + /* Float vector. Test 0x1E */ + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ) * 4, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f4", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, 0, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f4", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), 0, S_OK, &SET_RAWDATA_FLOAT44_B, "f4", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), 1, S_OK, &SET_RAWDATA_FLOAT4_BA, "f4", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT4_BBA, "f4", &SET_RAWDATA_FLOAT44_A }, + + /* Float matrix. Test 0x23 */ + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ) * 16, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f44", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, 0, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f44", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), 0, S_OK, &SET_RAWDATA_FLOAT44_B, "f44", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), 1, S_OK, &SET_RAWDATA_FLOAT44_BA, "f44", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT44_BBA, "f44", &SET_RAWDATA_FLOAT44_A }, + + /* Strings. Test 0x28 */ + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, S_OK, "\0", "s", "test" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, D3DERR_INVALIDCALL, "\0", "s", NULL }, + { D3DXPT_STRING, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), 0, D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), sizeof( "test" ), D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + + /* String vector. Test 0x2E */ + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, S_OK, "\0", "s_2", "test" }, + { D3DXPT_STRING, sizeof( "\0" ), 0, 0, S_OK, "\0", "s_2", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), 0, S_OK, "\0", "s_2", "test_longer_string" }, + + /* Textures. Test 0x31 */ + { D3DXPT_TEXTURE, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "tex", "DEAFCAFE" }, + { D3DXPT_TEXTURE, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "tex", NULL }, + { D3DXPT_TEXTURE, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "tex", "DEADBEEF" }, + { D3DXPT_TEXTURE, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "tex", "DEAFCAFE" }, + + { D3DXPT_TEXTURE2D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "tex2d", "DEAFCAFE" }, + { D3DXPT_TEXTURE2D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "tex2d", NULL }, + { D3DXPT_TEXTURE2D, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "tex2d", "DEADBEEF" }, + { D3DXPT_TEXTURE2D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "tex2d", "DEAFCAFE" }, + + { D3DXPT_TEXTURE3D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "tex3d", "DEAFCAFE" }, + { D3DXPT_TEXTURE3D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "tex3d", NULL }, + { D3DXPT_TEXTURE3D, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "tex3d", "DEADBEEF" }, + { D3DXPT_TEXTURE3D, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "tex3d", "DEAFCAFE" }, + + /* Shaders. Test 0x3D */ + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "v", "DEAFCAFE" }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "v", NULL }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "v", "DEADBEEF" }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "v", "DEAFCAFE" }, + + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "v_2", "DEAFCAFE" }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "v_2", NULL }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "v_2", "DEADBEEF" }, + { D3DXPT_VERTEXSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "v_2", "DEAFCAFE" }, + + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "p", "DEAFCAFE" }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "p", NULL }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "p", "DEADBEEF" }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "p", "DEAFCAFE" }, + + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "p_2", "DEAFCAFE" }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, D3DERR_INVALIDCALL, "\0", "p_2", NULL }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "p_2", "DEADBEEF" }, + { D3DXPT_PIXELSHADER, sizeof( "\0" ), sizeof( "DEAFCAFE" ), sizeof( "DEAFCAFE" ), D3DERR_INVALIDCALL, "\0", "p_2", "DEAFCAFE" } +}; + +static void test_effect_setrawvalue_init_floats(void) +{ + memcpy( ((unsigned char*)&SET_RAWDATA_FLOAT4_BA) + 1, &SET_RAWDATA_FLOAT44_A, sizeof( FLOAT ) ); + memcpy( ((unsigned char*)&SET_RAWDATA_FLOAT4_BBA) + 1, &SET_RAWDATA_FLOAT44_A, sizeof( FLOAT ) ); + SET_RAWDATA_FLOAT4_BBA[1] = 1.0f; + memcpy( ((unsigned char*)&SET_RAWDATA_FLOAT44_BA) + 1, &SET_RAWDATA_FLOAT44_A, sizeof( FLOAT ) ); + SET_RAWDATA_FLOAT44_BA[4] = SET_RAWDATA_FLOAT44_BA[1]; + SET_RAWDATA_FLOAT44_BA[1] = 0.0f; + memcpy( ((unsigned char*)&SET_RAWDATA_FLOAT44_BBA) + 1, &SET_RAWDATA_FLOAT44_A, sizeof( FLOAT ) ); + SET_RAWDATA_FLOAT44_BBA[1] = 0.0f; +} + +static inline void dbg_print_bits(void * byte, unsigned int count, char *str) +{ + int x, y; + + for (y = 0; y < count; y++) + { + for (x = 0; x < CHAR_BIT; x++) + { + str[(y * CHAR_BIT) + x] = (*(unsigned char*)byte & (1 << x)) ? '1' : '0'; + } + } +} + +static void test_effect_setrawvalue(IDirect3DDevice9 *device) +{ + char expected_bit_buf[33] = { '\0' }; + char result_bit_buf[33] = { '\0' }; + unsigned int x, y, single_size; + ID3DXEffect *effect; + void *result_buf; + int memcmp_ret; + HRESULT hr; + + hr = D3DXCreateEffect( device, test_set_raw_value_blob, + sizeof(test_set_raw_value_blob), NULL, NULL, 0, NULL, &effect, NULL ); + ok( hr == D3D_OK, "Got result %#1.1x, expected 0 (D3D_OK).\n", hr ); + + /* All of these cause C0000005 on win10. */ + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + + test_effect_setrawvalue_init_floats(); + + /* Begin value loop. */ + for (x = 0; x < (sizeof( test_set_raw_data_values ) / sizeof( struct test_set_raw_data_value )); x++) + { + result_buf = malloc( test_set_raw_data_values[x].expected_data_len ); + if (result_buf) + { + memset( result_buf, '\0', test_set_raw_data_values[x].expected_data_len ); + hr = effect->lpVtbl->SetRawValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + test_set_raw_data_values[x].param_data, + test_set_raw_data_values[x].offset, + test_set_raw_data_values[x].param_data_len ); + ok( hr == test_set_raw_data_values[x].expected_result, + "Test # %#1.1x var name %s : got HRESULT %#1.1x, expected %#1.1x.\n", x, + test_set_raw_data_values[x].param_name, hr, + test_set_raw_data_values[x].expected_result ); + effect->lpVtbl->GetValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + result_buf, + test_set_raw_data_values[x].expected_data_len ); + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + single_size = (test_set_raw_data_values[x].param_type == D3DXPT_BOOL) ? sizeof( BOOL ) : + (test_set_raw_data_values[x].param_type == D3DXPT_INT) ? sizeof( DWORD ) : + sizeof( FLOAT ); + + if (test_set_raw_data_values[x].expected_data_len > single_size) + { + for (y = 0; y < test_set_raw_data_values[x].expected_data_len / single_size; y++) + { + memcmp_ret = memcmp( ((unsigned char*)result_buf) + (single_size * y), + ((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y), + single_size ); + if (memcmp_ret != 0) + { + dbg_print_bits( ((unsigned char*)result_buf) + (single_size * y), + single_size, result_bit_buf ); + dbg_print_bits( ((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y), single_size, expected_bit_buf ); + } + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_FLOAT: + ok( memcmp_ret == 0, + "Test #%#1.1x type %#1.1x var name %s index %#1.1x: got %#1.1f (bits: %s), expected %#1.1f (bits: %s).\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + y, + *(float*)(((unsigned char*)result_buf) + (single_size * y)), + result_bit_buf, + *(float*)(((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y)), + expected_bit_buf ); + break; + case D3DXPT_INT: + ok( memcmp_ret == 0, + "Test #%#1.1x type %#1.1x var name %s index %#1.1x: got %#1.1x (bits: %s), expected %#1.1x (bits: %s).\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + y, + *(unsigned int*)(((unsigned char*)result_buf) + (single_size * y)), + result_bit_buf, + *(unsigned int*)(((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y)), + expected_bit_buf ); + break; + case D3DXPT_BOOL: + ok( memcmp_ret == 0, + "Test #%#1.1x type %#1.1x var name %s index %#1.1x: got %#1.1x (bits: %s), expected %#1.1x (bits: %s).\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + y, + *(((unsigned char*)result_buf) + (single_size * y)), + result_bit_buf, + *(((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y)), + expected_bit_buf ); + break; + default: + /* Should never get here, but gcc complains without a default... */ + trace("Test #%#x not performed. Reached dead codepath.\n", x); + break; + }; + } + } + break; + case D3DXPT_STRING: + ok( memcmp( result_buf, + test_set_raw_data_values[x].expected_data, + test_set_raw_data_values[x].expected_data_len ) == 0, + "Test #%#1.1x type %#1.1x var name %s: got %s, expected %s.\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + (char *)result_buf, + (char *)test_set_raw_data_values[x].expected_data ); + break; + default: + ok( memcmp( result_buf, + test_set_raw_data_values[x].expected_data, + test_set_raw_data_values[x].expected_data_len ) == 0, + "Test #%#1.1x type %#1.1x var name %s: got %#1.1x, expected %#1.1x.\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + *(unsigned char*)result_buf, + *(unsigned char*)test_set_raw_data_values[x].expected_data ); + break; + }; + free( result_buf ); + } + else + { + skip("Could not allocate memory for test!"); + break; + } + } + effect->lpVtbl->Release(effect); +} + START_TEST(effect) { IDirect3DDevice9 *device; @@ -8731,6 +9329,7 @@ START_TEST(effect) test_effect_large_address_aware_flag(device); test_effect_get_pass_desc(device); test_effect_skip_constants(device); + test_effect_setrawvalue( device ); refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %u references left.\n", refcount); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/979
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125083 Your paranoid android. === w1064v1507 (testbot log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 6. === w1064v1507 (testbot log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 6. === debian11 (32 bit report) === ddraw: ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x1000. ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x3000. Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit ar:MA report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit de report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit fr report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit he:IL report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit hi:IN report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit ja:JP report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (32 bit zh:CN report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (build log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24700. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24700. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24700. === debian11 (32 bit WoW report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures === debian11 (64 bit WoW report) === Report validation errors: d3dx9_36:effect has no test summary line (early exit of the main process?) d3dx9_36:effect has unaccounted for todo messages d3dx9_36:effect has unaccounted for skip messages d3dx9_36:effect returned a non-zero exit code despite reporting no failures
On Mon Oct 10 00:52:11 2022 +0000, Patrick Hibbs wrote:
OK, fixed the commit subject. I've also added some tests. There will need to be another revision to this MR for wine to pass them correctly, but they currently pass without error on Win10. I do have a few questions: 1) Is there a preferred method for testing byte-misaligned floats in wine tests? Currently, I'm setting the unaffected parts of the array in the initializer, and then fixing it up at runtime with a memcpy(). Is there another method I should use? 2) As Win10 allows setting the byte-misaligned float, how should I deal with the assert in param_get_data_and_dirtify()? As the tests currently fail under wine due to it. 3) How did you get fx.exe to output DWORDs? I can manually clean it up but I'd prefer to have less to do. :smile: 4) Any other clean ups or tests to recommend? First of all, sorry for the delay.
These are some good tests, in principle. They already show that the implementation is not correct (e.g. it doesn't sanitize D3DXPT_BOOL values to 0/1) but I guess you already knew that. Actually, in regard to that point specifically: assuming that SetRawValue() does in fact do what it says, it suggests that BOOL values are stored unchanged and are instead sanitized on use (or e.g. by GetValue()). That's probably something that we want to fix in our implementation, separately from SetRawValue() itself. Marking the relevant tests as todo_wine initially and fixing them afterwards is an option. The expected results used by the tests are muddied quite a bit by the fact that the parameter value is not cleared to 0 (or any other value) between individual tests. That also makes it more complicated than necessary to add more tests. I'd fix that by explicitly clearing the tested parameter before each test. WRT your questions: 1. I think you can simply compute the expected results beforehand and put them into static const variables. You can certainly e.g. use your test_effect_setrawvalue_init_floats() function with some additional traces to figure out what those values are supposed to be, but I don't see any reason to recalculate the expected results at runtime in the final tests. It might make sense to have a comment in the test explaining what those "weird" expected results actually mean, hard to say at this point. 2. If I understand the test correctly, there is no evidence that SetRawValue() can "overflow" the current parameter. Either way, I think it's a matter of cutting the value size to the maximum of bytes and param->bytes or something like that. 3. For sure :smile:. I think we used the /Fx output parameter to get DWORD data. 4. Testing that SetRawValue() doesn't affect the "next" parameter when used with larger offsets / sizes is probably interesting. I also have a bunch of comments that I'll mention inline. There's most likely more, it's not an exhaustive list. One more thing. I saw you just pushed a new version of the patches, fixing a couple issues I had noticed. Nice, but generally please wait to push updates while I have the MR assigned to me, it means I'm actively reviewing it and changing stuff at that time can be somewhat disruptive for me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11081
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/effect.c:
static HRESULT WINAPI d3dx_effect_SetRawValue(ID3DXEffect *iface, D3DXHANDLE parameter, const void *data, UINT byte_offset, UINT bytes) { - FIXME("iface %p, parameter %p, data %p, byte_offset %u, bytes %u stub!\n", + struct d3dx_effect *src = impl_from_ID3DXEffect( iface ); I guess you took inspiration from the previous function, but I think here it would be better to call the effect variable "effect" instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11082
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/effect.c:
UINT byte_offset, UINT bytes) { - FIXME("iface %p, parameter %p, data %p, byte_offset %u, bytes %u stub!\n", + struct d3dx_effect *src = impl_from_ID3DXEffect( iface ); + struct d3dx_parameter *param = get_valid_parameter( src, parameter ); + void *raw = NULL; + + TRACE("iface %p, parameter %p, data %p, byte_offset %u, bytes %u.\n", iface, parameter, data, byte_offset, bytes);
- return E_NOTIMPL; + if (!src) + { + WARN("Invalid variable.\n"); + return E_FAIL; + }
No point in checking that the This pointer is non-NULL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11083
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/effect.c:
+ struct d3dx_parameter *param = get_valid_parameter( src, parameter ); + void *raw = NULL; + + TRACE("iface %p, parameter %p, data %p, byte_offset %u, bytes %u.\n", iface, parameter, data, byte_offset, bytes);
- return E_NOTIMPL; + if (!src) + { + WARN("Invalid variable.\n"); + return E_FAIL; + } + + if (!data || !param || !(raw = param_get_data_and_dirtify( src, param, + byte_offset + bytes, TRUE ))) + return D3DERR_INVALIDCALL; param_get_data_and_dirtify() can't really return NULL.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11084
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/effect.c:
+ + TRACE("iface %p, parameter %p, data %p, byte_offset %u, bytes %u.\n", iface, parameter, data, byte_offset, bytes);
- return E_NOTIMPL; + if (!src) + { + WARN("Invalid variable.\n"); + return E_FAIL; + } + + if (!data || !param || !(raw = param_get_data_and_dirtify( src, param, + byte_offset + bytes, TRUE ))) + return D3DERR_INVALIDCALL; + + memcpy( raw, data, bytes ); This should presumably copy data to an offset inside "raw", right?
Actually, this poses an issue with preserving the existing contents over the part that's skipped over. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11085
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ 97, 110, 107, 0, 0, 0 +}; + +struct test_set_raw_data_value +{ + D3DXPARAMETER_TYPE param_type; + unsigned int expected_data_len; + unsigned int param_data_len; + unsigned int offset; + HRESULT expected_result; + const void *expected_data; + const char *param_name; + const void *param_data; +}; + +static const int SET_RAWDATA_BOOL44_A[16] = { 1 }; All caps names are usually reserved to preprocessor directives and enum values. Please use lowercase names for these.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11086
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ 0, 0, 1, 0, 0, 0, + 6, 0, 0, 0, 98, 108, + 97, 110, 107, 0, 0, 0 +}; + +struct test_set_raw_data_value +{ + D3DXPARAMETER_TYPE param_type; + unsigned int expected_data_len; + unsigned int param_data_len; + unsigned int offset; + HRESULT expected_result; + const void *expected_data; + const char *param_name; + const void *param_data; +}; Can you reorder the fields such that the values to set are all at the start and the expected values all at the end? E.g.:
```suggestion:-10+0 struct test_set_raw_data_value { const char *param_name; D3DXPARAMETER_TYPE param_type; const void *data; unsigned int data_size; unsigned int offset; HRESULT expected_hr; const void *expected_data; unsigned int expected_data_size; }; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11087
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ { D3DXPT_INT, sizeof( DWORD ), sizeof( DWORD ), 0, D3DERR_INVALIDCALL, NULL, "i", NULL }, + { D3DXPT_INT, sizeof( DWORD ) * 4, sizeof( DWORD ) * 4, 0, D3DERR_INVALIDCALL, NULL, "i4", NULL }, + { D3DXPT_INT, sizeof( DWORD ) * 16, sizeof( DWORD ) * 16, 0, D3DERR_INVALIDCALL, NULL, "i44", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ), sizeof( DWORD ), 0, D3DERR_INVALIDCALL, NULL, "f", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ) * 4, sizeof( DWORD ) * 4, 0, D3DERR_INVALIDCALL, NULL, "f4", NULL }, + { D3DXPT_FLOAT, sizeof( DWORD ) * 16, sizeof( DWORD ) * 16, 0, D3DERR_INVALIDCALL, NULL, "f44", NULL }, + { D3DXPT_STRING, sizeof( "test" ), sizeof( "test" ), 0, D3DERR_INVALIDCALL, "test", "s_2", NULL }, + */ + + /* Boolean. Test 0x0 */ + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_BOOL44_A }, + { D3DXPT_BOOL, sizeof( BOOL ), 0, 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), sizeof( BOOL ), S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + { D3DXPT_BOOL, sizeof( BOOL ), sizeof( BOOL ), 0, S_OK, &SET_RAWDATA_BOOL44_A, "b", &SET_RAWDATA_INT44_B }, + + /* Boolean vector. Test 0x4 */ Good idea to keep track of the test index. Decimal numbers would probably be easier to read though.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11088
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ /* Float vector. Test 0x1E */ + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ) * 4, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f4", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, 0, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f4", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), 0, S_OK, &SET_RAWDATA_FLOAT44_B, "f4", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), 1, S_OK, &SET_RAWDATA_FLOAT4_BA, "f4", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 4, sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT4_BBA, "f4", &SET_RAWDATA_FLOAT44_A }, + + /* Float matrix. Test 0x23 */ + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ) * 16, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f44", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, 0, 0, S_OK, &SET_RAWDATA_FLOAT44_A, "f44", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), 0, S_OK, &SET_RAWDATA_FLOAT44_B, "f44", &SET_RAWDATA_FLOAT44_B }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), 1, S_OK, &SET_RAWDATA_FLOAT44_BA, "f44", &SET_RAWDATA_FLOAT44_A }, + { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT44_BBA, "f44", &SET_RAWDATA_FLOAT44_A }, + + /* Strings. Test 0x28 */ + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, S_OK, "\0", "s", "test" }, "\0" looks pretty weird. Use "" if possible, or add a comment explaining why you had to resort to that if not.
These string test results are also quite surprising. I'd double check that everything is correct (see below for something to look for). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11089
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ { D3DXPT_FLOAT, sizeof( FLOAT ) * 16, sizeof( FLOAT ), sizeof( FLOAT ), S_OK, &SET_RAWDATA_FLOAT44_BBA, "f44", &SET_RAWDATA_FLOAT44_A }, + + /* Strings. Test 0x28 */ + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, S_OK, "\0", "s", "test" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, D3DERR_INVALIDCALL, "\0", "s", NULL }, + { D3DXPT_STRING, sizeof( "\0" ), 0, 0, D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), 0, D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), sizeof( "test" ), D3DERR_INVALIDCALL, "\0", "s", "test_longer_string" }, + + /* String vector. Test 0x2E */ + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test" ), 0, S_OK, "\0", "s_2", "test" }, + { D3DXPT_STRING, sizeof( "\0" ), 0, 0, S_OK, "\0", "s_2", "test_longer_string" }, + { D3DXPT_STRING, sizeof( "\0" ), sizeof( "test_longer_string" ), 0, S_OK, "\0", "s_2", "test_longer_string" }, + + /* Textures. Test 0x31 */ + { D3DXPT_TEXTURE, sizeof( "\0" ), sizeof( "DEAFCAFE" ), 0, S_OK, "\0", "tex", "DEAFCAFE" }, Assigning a string to a D3DXPT_TEXTURE parameter is not great. I guess it's interesting in its own way that it apparently doesn't crash on Windows, but I'd avoid this kind of weirdness in the test by creating and passing a proper texture object (or NULL).
Same comment for the shader objects below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11090
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ memcpy( ((unsigned char*)&SET_RAWDATA_FLOAT44_BBA) + 1, &SET_RAWDATA_FLOAT44_A, sizeof( FLOAT ) ); + SET_RAWDATA_FLOAT44_BBA[1] = 0.0f; +} + +static inline void dbg_print_bits(void * byte, unsigned int count, char *str) +{ + int x, y; + + for (y = 0; y < count; y++) + { + for (x = 0; x < CHAR_BIT; x++) + { + str[(y * CHAR_BIT) + x] = (*(unsigned char*)byte & (1 << x)) ? '1' : '0'; + } + } +} You forgot to increment the "byte" pointer, you're only printing the first byte.
BTW, I'd just get rid of this function and related machinery entirely. The hex prints do the job just fine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11091
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ } +} + +static void test_effect_setrawvalue(IDirect3DDevice9 *device) +{ + char expected_bit_buf[33] = { '\0' }; + char result_bit_buf[33] = { '\0' }; + unsigned int x, y, single_size; + ID3DXEffect *effect; + void *result_buf; + int memcmp_ret; + HRESULT hr; + + hr = D3DXCreateEffect( device, test_set_raw_value_blob, + sizeof(test_set_raw_value_blob), NULL, NULL, 0, NULL, &effect, NULL ); + ok( hr == D3D_OK, "Got result %#1.1x, expected 0 (D3D_OK).\n", hr );
ok(hr == D3D_OK, "Unexpected hr %#x.\n", hr);
As a general comment, in d3d code we don't put a whitespace after the opening parenthesis (or a whitespace before the closing parenthesis) of function calls or operators. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11092
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ int memcmp_ret; + HRESULT hr; + + hr = D3DXCreateEffect( device, test_set_raw_value_blob, + sizeof(test_set_raw_value_blob), NULL, NULL, 0, NULL, &effect, NULL ); + ok( hr == D3D_OK, "Got result %#1.1x, expected 0 (D3D_OK).\n", hr ); + + /* All of these cause C0000005 on win10. */ + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); No C++-style comments please. We usually leave those uncommented but inside an if (0) {}, so that it's still compiled.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11093
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ /* All of these cause C0000005 on win10. */ + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 0 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + + test_effect_setrawvalue_init_floats(); + + /* Begin value loop. */ + for (x = 0; x < (sizeof( test_set_raw_data_values ) / sizeof( struct test_set_raw_data_value )); x++) + { + result_buf = malloc( test_set_raw_data_values[x].expected_data_len ); No reason to malloc the buffer, just define a static array. We know how much memory we need and it's a very small amount.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11094
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ /* Begin value loop. */ + for (x = 0; x < (sizeof( test_set_raw_data_values ) / sizeof( struct test_set_raw_data_value )); x++) + { + result_buf = malloc( test_set_raw_data_values[x].expected_data_len ); + if (result_buf) + { + memset( result_buf, '\0', test_set_raw_data_values[x].expected_data_len ); + hr = effect->lpVtbl->SetRawValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + test_set_raw_data_values[x].param_data, + test_set_raw_data_values[x].offset, + test_set_raw_data_values[x].param_data_len ); + ok( hr == test_set_raw_data_values[x].expected_result, + "Test # %#1.1x var name %s : got HRESULT %#1.1x, expected %#1.1x.\n", x, + test_set_raw_data_values[x].param_name, hr, + test_set_raw_data_values[x].expected_result ); It's nice to make use of test contexts. At the start of the loop you'd have something like: `winetest_push_context("Test %u, parameter %s", i + 1, tests[i].param_name);` and at the end: `winetest_pop_context();`
Then the ok() call becomes something like ```suggestion:-0+0 ok(hr == tests[i].expected_hr, "Unexpected hr %#x, expected %#x.\n", hr, tests[i].expected_hr); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11095
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 0, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + //hr = effect->lpVtbl->SetRawValue( effect, INVALID_HANDLE_VALUE, NULL, 1, 1 ); + //ok( hr == D3DERR_INVALIDCALL, "Got result %#x, expected 0 (D3D_INVALIDCALL).\n", hr ); + + test_effect_setrawvalue_init_floats(); + + /* Begin value loop. */ + for (x = 0; x < (sizeof( test_set_raw_data_values ) / sizeof( struct test_set_raw_data_value )); x++) + { + result_buf = malloc( test_set_raw_data_values[x].expected_data_len ); + if (result_buf) + { + memset( result_buf, '\0', test_set_raw_data_values[x].expected_data_len ); + hr = effect->lpVtbl->SetRawValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), Let's put that on its own line and make sure to check the result with an ok() call. Checking that it's non-NULL is enough.
Also that way you can avoid an extra GetParameterByName() call immediately afterwards. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11096
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ if (result_buf) + { + memset( result_buf, '\0', test_set_raw_data_values[x].expected_data_len ); + hr = effect->lpVtbl->SetRawValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + test_set_raw_data_values[x].param_data, + test_set_raw_data_values[x].offset, + test_set_raw_data_values[x].param_data_len ); + ok( hr == test_set_raw_data_values[x].expected_result, + "Test # %#1.1x var name %s : got HRESULT %#1.1x, expected %#1.1x.\n", x, + test_set_raw_data_values[x].param_name, hr, + test_set_raw_data_values[x].expected_result ); + effect->lpVtbl->GetValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + result_buf, + test_set_raw_data_values[x].expected_data_len ); Please check the return value with ok(). I suspect this might play a role in your weird string test results.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11097
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ * an off-by-one byte alignment copied to them. + * See test_effect_setrawvalue_init_floats() below. + * + * E.x. arr[0] = (1.0f & 0x00FFFFFF) | 1.2f; + */ +static float SET_RAWDATA_FLOAT4_BA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, + 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f }; +static float SET_RAWDATA_FLOAT4_BBA[16] = { 1.2f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, + 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f }; +static float SET_RAWDATA_FLOAT44_BA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, + 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; +static float SET_RAWDATA_FLOAT44_BBA[16] = { 1.2f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, + 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; + +struct test_set_raw_data_value test_set_raw_data_values[] = +{ You can probably move this inside the test function and rename it to just "tests".
It can also be const, I think. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11098
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ ok( hr == test_set_raw_data_values[x].expected_result, + "Test # %#1.1x var name %s : got HRESULT %#1.1x, expected %#1.1x.\n", x, + test_set_raw_data_values[x].param_name, hr, + test_set_raw_data_values[x].expected_result ); + effect->lpVtbl->GetValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + result_buf, + test_set_raw_data_values[x].expected_data_len ); + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + single_size = (test_set_raw_data_values[x].param_type == D3DXPT_BOOL) ? sizeof( BOOL ) : + (test_set_raw_data_values[x].param_type == D3DXPT_INT) ? sizeof( DWORD ) : + sizeof( FLOAT ); element_size? scalar_size?
Practically speaking, those are all 4 bytes. I don't see any issue in hardcoding a sizeof(DWORD) where needed here in the tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11099
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ test_set_raw_data_values[x].param_name, hr, + test_set_raw_data_values[x].expected_result ); + effect->lpVtbl->GetValue( effect, + effect->lpVtbl->GetParameterByName( effect, NULL, test_set_raw_data_values[x].param_name ), + result_buf, + test_set_raw_data_values[x].expected_data_len ); + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + single_size = (test_set_raw_data_values[x].param_type == D3DXPT_BOOL) ? sizeof( BOOL ) : + (test_set_raw_data_values[x].param_type == D3DXPT_INT) ? sizeof( DWORD ) : + sizeof( FLOAT ); + + if (test_set_raw_data_values[x].expected_data_len > single_size) What's the point of this check?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11100
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ result_buf, + test_set_raw_data_values[x].expected_data_len ); + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_BOOL: + case D3DXPT_INT: + case D3DXPT_FLOAT: + single_size = (test_set_raw_data_values[x].param_type == D3DXPT_BOOL) ? sizeof( BOOL ) : + (test_set_raw_data_values[x].param_type == D3DXPT_INT) ? sizeof( DWORD ) : + sizeof( FLOAT ); + + if (test_set_raw_data_values[x].expected_data_len > single_size) + { + for (y = 0; y < test_set_raw_data_values[x].expected_data_len / single_size; y++) + { + memcmp_ret = memcmp( ((unsigned char*)result_buf) + (single_size * y), Given that you're going to reuse that pointer to the current element in the type-specific ok() messages, I'd store it in a variable for clarity.
v = (unsigned char *)buffer + j * sizeof(DWORD);
e = (unsigned char *)tests[i].expected_data + j * sizeof(DWORD);
r = memcmp(v,
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11101
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ memcmp_ret = memcmp( ((unsigned char*)result_buf) + (single_size * y), + ((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y), + single_size ); + if (memcmp_ret != 0) + { + dbg_print_bits( ((unsigned char*)result_buf) + (single_size * y), + single_size, result_bit_buf ); + dbg_print_bits( ((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y), single_size, expected_bit_buf ); + } + switch (test_set_raw_data_values[x].param_type) + { + case D3DXPT_FLOAT: + ok( memcmp_ret == 0, + "Test #%#1.1x type %#1.1x var name %s index %#1.1x: got %#1.1f (bits: %s), expected %#1.1f (bits: %s).\n", Please print floats using the "%.8e" specifier, like elsewhere in this file.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11102
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/effect.c:
+ ok( memcmp_ret == 0, + "Test #%#1.1x type %#1.1x var name %s index %#1.1x: got %#1.1x (bits: %s), expected %#1.1x (bits: %s).\n", + x, + test_set_raw_data_values[x].param_type, + test_set_raw_data_values[x].param_name, + y, + *(((unsigned char*)result_buf) + (single_size * y)), + result_bit_buf, + *(((unsigned char*)test_set_raw_data_values[x].expected_data) + + (single_size * y)), + expected_bit_buf ); + break; + default: + /* Should never get here, but gcc complains without a default... */ + trace("Test #%#x not performed. Reached dead codepath.\n", x); + break; You can probably abort or assert here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_11103
participants (4)
-
Marvin -
Matteo Bruni (＠Mystral) -
Patrick Hibbs
-
Patrick Hibbs (＠hibbsncc1701)