Fix location assignment

[Firestar99]

Locations for Input, Output (and UniformConstant, where I have no idea how they work without explicit descriptor_set and binding) were assigned one per declaration, whereas they should have been assigned one per 16 bytes following the Vulkan spec on 16.1.4. Location and Component Assignment.

close #481.

Also adds an explicit location assignment, that keeps on counting from the last assigned value, just like glsl does:

rust-gpu/tests/compiletests/ui/spirv-attr/location_assignment_explicit.rs

Lines 25 to 32 in d00ba02

	#[spirv(vertex)]
	pub fn main(
	#[spirv(location = 4)] out1: &mut LargerThanVec4,
	out2: &mut Vec2, // should be 6
	#[spirv(location = 0)] out3: &mut Mat4,
	// 8 to 11 are unused, that's fine
	#[spirv(location = 12)] out4: &mut f32,
	) {

We don't check for overlaps when locations are manually assigned, leaving that for spirv-val:

rust-gpu/tests/compiletests/ui/spirv-attr/location_assignment_explicit_overlap.stderr

Lines 44 to 48 in d00ba02

	error: error:0:0 - [VUID-StandaloneSpirv-OpEntryPoint-08722] Entry-point has conflicting output location assignment at location 1, component 0
	OpEntryPoint Vertex %1 "main" %out1 %out2
	|
	= note: spirv-val failed
	= note: module `$TEST_BUILD_DIR/spirv-attr/location_assignment_explicit_overlap.vulkan1.2`

See vulkan spec on 16.1.4. Location and Component Assignment and on 16.9.4. Offset and Stride Assignment of structs. spirv spec is irrelevant here, it simply declares the Location decoration exists, and leaves all further spec for the client APIs to handle.

[marstaik]

Trying this branch out with this code:

#[shader_type]
pub struct BlinnPhongMaterialData {
	pub albedo:            Vec3,
	pub specular_color:    Vec3, // specular tint (often white for dielectrics)
	pub specular_strength: f32,  // controls the intensity of the specular highlight
	pub shininess:         f32,  // controls the size of the specular highlight
}

#[shader_type]
pub struct BlinnPhongFragData {
	pub position: Vec3, // surface position in world space
	pub normal:   Vec3, // surface normal in world space
	pub view_dir: Vec3, // direction from surface to camera
}

#[spirv(vertex)]
#[scene_descriptors]
pub fn blinn_phong_material_vert(
	#[camera] cameras: &[Camera],
	#[instance_data] instance_data: &[InstanceData],
	#[material_data] material_data: &[BlinnPhongMaterialData],
	#[spirv(instance_index)] instance_index: usize,
	// vert
	vertex: PositionNormal,
	// outputs
	out_material: &mut BlinnPhongMaterialData,
	out_frag: &mut BlinnPhongFragData,
	#[spirv(position)] out_pos: &mut Vec4,
) {
	/// 
}

#[spirv(fragment)]
#[scene_descriptors]
pub fn blinn_phong_material_frag(
	#[framebuffer_info] framebuffer_info: &FramebufferInfo,
	#[ambient_lights] ambient_lights: &[AmbientLight],
	#[directional_lights] directional_lights: &[DirectionalLight],
	#[point_lights] point_lights: &[PointLight],
	#[spot_lights] spot_lights: &[SpotLight],
	// outputs
	#[spirv(flat)] material: BlinnPhongMaterialData,
	frag: BlinnPhongFragData,
	// frag
	frag_color: &mut Vec4,
) {
	///
}

Still results in error:

  --- stderr
     Compiling rtd_scene_shaders v0.1.0 (/home/marios/proj/rtd/packages/rtd_scene_shaders)
  error: [VUID-StandaloneSpirv-OpEntryPoint-08722] Entry-point has conflicting output location assignment at location 2, component 0
    |
    = note: module `/home/marios/proj/rtd/target/spirv-builder/spirv-unknown-vulkan1.2/release/deps/rtd_scene_shaders.spv`

  warning: an unknown error occurred
    |
    = note: spirv-opt failed, leaving as unoptimized
    = note: module `/home/marios/proj/rtd/target/spirv-builder/spirv-unknown-vulkan1.2/release/deps/rtd_scene_shaders.spv`

  error: error:0:0 - [VUID-StandaloneSpirv-OpEntryPoint-08722] Entry-point has conflicting output location assignment at location 2, component 0
           OpEntryPoint Vertex %2 "materials::blinn_phong_material::blinn_phong_material_vert" %cameras %instance_data %material_data %instance_index %vertex %out_material %out_frag %out_pos
    |
    = note: spirv-val failed
    = note: module `/home/marios/proj/rtd/target/spirv-builder/spirv-unknown-vulkan1.2/release/deps/rtd_scene_shaders.spv`

Double checked to make sure it was from the right branch:

[marstaik]

Note: I do use scalar-block-layouts if that matters - both enabled in Vulkan1.2, and I pass the command line argument for spirv-builder

[Firestar99]

We don't care about scalar block layout or std430, we're just using whatever layout rustc gives us to ensure it's the same if you're using the same struct on shader and CPU side (at least since #380). If someone needs higher alignment (for silly wgpu uniform buffers or so), they should just #[repr(align(16))] the struct.

I'm not actually too sure how scalar block layout and spirv-val interact precisely. Even though it's not enabled by default, we've never experienced any issues wrt layout with it being disabled. Even though SpirvBuilder.validator.scalar_bock_layout exists.

Note that you are technically required to #[repr(C)] any struct shared between CPU and GPU, since rustc makes no guarantees about the rust layout. In theory, even two different compile invocations could result in different layouts, though in practice we haven't seen the layout algorithm change, like ever.

[Firestar99]

Even more minified code:

#[repr(C)]
#[derive(Copy, Clone, Default)]
pub struct ManyFloats {
    a: f32,
    b: f32,
    c: f32,
}

#[spirv(vertex)]
pub fn main(out1: &mut ManyFloats, #[spirv(location = 3)] out2: &mut f32) {
    const {
        assert!(size_of::<ManyFloats>() == 12);
    }
    *out1 = Default::default();
    *out2 = Default::default();
}

This fails as well, as according to spec array elements can't share slots, each array element has to get their own slot.

#[spirv(vertex)]
pub fn main(out1: &mut [f32; 3], out2: &mut f32) {
    *out1 = Default::default();
    *out2 = Default::default();
}

[Firestar99]

I've reread the Vulkan spec on location assignment and noticed that locations are not assigned for every 16 bytes, but there's a totally custom algorithm on how locations are assigned. Implemented the layout algorithm in SpirvType::location_size(), so our locations are hopefully correct now.

[marstaik]

Success! 🥳

I'm not actually too sure how scalar block layout and spirv-val interact precisely. Even though it's not enabled by default, we've never experienced any issues wrt layout with it being disabled. Even though SpirvBuilder.validator.scalar_bock_layout exists.

Fun fact: I actually had it enabled only on the Vulkan12 feature side, and the spirv compiler threw an error and I had to enable it.

[LegNeato]

I'll let @eddyb review this one as I am not qualified.