[PATCH 5/5] d3d10/effect: Add initial support for indexing expressions.
Matteo Bruni
matteo.mystral at gmail.com
Fri Oct 29 14:29:37 CDT 2021
On Thu, Oct 28, 2021 at 9:46 AM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
> ---
> dlls/d3d10/effect.c | 560 +++++++++++++++++++++++++++++++++++++-
> dlls/d3d10/tests/effect.c | 114 ++++++++
> 2 files changed, 672 insertions(+), 2 deletions(-)
My general question / concern on this patch is whether there is room
for code sharing of some kind with d3dx9. Aside from the practical
issue that there is no proper place to put the shared code at the
moment (there is still hope I'll send patches for that eventually), my
feeling is that we could reuse some helpers at least, with some
moderate work.
I'm okay with duplicating stuff like this for the time being, as long
as we keep the door open to merging code back afterwards.
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 003060598a3..5e93fdddde3 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -30,6 +30,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(d3d10);
> ((DWORD)(ch2) << 16) | ((DWORD)(ch3) << 24 ))
> #define TAG_DXBC MAKE_TAG('D', 'X', 'B', 'C')
> #define TAG_FX10 MAKE_TAG('F', 'X', '1', '0')
> +#define TAG_FXLC MAKE_TAG('F', 'X', 'L', 'C')
> +#define TAG_CLI4 MAKE_TAG('C', 'L', 'I', '4')
> +#define TAG_CTAB MAKE_TAG('C', 'T', 'A', 'B')
>
> #define D3D10_FX10_TYPE_COLUMN_SHIFT 11
> #define D3D10_FX10_TYPE_COLUMN_MASK (0x7 << D3D10_FX10_TYPE_COLUMN_SHIFT)
> @@ -168,6 +171,92 @@ enum d3d10_effect_container_type
> D3D10_C_SAMPLER,
> };
>
> +struct preshader_instr
> +{
> + unsigned int comp_count : 16;
> + unsigned int reserved : 4;
> + unsigned int opcode : 11;
> + unsigned int scalar : 1;
> +};
> +
> +typedef float (*pres_op_func)(float **args, unsigned int n);
> +
> +struct preshader_op_info
> +{
> + unsigned short idx;
> + unsigned short opcode;
> + char name[16];
> + pres_op_func func;
> +};
> +
> +static float pres_ftou(float **args, unsigned int n)
> +{
> + unsigned int u = *args[0];
> + return *(float *)&u;
> +}
> +
> +static float pres_add(float **args, unsigned int n)
> +{
> + return *args[0] + *args[1];
> +}
> +
> +enum preshader_op
> +{
> + D3D10_PRESHADER_OP_FTOU = 0,
> + D3D10_PRESHADER_OP_ADD,
> + D3D10_PRESHADER_OP_MAX,
> +};
It looks like you meant to use D3D10_PRESHADER_OP_MAX purely as a
sentinel but that's confusing at best (in fact there is a MAX op in
d3dx9 at least).
I also have a nitpick: moving the definition of preshader_op_info
right here, just above the definition of preshader_ops[], seems nicer.
> +static const struct preshader_op_info preshader_ops[] =
> +{
> + { D3D10_PRESHADER_OP_FTOU, 0x133, "ftou", pres_ftou },
> + { D3D10_PRESHADER_OP_ADD, 0x204, "add", pres_add },
> +};
> +
> +struct d3d10_ctab_var
> +{
> + struct d3d10_effect_variable *v;
> + unsigned int offset;
> + unsigned int length;
> +};
> +
> +struct d3d10_reg_table
> +{
> + union
> + {
> + float *f;
> + DWORD *dword;
> + char *byte;
> + } u;
> + unsigned int count;
> +};
> +
> +enum d3d10_reg_table_type
> +{
> + D3D10_REG_TABLE_CONSTANTS = 1,
> + D3D10_REG_TABLE_CB = 2,
> + D3D10_REG_TABLE_RESULT = 4,
> + D3D10_REG_TABLE_TEMP = 7,
> +};
> +
> +struct d3d10_effect_preshader
> +{
> + struct d3d10_reg_table constants;
> + struct d3d10_reg_table cb;
> + struct d3d10_reg_table result;
> + struct d3d10_reg_table temp;
> + struct d3d10_reg_table code;
> +
> + struct d3d10_ctab_var *vars;
> + unsigned int vars_count;
> +};
It might make sense to organize the various struct d3d10_reg_table
entries into a single array indexed by an enum d3d10_reg_table_type
value. It should allow some simplification down the line, like for
example...
> +static void d3d10_effect_preshader_clear(struct d3d10_effect_preshader *p)
> +{
> + d3d10_reg_table_clear(&p->constants);
> + d3d10_reg_table_clear(&p->cb);
> + d3d10_reg_table_clear(&p->result);
> + d3d10_reg_table_clear(&p->temp);
> + d3d10_reg_table_clear(&p->code);
> + heap_free(p->vars);
> + p->vars = NULL;
> + p->vars_count = 0;
> +}
... here, or...
> +
> +static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_preshader *p,
> + enum d3d10_reg_table_type regt, unsigned int offset)
> +{
> + switch (regt)
> + {
> + case D3D10_REG_TABLE_CONSTANTS:
> + return p->constants.u.f + offset;
> + case D3D10_REG_TABLE_CB:
> + return p->cb.u.f + offset;
> + case D3D10_REG_TABLE_RESULT:
> + return p->result.u.f + offset;
> + case D3D10_REG_TABLE_TEMP:
> + return p->temp.u.f + offset;
> + default:
> + return NULL;
> + }
> +}
... here, this one should become trivial. You want to NULL-check the
table before accessing it, either explicitly or with an assert.
> @@ -543,6 +748,39 @@ static void d3d10_effect_update_dependent_props(struct d3d10_effect_prop_depende
>
> break;
>
> + case D3D10_EOO_INDEX_EXPRESSION:
> +
> + v = d->u.index_expr.v;
> +
> + /* Evaluate index. */
> + if (FAILED(hr = d3d10_effect_preshader_eval(&d->u.index_expr.index)))
> + {
> + WARN("Failed to evaluate index expression, hr %#x.\n", hr);
> + return;
> + }
That kind of comment doesn't seem very helpful.
> +
> + variable_idx = *d->u.index_expr.index.result.u.dword;
> +
> + if (variable_idx >= v->type->element_count)
> + {
> + WARN("Expression evaluated to invalid index value %u, array %s of size %u.\n",
> + variable_idx, debugstr_a(v->name), v->type->element_count);
> + return;
> + }
> +
> + switch (property_info->type)
> + {
> + case D3D10_SVT_VERTEXSHADER:
> + case D3D10_SVT_PIXELSHADER:
> + case D3D10_SVT_GEOMETRYSHADER:
> + ((void **)dst)[d->idx] = v;
> + *dst_index = variable_idx;
> + break;
> + default:
> + ((void **)dst)[d->idx] = &v->elements[variable_idx];
> + }
> + break;
> +
> default:
> FIXME("Unsupported property update for %u.\n", d->operation);
> }
> +static HRESULT parse_fx10_ctab(void *ctx, const char *data, unsigned int data_size)
> +{
> + struct d3d10_preshader_parse_context *context = ctx;
> + struct d3d10_effect_preshader *p = context->preshader;
> + struct ctab_header
> + {
> + DWORD size;
> + DWORD creator;
> + DWORD version;
> + DWORD constants;
> + DWORD constantinfo;
> + DWORD flags;
> + DWORD target;
> + } header;
> + struct ctab_const_info
> + {
> + DWORD name;
> + WORD register_set;
> + WORD register_index;
> + WORD register_count;
> + WORD reserved;
> + DWORD typeinfo;
> + DWORD default_value;
> + } *info;
> + unsigned int i, info_offset, unused, cb_reg_count = 0;
> + const char *ptr = data;
> + const char *name;
> + size_t name_len;
> + HRESULT hr;
> +
> + if (data_size < sizeof(header))
> + {
> + WARN("Invalid constant table size %u.\n", data_size);
> + return E_FAIL;
> + }
> +
> + read_dword(&ptr, &unused);
> + read_dword(&ptr, &unused);
> + read_dword(&ptr, &unused);
> + read_dword(&ptr, &p->vars_count);
> + read_dword(&ptr, &info_offset);
> + read_dword(&ptr, &unused);
> + read_dword(&ptr, &unused);
Any clue about all those unused DWORDs?
> +
> + if (!require_space(info_offset, p->vars_count, sizeof(*info) / sizeof(DWORD),
> + data_size))
> + {
> + WARN("Invalid constant info section offset %#x.\n", info_offset);
> + return E_FAIL;
> + }
> +
> + TRACE("Variable count %u.\n", p->vars_count);
> +
> + if (!(p->vars = heap_calloc(p->vars_count, sizeof(*p->vars))))
> + return E_OUTOFMEMORY;
> +
> + /* Collect variables used in expression. */
> + info = (struct ctab_const_info *)(data + info_offset);
> + for (i = 0; i < p->vars_count; ++i, ++info)
> + {
> + if (!fx10_get_string(data, data_size, info->name, &name, &name_len))
> + return E_FAIL;
> +
> + if (!(p->vars[i].v = d3d10_effect_get_variable_by_name(context->effect, name)))
> + {
> + WARN("Couldn't find variable %s.\n", debugstr_a(name));
> + return E_FAIL;
> + }
> +
> + /* 4 components per register */
> + p->vars[i].offset = info->register_index * 4;
> + p->vars[i].length = info->register_count * 4;
> +
> + cb_reg_count = max(cb_reg_count, info->register_index + info->register_count);
> + }
> +
> + /* Allocate contiguous "constant buffer" for all referenced variables. */
> + if (FAILED(hr = d3d10_reg_table_allocate(&p->cb, cb_reg_count * 4)))
> + {
> + WARN("Failed to allocate variables buffer.\n");
> + return hr;
> + }
> +
> + return S_OK;
> +}
More information about the wine-devel
mailing list