[PATCH 2/5] d3d10/effect: Implement numeric pass properties updates.

Matteo Bruni matteo.mystral at gmail.com
Fri Oct 29 13:35:09 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/d3d10_private.h |   9 +
>  dlls/d3d10/effect.c        | 341 ++++++++++++++++++++++++++-----------
>  dlls/d3d10/tests/effect.c  | 207 ++++++++++++++++++----
>  3 files changed, 420 insertions(+), 137 deletions(-)
>
> diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
> index 11b3b4e9482..f8e415f860b 100644
> --- a/dlls/d3d10/d3d10_private.h
> +++ b/dlls/d3d10/d3d10_private.h
> @@ -95,6 +95,13 @@ struct d3d10_effect_shader_variable
>      unsigned int isinline : 1;
>  };
>
> +struct d3d10_effect_prop_dependencies
> +{
> +    struct d3d10_effect_prop_dependency *entries;
> +    SIZE_T count;
> +    SIZE_T capacity;
> +};
> +
>  struct d3d10_effect_sampler_desc
>  {
>      D3D10_SAMPLER_DESC desc;
> @@ -118,6 +125,7 @@ struct d3d10_effect_state_object_variable
>          ID3D10SamplerState *sampler;
>          IUnknown *object;
>      } object;
> +    struct d3d10_effect_prop_dependencies dependencies;
>  };
>
>  struct d3d10_effect_resource_variable
> @@ -217,6 +225,7 @@ struct d3d10_effect_pass
>      char *name;
>      struct d3d10_effect_annotations annotations;
>
> +    struct d3d10_effect_prop_dependencies dependencies;
>      struct d3d10_effect_pass_shader_desc vs;
>      struct d3d10_effect_pass_shader_desc ps;
>      struct d3d10_effect_pass_shader_desc gs;
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 23374fdb48f..34a5eb1b701 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -168,6 +168,27 @@ enum d3d10_effect_container_type
>      D3D10_C_SAMPLER,
>  };
>
> +struct d3d10_effect_prop_dependency
> +{
> +    unsigned int id;
> +    unsigned int idx;
> +    unsigned int operation;
> +    union
> +    {
> +        struct
> +        {
> +            struct d3d10_effect_variable *v;
> +            unsigned int offset;
> +        } var;
> +    } u;
> +};

It should be possible to use anonymous unions now in d3d10, right?
Not that you necessarily have to do that here, depending on what's
going to happen to this struct in the following patches.

> @@ -386,6 +407,148 @@ static const char *debug_d3d10_shader_variable_type(D3D10_SHADER_VARIABLE_TYPE t
>
>  #undef WINE_D3D10_TO_STR
>
> +static HRESULT d3d10_effect_variable_get_raw_value(struct d3d10_effect_variable *v,
> +        void *data, unsigned int offset, unsigned int count)
> +{
> +    BOOL is_buffer;
> +
> +    is_buffer = v->type->basetype == D3D10_SVT_CBUFFER || v->type->basetype == D3D10_SVT_TBUFFER;
> +
> +    if (v->type->type_class == D3D10_SVC_OBJECT && !is_buffer)
> +    {
> +        WARN("Not supported on object variables of type %s.\n",
> +                debug_d3d10_shader_variable_type(v->type->basetype));
> +        return D3DERR_INVALIDCALL;
> +    }
> +
> +    if (!is_buffer)
> +    {
> +        offset += v->buffer_offset;
> +        v = v->buffer;
> +    }
> +
> +    memcpy(data, v->u.buffer.local_buffer + offset, count);
> +
> +    return S_OK;
> +}
> +
> +static BOOL read_float_value(DWORD value, D3D_SHADER_VARIABLE_TYPE in_type, float *out_data, UINT idx)

I think you forgot to update these helpers after you introduced
d3d10_effect_read_numeric_value(), specifically WRT replacing DWORD
with uint32_t for value and UINT with unsigned int for idx.

Actually, it's probably nicer if you split the introduction of that
helper to a separate patch. You could also change the argument types
of read_value_list() in the same patch.

> +static void d3d10_effect_update_dependent_props(struct d3d10_effect_prop_dependencies *deps,
> +        void *container)
> +{
> +    const struct d3d10_effect_state_property_info *property_info;
> +    struct d3d10_effect_prop_dependency *d;
> +    struct d3d10_effect_variable *v;
> +    unsigned int i, j, count;
> +    uint32_t value;
> +    void *dst;
> +
> +    for (i = 0; i < deps->count; ++i)
> +    {
> +        d = &deps->entries[i];
> +
> +        property_info = &property_infos[d->id];
> +
> +        dst = (char *)container + property_info->offset;
> +
> +        switch (d->operation)
> +        {
> +            case D3D10_EOO_VAR:
> +            case D3D10_EOO_CONST_INDEX:
> +
> +                v = d->u.var.v;
> +
> +                count = v->type->type_class == D3D10_SVC_VECTOR ? 4 : 1;
> +
> +                for (j = 0; j < count; ++j)
> +                {
> +                    d3d10_effect_variable_get_raw_value(v, &value, d->u.var.offset + j * sizeof(value), sizeof(value));
> +                    d3d10_effect_read_numeric_value(value, v->type->basetype, property_info->type, dst, j);
> +                }
> +
> +                break;
> +
> +            default:
> +                FIXME("Unsupported property update for %u.\n", d->operation);
> +        }
> +    }
> +}
> +

It would be nice to have dirty flags to avoid unnecessary
recomputation. For example, something like the "changed" flag we have
for buffers but referring to variables and their values. Problem is,
there are a few quirks (e.g. SetRawValue(), those wild out-of-bounds
accesses) that make this quite problematic. So I can be convinced to
let it go :)

It would be something for a separate patch anyway.

> @@ -1695,6 +1793,14 @@ static BOOL read_value_list(const char *data, size_t data_size, DWORD offset,
>                  *(void **)out_data = &null_shader_resource_variable;
>                  break;
>
> +            case D3D10_SVT_DEPTHSTENCIL:
> +                *(void **)out_data = &null_depth_stencil_variable;
> +                break;
> +
> +            case D3D10_SVT_BLEND:
> +                *(void **)out_data = &null_blend_variable;
> +                break;
> +
>              default:
>                  FIXME("Unhandled out_type %#x.\n", out_type);
>                  return FALSE;

Would this also make sense as a separate patch?

> @@ -1745,21 +1851,31 @@ static BOOL is_object_property_type_matching(const struct d3d10_effect_state_pro
>      }
>  }
>
> +static HRESULT d3d10_effect_add_prop_dependency(struct d3d10_effect_prop_dependencies *d,
> +        const struct d3d10_effect_prop_dependency *dep)
> +{
> +    if (!d3d_array_reserve((void **)&d->entries, &d->capacity, d->count + 1, sizeof(*d->entries)))
> +        return E_OUTOFMEMORY;
> +
> +    d->entries[d->count++] = *dep;
> +
> +    return S_OK;
> +}
> +

We probably want to go for the usual exponential growth pattern here.
This separate helper is nice in that it can hide that kind of details
away.

> @@ -4873,6 +5004,7 @@ static BOOL get_value_as_bool(void *src_data, D3D10_SHADER_VARIABLE_TYPE src_typ
>      {
>          case D3D10_SVT_FLOAT:
>          case D3D10_SVT_INT:
> +        case D3D10_SVT_UINT:
>          case D3D10_SVT_BOOL:
>              if (*(DWORD *)src_data)
>                  return -1;
> @@ -4893,6 +5025,7 @@ static int get_value_as_int(void *src_data, D3D10_SHADER_VARIABLE_TYPE src_type)
>              return (int)(*(float *)src_data);
>
>          case D3D10_SVT_INT:
> +        case D3D10_SVT_UINT:
>              return *(int *)src_data;
>
>          case D3D10_SVT_BOOL:
> @@ -4911,6 +5044,7 @@ static float get_value_as_float(void *src_data, D3D10_SHADER_VARIABLE_TYPE src_t
>              return *(float *)src_data;
>
>          case D3D10_SVT_INT:
> +        case D3D10_SVT_UINT:
>              return (float)(*(int *)src_data);
>
>          case D3D10_SVT_BOOL:
> @@ -4941,6 +5075,7 @@ static void get_vector_as_type(BYTE *dst_data, D3D_SHADER_VARIABLE_TYPE dst_type
>                      break;
>
>                  case D3D10_SVT_INT:
> +                case D3D10_SVT_UINT:
>                      *(int *)dst_data_dword = get_value_as_int(src_data_dword, src_type);
>                      break;
>

These hunks should probably be split to a separate patch...

> diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c
> index a8a64a6ed0d..b379fc7bddc 100644
> --- a/dlls/d3d10/tests/effect.c
> +++ b/dlls/d3d10/tests/effect.c
> @@ -4843,43 +4843,32 @@ cbuffer cb
>      float f0, f_a[2];
>      int i0, i_a[2];
>      bool b0, b_a[2];
> +    uint i1, i1_a[2];
>  };
>  #endif
>  static DWORD fx_test_scalar_variable[] =
>  {
> -    0x43425844, 0xe4da4aa6, 0x1380ddc5, 0x445edad5,
> -    0x08581666, 0x00000001, 0x0000020b, 0x00000001,
> -    0x00000024, 0x30315846, 0x000001df, 0xfeff1001,
> -    0x00000001, 0x00000006, 0x00000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x00000000, 0x000000d3,
> -    0x00000000, 0x00000000, 0x00000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x00000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x00000000, 0x66006263,
> -    0x74616f6c, 0x00000700, 0x00000100, 0x00000000,
> -    0x00000400, 0x00001000, 0x00000400, 0x00090900,
> -    0x00306600, 0x00000007, 0x00000001, 0x00000002,
> -    0x00000014, 0x00000010, 0x00000008, 0x00000909,
> -    0x00615f66, 0x00746e69, 0x0000004c, 0x00000001,
> -    0x00000000, 0x00000004, 0x00000010, 0x00000004,
> -    0x00000911, 0x4c003069, 0x01000000, 0x02000000,
> -    0x14000000, 0x10000000, 0x08000000, 0x11000000,
> -    0x69000009, 0x6200615f, 0x006c6f6f, 0x0000008f,
> -    0x00000001, 0x00000000, 0x00000004, 0x00000010,
> -    0x00000004, 0x00000921, 0x8f003062, 0x01000000,
> -    0x02000000, 0x14000000, 0x10000000, 0x08000000,
> -    0x21000000, 0x62000009, 0x0400615f, 0x70000000,
> -    0x00000000, 0x06000000, 0xff000000, 0x00ffffff,
> -    0x29000000, 0x0d000000, 0x00000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x00000000, 0x48000000,
> -    0x2c000000, 0x00000000, 0x10000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x6c000000, 0x50000000,
> -    0x00000000, 0x24000000, 0x00000000, 0x00000000,
> -    0x00000000, 0x8b000000, 0x6f000000, 0x00000000,
> -    0x30000000, 0x00000000, 0x00000000, 0x00000000,
> -    0xb0000000, 0x94000000, 0x00000000, 0x44000000,
> -    0x00000000, 0x00000000, 0x00000000, 0xcf000000,
> -    0xb3000000, 0x00000000, 0x50000000, 0x00000000,
> -    0x00000000, 0x00000000, 0x00000000,
> +    0x43425844, 0x7d97f44c, 0x1da4b110, 0xb710407e, 0x26750c1c, 0x00000001, 0x00000288, 0x00000001,
> +    0x00000024, 0x30315846, 0x0000025c, 0xfeff1001, 0x00000001, 0x00000008, 0x00000000, 0x00000000,
> +    0x00000000, 0x00000000, 0x00000000, 0x00000118, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> +    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x66006263,
> +    0x74616f6c, 0x00000700, 0x00000100, 0x00000000, 0x00000400, 0x00001000, 0x00000400, 0x00090900,
> +    0x00306600, 0x00000007, 0x00000001, 0x00000002, 0x00000014, 0x00000010, 0x00000008, 0x00000909,
> +    0x00615f66, 0x00746e69, 0x0000004c, 0x00000001, 0x00000000, 0x00000004, 0x00000010, 0x00000004,
> +    0x00000911, 0x4c003069, 0x01000000, 0x02000000, 0x14000000, 0x10000000, 0x08000000, 0x11000000,
> +    0x69000009, 0x6200615f, 0x006c6f6f, 0x0000008f, 0x00000001, 0x00000000, 0x00000004, 0x00000010,
> +    0x00000004, 0x00000921, 0x8f003062, 0x01000000, 0x02000000, 0x14000000, 0x10000000, 0x08000000,
> +    0x21000000, 0x62000009, 0x7500615f, 0x00746e69, 0x000000d3, 0x00000001, 0x00000000, 0x00000004,
> +    0x00000010, 0x00000004, 0x00000919, 0xd3003169, 0x01000000, 0x02000000, 0x14000000, 0x10000000,
> +    0x08000000, 0x19000000, 0x69000009, 0x00615f31, 0x00000004, 0x00000090, 0x00000000, 0x00000008,
> +    0xffffffff, 0x00000000, 0x00000029, 0x0000000d, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> +    0x00000000, 0x00000048, 0x0000002c, 0x00000000, 0x00000010, 0x00000000, 0x00000000, 0x00000000,
> +    0x0000006c, 0x00000050, 0x00000000, 0x00000024, 0x00000000, 0x00000000, 0x00000000, 0x0000008b,
> +    0x0000006f, 0x00000000, 0x00000030, 0x00000000, 0x00000000, 0x00000000, 0x000000b0, 0x00000094,
> +    0x00000000, 0x00000044, 0x00000000, 0x00000000, 0x00000000, 0x000000cf, 0x000000b3, 0x00000000,
> +    0x00000050, 0x00000000, 0x00000000, 0x00000000, 0x000000f4, 0x000000d8, 0x00000000, 0x00000064,
> +    0x00000000, 0x00000000, 0x00000000, 0x00000113, 0x000000f7, 0x00000000, 0x00000070, 0x00000000,
> +    0x00000000, 0x00000000,
>  };
>
>  static void test_scalar_methods(ID3D10EffectScalarVariable *var, D3D10_SHADER_VARIABLE_TYPE type,
> @@ -5146,6 +5135,7 @@ static void test_effect_scalar_variable(void)
>          {"f0", D3D10_SVT_FLOAT},
>          {"i0", D3D10_SVT_INT},
>          {"b0", D3D10_SVT_BOOL},
> +        {"i1", D3D10_SVT_UINT},
>          {"f_a", D3D10_SVT_FLOAT, TRUE},
>          {"i_a", D3D10_SVT_INT, TRUE},
>          {"b_a", D3D10_SVT_BOOL, TRUE},
> @@ -5178,7 +5168,7 @@ static void test_effect_scalar_variable(void)
>              effect_desc.ConstantBuffers);
>      ok(effect_desc.SharedConstantBuffers == 0, "Unexpected shared constant buffers count %u.\n",
>              effect_desc.SharedConstantBuffers);
> -    ok(effect_desc.GlobalVariables == 6, "Unexpected global variables count %u.\n",
> +    ok(effect_desc.GlobalVariables == 8, "Unexpected global variables count %u.\n",
>              effect_desc.GlobalVariables);
>      ok(effect_desc.SharedGlobalVariables == 0, "Unexpected shared global variables count %u.\n",
>              effect_desc.SharedGlobalVariables);

... together with this test change.



More information about the wine-devel mailing list