[PATCH vkd3d v2 2/5] vkd3d-shader: Track bytecode buffer size in bytes.

Matteo Bruni matteo.mystral at gmail.com
Wed Jul 7 08:55:28 CDT 2021


On Thu, Jul 1, 2021 at 6:08 PM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> On 7/1/21 4:29 AM, Matteo Bruni wrote:
> > On Thu, Jul 1, 2021 at 4:55 AM Zebediah Figura <zfigura at codeweavers.com> wrote:

> >> @@ -1524,12 +1515,12 @@ static void write_sm1_type(struct bytecode_buffer *buffer, struct hlsl_type *typ
> >>               write_sm1_type(buffer, field->type, ctab_start);
> >>           }
> >>
> >> -        fields_offset = (buffer->count - ctab_start) * sizeof(*buffer->data);
> >> +        fields_offset = get_buffer_size(buffer) - ctab_start;
> >
> > This isn't quite what I was expecting when I proposed the helper.
> > You access the current buffer offset whenever you want to reference it
> > afterwards, either in some range computation or to overwrite the
> > buffer in that location with the final value. In all cases (I
> > believe?) what you actually want is the aligned offset, i.e. the
> > location where you're going to start writing whatever is going to come
> > next in the buffer. Which is why, in principle, every access to the
> > current buffer offset needs to take the alignment (specifically of the
> > next thing that's going into the buffer, whenever it might make a
> > difference) into consideration.
> >
> > Renaming align_buffer to get_buffer_offset() (and getting rid of
> > get_buffer_size()) would do the trick, except it seems a bit ugly to
> > have a get_ function modify the buffer. So it's probably better to
> > only align the returned offset there without touching the buffer. That
> > means though that you still need to explicitly align the buffer after
> > the very last thing you're going to write (i.e. the current use of
> > align_buffer()). Unless...
> >
> > I'm not sure off the top of my head but it might be the case that the
> > alignment is a property of the buffer (e.g. always 4 for SM1 DXBC). In
> > that case it might make sense to store the alignment right in struct
> > bytecode_buffer instead and, probably, align after writing the data in
> > put_bytes() instead of before.
>
> That's essentially what we were doing before. The actual rule seems to
> be that alignment is always 4, *except* for strings, which as it turns
> out don't need to be aligned (in SM1 or SM4).
>
> Most of SM1/SM4 deals with bytecode offsets anyway, which means that
> theoretically the native parser could handle unaligned offsets. Since
> almost everything is aligned anyway it's a moot point.
>
> I guess I can dodge the issue by just leaving the alignment behaviour
> alone, though.

Okay, I wasn't entirely sure that the string alignment change was
intended. That means my last suggestion (i.e. my last paragraph above)
is invalid. The rest should still apply, so if at some point you want
to have another shot at changing the alignment handling, I guess you
could give them a try. No particular hurry of course.



More information about the wine-devel mailing list