[PATCH v2 4/4] d3dx9: Implement ID3DXEffect::CloneEffect().

[Zebediah Figura]

On 2/12/22 13:54, Matteo Bruni wrote:
>> @@ -199,6 +199,10 @@ struct d3dx_effect
>>
>>       struct list parameter_block_list;
>>       struct d3dx_parameter_block *current_parameter_block;
>> +
>> +    char *source;
>> +    SIZE_T source_size;
>> +    char *skip_constants_string;
> 
> Cloning the "skip constants" setting seems reasonable but I'd prefer
> to have a test confirming that.

Done in v3.

>> +        case D3DXPT_SAMPLER:
>> +        case D3DXPT_SAMPLER1D:
>> +        case D3DXPT_SAMPLER2D:
>> +        case D3DXPT_SAMPLER3D:
>> +        case D3DXPT_SAMPLERCUBE:
>> +        case D3DXPT_PIXELSHADER:
>> +        case D3DXPT_VERTEXSHADER:
>> +            /* Nothing to do; these parameters are not mutable. */
>> +            break;
> 
> The test doesn't confirm that those are just recreated from scratch
> (rather than e.g. shared with the original effect). Can we add some
> tests for this?

Shaders are in fact shared if possible. Unless I'm misreading though, 
there's no way for samplers to be shared.

>> +    dst->manager = src->manager;
>> +    if (dst->manager)
>> +        dst->manager->lpVtbl->AddRef(dst->manager);
> 
> Again, this makes perfect sense but a test would make it foolproof.

This one, surprisingly enough, is wrong.

>> +    if (!(effect->source = malloc(data_size)))
>> +        return E_OUTOFMEMORY;
>> +    memcpy(effect->source, data, data_size);
>> +    effect->source_size = data_size;
>> +
> 
> We should probably do this only if D3DXFX_NOT_CLONEABLE is not set.
> 
>>       if (pool)
>>       {
>>           effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
>> @@ -6429,6 +6546,12 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
>>
>>       if (skip_constants_string)
>>       {
>> +        if (!(effect->skip_constants_string = strdup(skip_constants_string)))
>> +        {
>> +            hr = E_OUTOFMEMORY;
>> +            goto fail;
>> +        }
>> +
> 
> Same here.

Both done in v3.