2016-03-21 14:55 GMT+01:00 Paul Gofman gofmanp@gmail.com:
+HRESULT d3dx_set_shader_const_fxlc(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass,
struct d3dx_parameter *param, unsigned int vs)
Using a BOOL for the vs flag seems nicer.
diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c index fec9de4..1770046 100644 --- a/dlls/d3dx9_36/preshader.c +++ b/dlls/d3dx9_36/preshader.c @@ -242,7 +242,7 @@ static void regstore_set_values(struct d3dx_regstore *rs, unsigned int table, vo static unsigned int regstore_is_val_set_reg(struct d3dx_regstore *rs, unsigned int table, unsigned int reg_idx) { return rs->table_value_set[table][reg_idx / PRES_BITMASK_BLOCK_SIZE] &
(1ul << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
(1u << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
When you end up changing stuff introduced in a previous patch in the series you know there is something wrong ;) I guess it's a rebase glitch (by any chance, was this a remnant of a try with the uint_ptr idea you mentioned earlier?)
+HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device, struct d3dx_param_eval *peval) +{
- HRESULT hr, res;
- struct d3dx_preshader *pres = &peval->pres;
- struct d3dx_regstore *rs = &pres->regs;
- TRACE("device %p, peval %p, param_type %u.\n", device, peval, peval->param_type);
- if (FAILED(hr = set_constants(rs, &pres->inputs)))
return hr;
- if (FAILED(hr = execute_preshader(pres)))
return hr;
- if (FAILED(hr = set_constants(rs, &peval->shader_inputs)))
return hr;
- #define SET_TABLE(table, func_suff, type) \
{ \
unsigned int is, n; \
\
is = 0; \
while (is < rs->table_sizes[table]) \
{\
n = 0; \
while (is < rs->table_sizes[table] && !regstore_is_val_set_reg(rs, table, is)) \
is++; \
while (is + n < rs->table_sizes[table] && regstore_is_val_set_reg(rs, table, is + n)) \
n++; \
if (!n) \
continue; \
TRACE("setting %u consts at %u.\n", n, is); \
hr = IDirect3DDevice9_##func_suff(device, is, \
(const type *)rs->tables[table] + is * table_info[table].reg_component_count, n); \
if (FAILED(hr)) \
{ \
ERR(#func_suff " failed, hr %#x.\n", hr); \
res = hr; \
} \
is += n; \
} \
regstore_reset_table(rs, table); \
}
- res = D3D_OK;
- if (peval->param_type == D3DXPT_VERTEXSHADER)
- {
SET_TABLE(PRES_REGTAB_OCONST, SetVertexShaderConstantF, float);
SET_TABLE(PRES_REGTAB_OICONST, SetVertexShaderConstantI, int);
SET_TABLE(PRES_REGTAB_OBCONST, SetVertexShaderConstantB, BOOL);
- }
- else if (peval->param_type == D3DXPT_PIXELSHADER)
- {
SET_TABLE(PRES_REGTAB_OCONST, SetPixelShaderConstantF, float);
SET_TABLE(PRES_REGTAB_OICONST, SetPixelShaderConstantI, int);
SET_TABLE(PRES_REGTAB_OBCONST, SetPixelShaderConstantB, BOOL);
- }
- else
- {
FIXME("Unexpected parameter type %u.\n", peval->param_type);
return D3DERR_INVALIDCALL;
- }
- #undef SET_TABLE
This is certainly hard to handle nicely because of the multiple similar d3d9 functions with slightly different signatures but I think it's better to avoid macros (especially such large ones) as much as possible.
There are multiple ways to achieve that. Perhaps the easiest one is to turn SET_TABLE into a function taking an array of enum PRES_REG_TABLES (which, btw, should be lowercase) and the shader type as additional parameters, loop over the tables and call the correct d3d9 function by using two levels of switches (or ifs). The type isn't really necessary, you can calculate the offset with table_info[].component_size.
On 03/23/2016 01:09 AM, Matteo Bruni wrote:
diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c index fec9de4..1770046 100644 --- a/dlls/d3dx9_36/preshader.c +++ b/dlls/d3dx9_36/preshader.c @@ -242,7 +242,7 @@ static void regstore_set_values(struct d3dx_regstore *rs, unsigned int table, vo static unsigned int regstore_is_val_set_reg(struct d3dx_regstore *rs, unsigned int table, unsigned int reg_idx) { return rs->table_value_set[table][reg_idx / PRES_BITMASK_BLOCK_SIZE] &
(1ul << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
(1u << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
When you end up changing stuff introduced in a previous patch in the series you know there is something wrong ;) I guess it's a rebase glitch (by any chance, was this a remnant of a try with the uint_ptr idea you mentioned earlier?)
Yes, I put uint_ptr in, then realized that it would require an extra include to the common header file, or using ULONG_PTR instead (for which I was not sure if it is decent enough :), so I gave up as the difference introduced by this looked not important really. So I got back in patch stack and changed 1ul's, which were introduced in two different patches, and took a wrong (later) patch for the second change. So I saw 1u in the final file and I did not notice this mess in between, sorry.
2016-03-22 23:48 GMT+01:00 Paul Gofman gofmanp@gmail.com:
On 03/23/2016 01:09 AM, Matteo Bruni wrote:
diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c index fec9de4..1770046 100644 --- a/dlls/d3dx9_36/preshader.c +++ b/dlls/d3dx9_36/preshader.c @@ -242,7 +242,7 @@ static void regstore_set_values(struct d3dx_regstore *rs, unsigned int table, vo static unsigned int regstore_is_val_set_reg(struct d3dx_regstore *rs, unsigned int table, unsigned int reg_idx) { return rs->table_value_set[table][reg_idx / PRES_BITMASK_BLOCK_SIZE] &
(1ul << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
(1u << (reg_idx % PRES_BITMASK_BLOCK_SIZE));
When you end up changing stuff introduced in a previous patch in the series you know there is something wrong ;) I guess it's a rebase glitch (by any chance, was this a remnant of a try with the uint_ptr idea you mentioned earlier?)
Yes, I put uint_ptr in, then realized that it would require an extra include to the common header file, or using ULONG_PTR instead (for which I was not sure if it is decent enough :), so I gave up as the difference introduced by this looked not important really. So I got back in patch stack and changed 1ul's, which were introduced in two different patches, and took a wrong (later) patch for the second change. So I saw 1u in the final file and I did not notice this mess in between, sorry.
No problem at all :)