Vertex buffer cleanup

src/render/gpustate.rs

@@ -1,18 +1,21 @@
 use anyhow::Result;
 use bytemuck;
 use cgmath::{EuclideanSpace, Point2};
-use std::iter;
+use std::{iter, rc::Rc};
-use wgpu::{self, util::DeviceExt};
+use wgpu;
 use winit::{self, window::Window};
 
 use crate::{Camera, Sprite};
 
 use super::{
-	bufferdata::{PlainVertex, SpriteInstance, StarInstance, TexturedVertex},
 	pipeline::PipelineBuilder,
 	texturearray::TextureArray,
 	util::Transform,
-	SPRITE_INDICES, SPRITE_VERTICES, STARFIELD_VERTICES,
+	vertexbuffer::{
+		data::{SPRITE_INDICES, SPRITE_VERTICES},
+		types::{PlainVertex, SpriteInstance, StarInstance, TexturedVertex},
+		VertexBuffer,
+	},
 };
 
 pub struct GPUState {
@@ -27,13 +30,13 @@ pub struct GPUState {
 	sprite_pipeline: wgpu::RenderPipeline,
 	starfield_pipeline: wgpu::RenderPipeline,
 
-	star_vertex_buffer: wgpu::Buffer,
-	sprite_vertex_buffer: wgpu::Buffer,
-	sprite_index_buffer: wgpu::Buffer,
 	texture_array: TextureArray,
+	vertex_buffers: VertexBuffers,
+}
 
-	sprite_instance_buffer: wgpu::Buffer,
+struct VertexBuffers {
-	star_instance_buffer: wgpu::Buffer,
+	sprite: Rc<VertexBuffer>,
+	starfield: Rc<VertexBuffer>,
 }
 
 impl GPUState {
@@ -104,10 +107,28 @@ impl GPUState {
 			surface.configure(&device, &config);
 		}
 
+		let vertex_buffers = VertexBuffers {
+			sprite: Rc::new(VertexBuffer::new::<TexturedVertex, SpriteInstance>(
+				"sprite",
+				&device,
+				Some(SPRITE_VERTICES),
+				Some(SPRITE_INDICES),
+				Self::SPRITE_LIMIT,
+			)),
+
+			starfield: Rc::new(VertexBuffer::new::<PlainVertex, StarInstance>(
+				"starfield",
+				&device,
+				None,
+				None,
+				Self::STAR_LIMIT,
+			)),
+		};
+
 		// Load textures
 		let texture_array = TextureArray::new(&device, &queue)?;
 
-		// Render pipelines
+		// Create render pipelines
 		let sprite_pipeline = PipelineBuilder::new("sprite", &device)
 			.set_shader(include_str!(concat!(
 				env!("CARGO_MANIFEST_DIR"),
@@ -116,7 +137,7 @@ impl GPUState {
 			)))
 			.set_format(config.format)
 			.set_triangle(true)
-			.set_vertex_buffers(&[TexturedVertex::desc(), SpriteInstance::desc()])
+			.set_vertex_buffer(&vertex_buffers.sprite)
 			.set_bind_group_layouts(&[&texture_array.bind_group_layout])
 			.build();
 
@@ -128,56 +149,23 @@ impl GPUState {
 			)))
 			.set_format(config.format)
 			.set_triangle(false)
-			.set_vertex_buffers(&[PlainVertex::desc(), StarInstance::desc()])
+			.set_vertex_buffer(&vertex_buffers.starfield)
 			.build();
 
-		let sprite_vertex_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
-			label: Some("vertex buffer"),
-			contents: bytemuck::cast_slice(SPRITE_VERTICES),
-			usage: wgpu::BufferUsages::VERTEX,
-		});
-
-		let sprite_index_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
-			label: Some("vertex index buffer"),
-			contents: bytemuck::cast_slice(SPRITE_INDICES),
-			usage: wgpu::BufferUsages::INDEX,
-		});
-
-		let star_vertex_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
-			label: Some("vertex buffer"),
-			contents: bytemuck::cast_slice(STARFIELD_VERTICES),
-			usage: wgpu::BufferUsages::VERTEX,
-		});
-
-		let sprite_instance_buffer = device.create_buffer(&wgpu::BufferDescriptor {
-			label: Some("sprite instance buffer"),
-			usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
-			size: SpriteInstance::SIZE * Self::SPRITE_LIMIT,
-			mapped_at_creation: false,
-		});
-
-		let star_instance_buffer = device.create_buffer(&wgpu::BufferDescriptor {
-			label: Some("star instance buffer"),
-			usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
-			size: StarInstance::SIZE * Self::STAR_LIMIT,
-			mapped_at_creation: false,
-		});
-
 		return Ok(Self {
-			surface,
 			device,
-			queue,
 			config,
-			size,
+			surface,
+			queue,
+
 			window,
+			size,
+
 			sprite_pipeline,
 			starfield_pipeline,
-			sprite_vertex_buffer,
+
-			sprite_index_buffer,
-			star_vertex_buffer,
-			sprite_instance_buffer,
-			star_instance_buffer,
 			texture_array,
+			vertex_buffers,
 		});
 	}
 
@@ -298,7 +286,7 @@ impl GPUState {
 
 		// Write new sprite data to buffer
 		self.queue.write_buffer(
-			&self.sprite_instance_buffer,
+			&self.vertex_buffers.sprite.instances,
 			0,
 			bytemuck::cast_slice(&instances),
 		);
@@ -317,24 +305,18 @@ impl GPUState {
 			.collect();
 
 		self.queue.write_buffer(
-			&self.star_instance_buffer,
+			&self.vertex_buffers.starfield.instances,
 			0,
 			bytemuck::cast_slice(&nstances),
 		);
 
 		// Starfield pipeline
-		render_pass.set_vertex_buffer(0, self.star_vertex_buffer.slice(..));
+		self.vertex_buffers.starfield.set_in_pass(&mut render_pass);
-		render_pass.set_vertex_buffer(1, self.star_instance_buffer.slice(..));
 		render_pass.set_pipeline(&self.starfield_pipeline);
-		render_pass.draw(0..STARFIELD_VERTICES.len() as _, 0..nstances.len() as _);
+		render_pass.draw(0..1, 0..nstances.len() as _);
 
 		// Sprite pipeline
-		render_pass.set_vertex_buffer(0, self.sprite_vertex_buffer.slice(..));
+		self.vertex_buffers.sprite.set_in_pass(&mut render_pass);
-		render_pass.set_vertex_buffer(1, self.sprite_instance_buffer.slice(..));
-		render_pass.set_index_buffer(
-			self.sprite_index_buffer.slice(..),
-			wgpu::IndexFormat::Uint16,
-		);
 		render_pass.set_pipeline(&self.sprite_pipeline);
 		render_pass.draw_indexed(0..SPRITE_INDICES.len() as u32, 0, 0..instances.len() as _);
 

src/render/mod.rs

@@ -1,14 +1,13 @@
-mod bufferdata;
 mod gpustate;
 mod pipeline;
 mod rawtexture;
 mod texturearray;
 mod util;
+mod vertexbuffer;
 
 pub use gpustate::GPUState;
 pub use texturearray::Texture;
 
-use self::bufferdata::{PlainVertex, TexturedVertex};
 use cgmath::Matrix4;
 
 /// API correction matrix.
@@ -21,39 +20,3 @@ const OPENGL_TO_WGPU_MATRIX: Matrix4<f32> = Matrix4::new(
 	0.0, 0.0, 0.5, 0.5,
 	0.0, 0.0, 0.0, 1.0,
 );
-
-const STARFIELD_VERTICES: &[PlainVertex] = &[PlainVertex {
-	position: [0.0, 0.0, 0.0],
-}];
-
-// The surface we draw sprites on.
-// Every sprite is an instance of this.
-//
-// These vertices form a rectangle that covers the whole screen.
-// Two facts are important to note:
-// - This is centered at (0, 0), so scaling doesn't change a sprite's position
-// - At scale = 1, this covers the whole screen. Makes scale calculation easier.
-//
-// Screen coordinates range from -1 to 1, with the origin at the center.
-// Texture coordinates range from 0 to 1, with the origin at the top-left
-// and (1,1) at the bottom-right.
-const SPRITE_VERTICES: &[TexturedVertex] = &[
-	TexturedVertex {
-		position: [-1.0, 1.0, 0.0],
-		texture_coords: [0.0, 0.0],
-	},
-	TexturedVertex {
-		position: [1.0, 1.0, 0.0],
-		texture_coords: [1.0, 0.0],
-	},
-	TexturedVertex {
-		position: [1.0, -1.0, 0.0],
-		texture_coords: [1.0, 1.0],
-	},
-	TexturedVertex {
-		position: [-1.0, -1.0, 0.0],
-		texture_coords: [0.0, 1.0],
-	},
-];
-
-const SPRITE_INDICES: &[u16] = &[0, 3, 2, 0, 2, 1];

src/render/pipeline.rs

@@ -1,5 +1,8 @@
+use std::rc::Rc;
 use wgpu;
 
+use super::vertexbuffer::VertexBuffer;
+
 pub struct PipelineBuilder<'a> {
 	// These are provided with new()
 	label: &'a str,
@@ -7,12 +10,12 @@ pub struct PipelineBuilder<'a> {
 
 	// These have empty defaults
 	triangle: bool,
-	vertex_buffers: &'a [wgpu::VertexBufferLayout<'a>],
 	bind_group_layouts: &'a [&'a wgpu::BindGroupLayout],
 
 	// These must be provided
 	shader: Option<&'a str>,
 	format: Option<wgpu::TextureFormat>,
+	vertex_buffer: Option<&'a Rc<VertexBuffer>>,
 }
 
 impl<'a> PipelineBuilder<'a> {
@@ -25,7 +28,7 @@ impl<'a> PipelineBuilder<'a> {
 			device,
 
 			triangle: true,
-			vertex_buffers: &[],
+			vertex_buffer: None,
 			bind_group_layouts: &[],
 
 			shader: None,
@@ -48,11 +51,8 @@ impl<'a> PipelineBuilder<'a> {
 		self
 	}
 
-	pub fn set_vertex_buffers(
+	pub fn set_vertex_buffer(mut self, vertex_buffer: &'a Rc<VertexBuffer>) -> Self {
-		mut self,
+		self.vertex_buffer = Some(vertex_buffer);
-		vertex_buffers: &'a [wgpu::VertexBufferLayout<'a>],
-	) -> Self {
-		self.vertex_buffers = vertex_buffers;
 		self
 	}
 
@@ -99,7 +99,7 @@ impl<'a> PipelineBuilder<'a> {
 				vertex: wgpu::VertexState {
 					module: &shader,
 					entry_point: Self::VERTEX_MAIN,
-					buffers: self.vertex_buffers,
+					buffers: &self.vertex_buffer.unwrap().layout,
 				},
 
 				fragment: Some(wgpu::FragmentState {

src/render/shaders/sprite.wgsl

@@ -1,3 +1,4 @@
+// Vertex shader
 struct InstanceInput {
 	@location(2) transform_matrix_0: vec4<f32>,
 	@location(3) transform_matrix_1: vec4<f32>,
@@ -6,9 +7,6 @@ struct InstanceInput {
 	@location(6) texture_idx: u32,
 };
 
-
-// Vertex shader
-
 struct VertexInput {
 	@location(0) position: vec3<f32>,
 	@location(1) texture_coords: vec2<f32>,
@@ -41,7 +39,6 @@ fn vertex_shader_main(
 
 
 // Fragment shader
-
 @group(0) @binding(0)
 var texture_array: binding_array<texture_2d<f32>>;
 @group(0) @binding(1)

src/render/shaders/starfield.wgsl

@@ -1,3 +1,4 @@
+// Vertex shader
 struct InstanceInput {
 	@location(2) transform_matrix_0: vec4<f32>,
 	@location(3) transform_matrix_1: vec4<f32>,
@@ -5,19 +6,12 @@ struct InstanceInput {
 	@location(5) transform_matrix_3: vec4<f32>,
 };
 
-// Vertex shader
-
-struct VertexInput {
-	@location(0) position: vec3<f32>,
-}
-
 struct VertexOutput {
 	@builtin(position) clip_position: vec4<f32>,
 }
 
 @vertex
 fn vertex_shader_main(
-	model: VertexInput,
 	instance: InstanceInput,
 ) -> VertexOutput {
 	let transform_matrix = mat4x4<f32>(
@@ -27,12 +21,16 @@ fn vertex_shader_main(
 		instance.transform_matrix_3,
 	);
 
+	// Stars consist of only one vertex, so we don't need to pass a buffer into this shader.
+	// We need one instance per star, and that's it!
+	let pos = vec4<f32>(0.0, 0.0, 0.0, 1.0);
+
 	var out: VertexOutput;
-	out.clip_position = transform_matrix * vec4<f32>(model.position, 1.0);
+	out.clip_position = transform_matrix * pos;
 	return out;
 }
 
-
+// Fragment shader
 @fragment
 fn fragment_shader_main(in: VertexOutput) -> @location(0) vec4<f32> {
 	return vec4<f32>(1.0, 1.0, 1.0, 1.0);

src/render/vertexbuffer/data.rs

@@ -0,0 +1,36 @@
+use super::types::TexturedVertex;
+
+// The surface we draw sprites on.
+// Every sprite is an instance of this mesh.
+//
+// These vertices form a rectangle that covers the whole screen.
+// Two facts are important to note:
+// - This is centered at (0, 0), so scaling doesn't change a sprite's position
+// - At scale = 1, this covers the whole screen. Makes scale calculation easier.
+//
+// Screen coordinates range from -1 to 1, with the origin at the center.
+// Texture coordinates range from 0 to 1, with the origin at the top-left
+// and (1,1) at the bottom-right.
+//
+// We can't use the same trick we use for starfield here,
+// since each instance of a sprite has multiple vertices.
+pub const SPRITE_VERTICES: &[TexturedVertex] = &[
+	TexturedVertex {
+		position: [-1.0, 1.0, 0.0],
+		texture_coords: [0.0, 0.0],
+	},
+	TexturedVertex {
+		position: [1.0, 1.0, 0.0],
+		texture_coords: [1.0, 0.0],
+	},
+	TexturedVertex {
+		position: [1.0, -1.0, 0.0],
+		texture_coords: [1.0, 1.0],
+	},
+	TexturedVertex {
+		position: [-1.0, -1.0, 0.0],
+		texture_coords: [0.0, 1.0],
+	},
+];
+
+pub const SPRITE_INDICES: &[u16] = &[0, 3, 2, 0, 2, 1];

src/render/vertexbuffer/mod.rs

@@ -0,0 +1,19 @@
+pub mod data;
+pub mod types;
+mod vertexbuffer;
+
+pub use vertexbuffer::VertexBuffer;
+
+use std::mem;
+use wgpu;
+
+pub trait BufferObject
+where
+	Self: Sized + bytemuck::Pod,
+{
+	/// Number of bytes used to store this data.
+	/// Should match length in the layout() implementation.
+	const SIZE: u64 = mem::size_of::<Self>() as wgpu::BufferAddress;
+
+	fn layout() -> wgpu::VertexBufferLayout<'static>;
+}

src/render/vertexbuffer/types.rs

@@ -1,4 +1,7 @@
 use std::mem;
+use wgpu;
+
+use super::BufferObject;
 
 // Represents a textured vertex in WGSL
 #[repr(C)]
@@ -8,10 +11,10 @@ pub struct TexturedVertex {
 	pub texture_coords: [f32; 2],
 }
 
-impl TexturedVertex {
+impl BufferObject for TexturedVertex {
-	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
+	fn layout() -> wgpu::VertexBufferLayout<'static> {
 		wgpu::VertexBufferLayout {
-			array_stride: mem::size_of::<TexturedVertex>() as wgpu::BufferAddress,
+			array_stride: Self::SIZE,
 			step_mode: wgpu::VertexStepMode::Vertex,
 			attributes: &[
 				wgpu::VertexAttribute {
@@ -36,10 +39,10 @@ pub struct PlainVertex {
 	pub position: [f32; 3],
 }
 
-impl PlainVertex {
+impl BufferObject for PlainVertex {
-	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
+	fn layout() -> wgpu::VertexBufferLayout<'static> {
 		wgpu::VertexBufferLayout {
-			array_stride: mem::size_of::<PlainVertex>() as wgpu::BufferAddress,
+			array_stride: Self::SIZE,
 			step_mode: wgpu::VertexStepMode::Vertex,
 			attributes: &[wgpu::VertexAttribute {
 				offset: 0,
@@ -52,18 +55,14 @@ impl PlainVertex {
 
 // Represents a star instance in WGSL
 #[repr(C)]
-#[derive(Debug, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
+#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
 pub struct StarInstance {
 	// All transformations we need to put this star in its place
 	pub transform: [[f32; 4]; 4],
 }
 
-impl StarInstance {
+impl BufferObject for StarInstance {
-	// Number of bytes used to store this data.
+	fn layout() -> wgpu::VertexBufferLayout<'static> {
-	// Should match desc() below.
-	pub const SIZE: u64 = mem::size_of::<StarInstance>() as wgpu::BufferAddress;
-
-	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
 		wgpu::VertexBufferLayout {
 			array_stride: Self::SIZE,
 			// We need to switch from using a step mode of TexturedVertex to Instance
@@ -110,12 +109,8 @@ pub struct SpriteInstance {
 	pub texture_index: u32,
 }
 
-impl SpriteInstance {
+impl BufferObject for SpriteInstance {
-	// Number of bytes used to store this data.
+	fn layout() -> wgpu::VertexBufferLayout<'static> {
-	// Should match desc() below.
-	pub const SIZE: u64 = mem::size_of::<SpriteInstance>() as wgpu::BufferAddress;
-
-	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
 		wgpu::VertexBufferLayout {
 			array_stride: Self::SIZE,
 			// We need to switch from using a step mode of TexturedVertex to Instance

src/render/vertexbuffer/vertexbuffer.rs

@@ -0,0 +1,97 @@
+use wgpu::{self, util::DeviceExt};
+
+use super::BufferObject;
+
+pub struct VertexBuffer {
+	/// Vertices to draw.
+	///
+	/// This can be set to None if each instance
+	/// only needs one vertex. Note that the shader
+	/// must be prepared for this!
+	vertices: Option<wgpu::Buffer>,
+
+	/// Vertex indices. This will be None if we're
+	/// drawing vertices directly, without indices.
+	indices: Option<wgpu::Buffer>,
+
+	pub instances: wgpu::Buffer,
+	pub layout: Box<[wgpu::VertexBufferLayout<'static>]>,
+}
+
+impl VertexBuffer {
+	pub fn new<VertexType, InstanceType>(
+		label: &str,
+		device: &wgpu::Device,
+		vertices: Option<&[VertexType]>,
+		indices: Option<&[u16]>,
+		length: u64,
+	) -> Self
+	where
+		VertexType: BufferObject,
+		InstanceType: BufferObject,
+	{
+		let vertices = if let Some(vertices) = vertices {
+			Some(
+				device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
+					label: Some(&format!("[{}] vertex buffer", label)),
+					contents: bytemuck::cast_slice(vertices),
+					usage: wgpu::BufferUsages::VERTEX,
+				}),
+			)
+		} else {
+			None
+		};
+
+		let indices = if let Some(indices) = indices {
+			if vertices.is_none() {
+				unreachable!("Bad code! You cannot draw indexed vertices if vertices is None!")
+			}
+
+			Some(
+				device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
+					label: Some(&format!("[{}] index buffer", label)),
+					contents: bytemuck::cast_slice(indices),
+					usage: wgpu::BufferUsages::INDEX,
+				}),
+			)
+		} else {
+			None
+		};
+
+		let instances = device.create_buffer(&wgpu::BufferDescriptor {
+			label: Some(&format!("[{}] instance buffer", label)),
+			usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
+			size: InstanceType::SIZE * length,
+			mapped_at_creation: false,
+		});
+
+		let mut layout = Vec::new();
+		if vertices.is_some() {
+			layout.push(VertexType::layout());
+		}
+		layout.push(InstanceType::layout());
+
+		Self {
+			vertices,
+			indices,
+			instances,
+			layout: layout.into(),
+		}
+	}
+
+	/// Set the buffers defined here as buffers in a RenderPass.
+	pub fn set_in_pass<'a, 'b: 'a>(&'b self, render_pass: &'a mut wgpu::RenderPass<'b>) {
+		// TODO: why do these lifetimes work? Write an article!
+		let mut v_slot = 0u32;
+		if let Some(vertices) = &self.vertices {
+			render_pass.set_vertex_buffer(v_slot, vertices.slice(..));
+			v_slot += 1;
+		}
+		render_pass.set_vertex_buffer(v_slot, self.instances.slice(..));
+		//v_slot += 1;
+
+		if let Some(indices) = &self.indices {
+			render_pass.set_index_buffer(indices.slice(..), wgpu::IndexFormat::Uint16);
+		}
+	}
+}