[PATCH vkd3d v3 05/12] vkd3d-shader/hlsl: Add Gather methods with offset and tests.

[Henri Verbeet]

On Mon, 20 Dec 2021 at 18:40, Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
> On 12/20/21 10:14, Francisco Casas wrote:
> > December 17, 2021 8:13 PM, "Zebediah Figura (she/her)" <zfigura at codeweavers.com> wrote:
> >> I think the assert is a bit superfluous.
> >>
> >> On the other hand, when this kind of code pattern comes up, I tend to prefer using a helper, along
> >> the lines of
> >>
> >> else if (!strcmp(name, "GatherGreen"))
> >> {
> >> add_gather(HLSL_RESOURCE_GATHER_GREEN, 1, ...);
> >> }
> >>
> >> which avoids that kind of implicit agreement.
> >
> > I am more inclined to just remove the assert.
> > I must confess that I don't like jumping from function definition to function definition too much,
> > so I prefer not to make new functions unless they will be called in two or more different places.
> >
> > But if you insist I will add the helper, and not follow that policy of mine too much.
> > In that case I think it would also make sense to add helpers for Load and Sample too.
> > This since the code for these methods is fairly similar, so I think it would be better to keep
> > them close, also for the sake of consistency.
>
> I don't personally mind large functions, but on the other hand, the code
> style we use in wined3d tends to be relatively liberal with helpers (as
> long as they're well-defined, I guess.)
>
> It's a minor thing, really, just something that struck me as a possible
> improvement.
>
For what it's worth, the idea there is that well-defined helpers
provide information about the higher level intent and structure of the
code.