[PATCH vkd3d v3 02/12] vkd3d-shader/hlsl: Add texel_offset field to hlsl_ir_resource_load.

[Zebediah Figura (she/her)]

On 12/20/21 06:51, Francisco Casas wrote:
> December 20, 2021 8:02 AM, "Giovanni Mascellani" <gmascellani at codeweavers.com> wrote:
> 
>> Hi,
>>
>> On 17/12/21 20:12, Francisco Casas wrote:
>>
>>> @@ -1917,9 +1916,16 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs,
>>> struct hl
>>> hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc)))
>>> coords = params->args[1];
>>>> + if (params->args_count == 3)
>>> + {
>>> + if (!(offset = add_implicit_conversion(ctx, instrs, params->args[2],
>>> + hlsl_get_vector_type(ctx, HLSL_TYPE_INT, sampler_dim), loc)))
>>> + offset = params->args[2];
>>> + }
>>> +
>>
>> I don't understand the point of "offset = params->args[2]" is the implicit conversion fails. Can
>> that failure be recovered later? If not (and we are assigning something to offset just to keep
>> compiling and produce as much diagnostics as we can), then I'd rather assign NULL to avoid further
>> problems. Am I missing something?
>>
>> Thanks, Giovanni.
> 
> Hi,
> 
> Now that I look at it it doesn't make much sense,
> perhaps I did it because the same thing was done for the coordinates
> a couple of lines up:
> 
> if (!(coords = add_implicit_conversion(ctx, instrs, params->args[1],
>          hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc)))
>      coords = params->args[1];
> 
> Maybe it just makes sense to return false in both places,
> unless Zebediah has a reason to keep it this way.
> 

Yes, the idea was to allow compilation to proceed. It doesn't seem 
actually necessary for that to not be NULL here though; possibly it was 
the case in an earlier local version of the patch implementing Load(), 
or it was just a mistake on my part. So that can be removed.