2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MINFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MAGFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MINFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MAGFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
2017-07-05 19:40 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MINFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MAGFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
Ah, cool. Yes, that sounds good.
On 07/05/2017 08:40 PM, Paul Gofman wrote:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1,
D3DSAMP_MINFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1,
D3DSAMP_MAGFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
More exactly, the state parameter in sampler state is float for FXLC constants for some reason (which is not the case for "normal" states, which are assigned ok). Maybe it should be set to correct type during the parse actually.
2017-07-05 19:45 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:40 PM, Paul Gofman wrote:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MINFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MAGFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
More exactly, the state parameter in sampler state is float for FXLC constants for some reason (which is not the case for "normal" states, which are assigned ok). Maybe it should be set to correct type during the parse actually.
Maybe FXLC preshaders are always supposed to output float, with the optional conversion after the fact. Not sure it makes any difference in practice.
On 07/05/2017 08:55 PM, Matteo Bruni wrote:
2017-07-05 19:45 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:40 PM, Paul Gofman wrote:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MINFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MAGFILTER,
&value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
More exactly, the state parameter in sampler state is float for FXLC constants for some reason (which is not the case for "normal" states, which are assigned ok). Maybe it should be set to correct type during the parse actually.
Maybe FXLC preshaders are always supposed to output float, with the optional conversion after the fact. Not sure it makes any difference in practice.
Yes, sure, but the output of FXLC preshader is actually converted to the type which is specified in state parameter which holds FXLC. That works fine for "top level" states, but for some reason the type of FXLC state parameter for sampler is float in the test case.
2017-07-05 20:00 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:55 PM, Matteo Bruni wrote:
2017-07-05 19:45 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:40 PM, Paul Gofman wrote:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
#define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ @@ -4505,12 +4551,38 @@ static void test_effect_preshader(IDirect3DDevice9 *device) hr = IDirect3DDevice9_GetSamplerState(device, 0, D3DSAMP_MAGFILTER, &value); ok(hr == D3D_OK, "Got result %#x.\n", hr); todo_wine ok(value == 3, "Unexpected sampler 0 magfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1,
D3DSAMP_MINFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
- hr = IDirect3DDevice9_GetSamplerState(device, 1,
D3DSAMP_MAGFILTER, &value);
- ok(hr == D3D_OK, "Got result %#x.\n", hr);
- todo_wine
- ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
More exactly, the state parameter in sampler state is float for FXLC constants for some reason (which is not the case for "normal" states, which are assigned ok). Maybe it should be set to correct type during the parse actually.
Maybe FXLC preshaders are always supposed to output float, with the optional conversion after the fact. Not sure it makes any difference in practice.
Yes, sure, but the output of FXLC preshader is actually converted to the type which is specified in state parameter which holds FXLC. That works fine for "top level" states, but for some reason the type of FXLC state parameter for sampler is float in the test case.
Doesn't it come from the effect itself though? Converting the state parameter value to the actual state type would match a tiny bit better with what the effect specifies, for what it might be worth. It probably won't make any practical difference either way you go, the additional type field in state_table[] should mostly be it in any case.
On 07/05/2017 09:12 PM, Matteo Bruni wrote:
2017-07-05 20:00 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:55 PM, Matteo Bruni wrote:
2017-07-05 19:45 GMT+02:00 Paul Gofman gofmanp@gmail.com:
On 07/05/2017 08:40 PM, Paul Gofman wrote:
On 07/05/2017 08:29 PM, Matteo Bruni wrote:
2017-06-28 13:28 GMT+02:00 Paul Gofman gofmanp@gmail.com:
Sorry for the delayed reviews. I did it at last...
> #define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \ > @@ -4505,12 +4551,38 @@ static void > test_effect_preshader(IDirect3DDevice9 *device) > hr = IDirect3DDevice9_GetSamplerState(device, 0, > D3DSAMP_MAGFILTER, &value); > ok(hr == D3D_OK, "Got result %#x.\n", hr); > todo_wine ok(value == 3, "Unexpected sampler 0 magfilter > %u.\n", > value); > + > + hr = IDirect3DDevice9_GetSamplerState(device, 1, > D3DSAMP_MINFILTER, > &value); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value); > + hr = IDirect3DDevice9_GetSamplerState(device, 1, > D3DSAMP_MAGFILTER, > &value); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + todo_wine > + ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value); Not new or really related to this patch: any idea about what's going on with the magfilter state computation? I didn't really look into it, does it have to do with default parameter values by any chance?
It needs type casting, the output table for FXLC is always float, but the input for state is integer, thus an enormous value failing the test. I was thinking of adding type specifier to state table in effect.c and doing the cast, but did not come to that yet.
More exactly, the state parameter in sampler state is float for FXLC constants for some reason (which is not the case for "normal" states, which are assigned ok). Maybe it should be set to correct type during the parse actually.
Maybe FXLC preshaders are always supposed to output float, with the optional conversion after the fact. Not sure it makes any difference in practice.
Yes, sure, but the output of FXLC preshader is actually converted to the type which is specified in state parameter which holds FXLC. That works fine for "top level" states, but for some reason the type of FXLC state parameter for sampler is float in the test case.
Doesn't it come from the effect itself though? Converting the state parameter value to the actual state type would match a tiny bit better with what the effect specifies, for what it might be worth. It probably won't make any practical difference either way you go, the additional type field in state_table[] should mostly be it in any case.
It comes from effect (parameter typedef), but it comes wrong for sampler states and FXLC parameter. I would suggest to "fix" that parameter type during parse state (based on the type added to state_table[]) with the WARN output. Doing it on the fly during state application would add unwanted extra checks and array access at runtime.
-
Matteo Bruni -
Paul Gofman