Add Single Member Construction Functions

[cwfitzgerald]

This isn't totally finished, wanted to get your opinion of the API change before I went and wrote more docs and tests.

The main change is that I switched the alignment operations from after the write to before the write. This is not an observable change with existing apis, but makes this API possible.

[teoxoy]

Given that it's not possible to specify size/alignment of individual members (this is probably not very important for the storage address space but it's how you satisfy the requirements of the uniform address space in some cases) and that we need to track more data for the uniform address space, should we ignore uniform buffers for this API?

Also, should we add this API to the non dynamic buffers as well?

[teoxoy]

This assert won't suffice, since there are 2 more invariants that need to be checked.

encase/derive/impl/src/lib.rs

Lines 347 to 381 in 14a6ebf

	let field_offset_check = quote_spanned! {ident.span()=>
	if let ::core::option::Option::Some(min_alignment) =
	<#ty as #root::ShaderType>::METADATA.uniform_min_alignment()
	{
	let offset = <Self as #root::ShaderType>::METADATA.offset(#i);
	#root::concat_assert!(
	min_alignment.is_aligned(offset),
	"offset of field '", #name, "' must be a multiple of ", min_alignment.get(),
	" (current offset: ", offset, ")"
	)
	}
	};
	let field_offset_diff = if i != 0 {
	let prev_field = &field_data[i - 1];
	let prev_field_ty = &prev_field.field.ty;
	let prev_ident_name = prev_field.ident().to_string();
	quote_spanned! {ident.span()=>
	if let ::core::option::Option::Some(min_alignment) =
	<#prev_field_ty as #root::ShaderType>::METADATA.uniform_min_alignment()
	{
	let prev_offset = <Self as #root::ShaderType>::METADATA.offset(#i - 1);
	let offset = <Self as #root::ShaderType>::METADATA.offset(#i);
	let diff = offset - prev_offset;
	let prev_size = <#prev_field_ty as #root::ShaderSize>::SHADER_SIZE.get();
	let prev_size = min_alignment.round_up(prev_size);
	#root::concat_assert!(
	diff >= prev_size,
	"offset between fields '", #prev_ident_name, "' and '", #name, "' must be at least ",
	min_alignment.get(), " (currently: ", diff, ")"
	)
	}
	}

[cwfitzgerald]

I'm not sure what exact rules these are checking for. Could you either explain them or point to the wgsl spec for them.

[teoxoy]

They are listed in https://gpuweb.github.io/gpuweb/wgsl/#address-space-layout-constraints but the short version is that:

• the offset of array/struct members needs to be aligned to 16
• given a member of type struct (S) at offset o0 and its subsequent member at offset o1,
  o1 - o0 >= roundUp(16, SizeOf(S)) must be true (definitions: roundUp SizeOf)

[teoxoy]

This might not be enough, some structs needs padding between the last field and the struct end; we probably have to keep track of the alignment of the current struct and pad here explicitly.

Also, we should probably error if no struct fields have been written but this fn was called since it's not valid to have an empty struct in WGSL.

[cwfitzgerald]

This might not be enough, some structs needs padding between the last field and the struct end; we probably have to keep track of the alignment of the current struct and pad here explicitly.

If self.alignment > needed_alignment (which we have asserted to be true during construction of a dynamic buffer as needed_alignment can't be more than 32), then we should always be good here.

[teoxoy]

Right, but the issue is that if the buffer itself is smaller than the size of the struct, we don't currently enlarge it to be the size of the struct (we only increment the offset).

[cwfitzgerald]

Should have added code to deal with this.

[teoxoy]

Looks good! What do you think of erroring on empty structs?

[cwfitzgerald]

I'n not sure it's necissarily the responsibility of this to detect that?

[cwfitzgerald]

I didn't add them to regular buffers are regular buffers don't have an offset to bump. If we were to add one, I feel like it would be very confusing that calling write twice (or write + per-member) would overwrite each other instead of following this offset.

[cwfitzgerald]

I very much need uniform buffers (shipping on WebGL) so I'll have to figure that one out.

[teoxoy]

I didn't add them to regular buffers are regular buffers don't have an offset to bump. If we were to add one, I feel like it would be very confusing that calling write twice (or write + per-member) would overwrite each other instead of following this offset.

Hmm, right. Maybe we can change the API on the non dynamic buffers to move self instead?

And/or depending on how much extra data we need to uphold the same invariants for these new operations, we could add new wrapper structs instead of adding new methods onto the existing ones.

[cwfitzgerald]

I'll give it a shot with it in the dynamic buffer one, and then see if we need to split it out.