Cleanup & optimizations

src/doodad.rs

@@ -1,6 +1,6 @@
 use cgmath::{Deg, Point2};
 
-use crate::{physics::Pfloat, Camera, Sprite, Spriteable};
+use crate::{physics::Pfloat, Sprite, Spriteable};
 
 pub struct Doodad {
 	pub sprite: String,
@@ -8,10 +8,9 @@ pub struct Doodad {
 }
 
 impl Spriteable for Doodad {
-	fn sprite(&self, camera: Camera) -> Sprite {
+	fn sprite(&self) -> Sprite {
 		return Sprite {
-			position: self.pos,
+			pos: self.pos,
-			camera: camera,
 			name: self.sprite.clone(),
 			angle: Deg { 0: 0.0 },
 			scale: 1.0,

src/main.rs

@@ -36,7 +36,7 @@ struct Camera {
 }
 
 trait Spriteable {
-	fn sprite(&self, camera: Camera) -> Sprite;
+	fn sprite(&self) -> Sprite;
 }
 
 struct Sprite {
@@ -44,16 +44,13 @@ struct Sprite {
 	name: String,
 
 	// This object's position, in world coordinates.
-	position: Point2<Pfloat>,
+	pos: Point2<Pfloat>,
 
 	scale: Pfloat,
 
 	// This sprite's rotation
 	// (relative to north, measured ccw)
 	angle: Deg<Pfloat>,
-
-	// The camera we want to draw this sprite from
-	camera: Camera,
 }
 
 struct Game {
@@ -119,8 +116,8 @@ impl Game {
 	fn sprites(&self) -> Vec<Sprite> {
 		let mut sprites: Vec<Sprite> = Vec::new();
 
-		sprites.append(&mut self.system.sprites(self.camera));
+		sprites.append(&mut self.system.sprites());
-		sprites.push(self.player.sprite(self.camera));
+		sprites.push(self.player.sprite());
 
 		return sprites;
 	}
@@ -141,7 +138,7 @@ pub async fn run() -> Result<()> {
 			Event::RedrawRequested(window_id) if window_id == gpu.window().id() => {
 				gpu.update();
 				game.update();
-				match gpu.render(&game.sprites()) {
+				match gpu.render(&game.sprites(), game.camera) {
 					Ok(_) => {}
 					// Reconfigure the surface if lost
 					Err(wgpu::SurfaceError::Lost) => gpu.resize(gpu.size),

src/render/bufferdata.rs

@@ -0,0 +1,89 @@
+use std::mem;
+
+// Represents a textured vertex in WGSL
+#[repr(C)]
+#[derive(Copy, Clone, Debug, bytemuck::Pod, bytemuck::Zeroable)]
+pub struct Vertex {
+	pub position: [f32; 3],
+	pub texture_coords: [f32; 2],
+}
+
+impl Vertex {
+	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
+		wgpu::VertexBufferLayout {
+			array_stride: mem::size_of::<Vertex>() as wgpu::BufferAddress,
+			step_mode: wgpu::VertexStepMode::Vertex,
+			attributes: &[
+				wgpu::VertexAttribute {
+					offset: 0,
+					shader_location: 0,
+					format: wgpu::VertexFormat::Float32x3,
+				},
+				wgpu::VertexAttribute {
+					offset: mem::size_of::<[f32; 3]>() as wgpu::BufferAddress,
+					shader_location: 1,
+					format: wgpu::VertexFormat::Float32x2,
+				},
+			],
+		}
+	}
+}
+
+// Represents a sprite instance in WGSL
+#[repr(C)]
+#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
+pub struct SpriteInstance {
+	// All transformations we need to put this sprite in its place
+	pub transform: [[f32; 4]; 4],
+
+	// Which texture we should use for this sprite
+	// (see TextureArray)
+	pub texture_index: u32,
+}
+
+impl SpriteInstance {
+	// Number of bytes used to store this data.
+	// Should match desc() below.
+	pub const SIZE: u64 = 20;
+
+	pub fn desc() -> wgpu::VertexBufferLayout<'static> {
+		wgpu::VertexBufferLayout {
+			array_stride: mem::size_of::<SpriteInstance>() as wgpu::BufferAddress,
+			// We need to switch from using a step mode of Vertex to Instance
+			// This means that our shaders will only change to use the next
+			// instance when the shader starts processing a new instance
+			step_mode: wgpu::VertexStepMode::Instance,
+			attributes: &[
+				// A mat4 takes up 4 vertex slots as it is technically 4 vec4s. We need to define a slot
+				// for each vec4. We'll have to reassemble the mat4 in the shader.
+				wgpu::VertexAttribute {
+					offset: 0,
+					// While our vertex shader only uses locations 0, and 1 now, in later tutorials, we'll
+					// be using 2, 3, and 4, for Vertex. We'll start at slot 5, not conflict with them later
+					shader_location: 5,
+					format: wgpu::VertexFormat::Float32x4,
+				},
+				wgpu::VertexAttribute {
+					offset: mem::size_of::<[f32; 4]>() as wgpu::BufferAddress,
+					shader_location: 6,
+					format: wgpu::VertexFormat::Float32x4,
+				},
+				wgpu::VertexAttribute {
+					offset: mem::size_of::<[f32; 8]>() as wgpu::BufferAddress,
+					shader_location: 7,
+					format: wgpu::VertexFormat::Float32x4,
+				},
+				wgpu::VertexAttribute {
+					offset: mem::size_of::<[f32; 12]>() as wgpu::BufferAddress,
+					shader_location: 8,
+					format: wgpu::VertexFormat::Float32x4,
+				},
+				wgpu::VertexAttribute {
+					offset: mem::size_of::<[f32; 16]>() as wgpu::BufferAddress,
+					shader_location: 9,
+					format: wgpu::VertexFormat::Uint32,
+				},
+			],
+		}
+	}
+}

src/render/gpustate.rs

@@ -1,12 +1,18 @@
 use anyhow::Result;
 use bytemuck;
-use cgmath::{Deg, EuclideanSpace, Matrix4, Point2, Vector3};
+use cgmath::{EuclideanSpace, Point2};
-use std::{iter, mem};
+use std::iter;
 use wgpu::{self, util::DeviceExt};
 use winit::{self, window::Window};
 
-use super::texturearray::TextureArray;
+use crate::{Camera, Sprite};
-use crate::Sprite;
+
+use super::{
+	bufferdata::{SpriteInstance, Vertex},
+	texturearray::TextureArray,
+	util::Transform,
+	SPRITE_MESH_INDICES, SPRITE_MESH_VERTICES,
+};
 
 pub struct GPUState {
 	device: wgpu::Device,
@@ -25,186 +31,6 @@ pub struct GPUState {
 	instance_buffer: wgpu::Buffer,
 }
 
-struct Instance {
-	transform: Transform,
-	texture_index: u32,
-}
-impl Instance {
-	fn to_raw(&self) -> InstanceRaw {
-		InstanceRaw {
-			model: (self.transform.to_matrix()).into(),
-			texture_index: self.texture_index,
-		}
-	}
-}
-
-#[repr(C)]
-#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
-struct InstanceRaw {
-	model: [[f32; 4]; 4],
-	texture_index: u32,
-}
-
-impl InstanceRaw {
-	fn get_size() -> u64 {
-		20
-	}
-
-	fn desc() -> wgpu::VertexBufferLayout<'static> {
-		wgpu::VertexBufferLayout {
-			array_stride: mem::size_of::<InstanceRaw>() as wgpu::BufferAddress,
-			// We need to switch from using a step mode of Vertex to Instance
-			// This means that our shaders will only change to use the next
-			// instance when the shader starts processing a new instance
-			step_mode: wgpu::VertexStepMode::Instance,
-			attributes: &[
-				// A mat4 takes up 4 vertex slots as it is technically 4 vec4s. We need to define a slot
-				// for each vec4. We'll have to reassemble the mat4 in the shader.
-				wgpu::VertexAttribute {
-					offset: 0,
-					// While our vertex shader only uses locations 0, and 1 now, in later tutorials, we'll
-					// be using 2, 3, and 4, for Vertex. We'll start at slot 5, not conflict with them later
-					shader_location: 5,
-					format: wgpu::VertexFormat::Float32x4,
-				},
-				wgpu::VertexAttribute {
-					offset: mem::size_of::<[f32; 4]>() as wgpu::BufferAddress,
-					shader_location: 6,
-					format: wgpu::VertexFormat::Float32x4,
-				},
-				wgpu::VertexAttribute {
-					offset: mem::size_of::<[f32; 8]>() as wgpu::BufferAddress,
-					shader_location: 7,
-					format: wgpu::VertexFormat::Float32x4,
-				},
-				wgpu::VertexAttribute {
-					offset: mem::size_of::<[f32; 12]>() as wgpu::BufferAddress,
-					shader_location: 8,
-					format: wgpu::VertexFormat::Float32x4,
-				},
-				wgpu::VertexAttribute {
-					offset: mem::size_of::<[f32; 16]>() as wgpu::BufferAddress,
-					shader_location: 9,
-					format: wgpu::VertexFormat::Uint32,
-				},
-			],
-		}
-	}
-}
-
-/// API correction matrix.
-/// cgmath uses OpenGL's matrix format, which
-/// needs to be converted to wgpu's matrix format.
-#[rustfmt::skip]
-const OPENGL_TO_WGPU_MATRIX: Matrix4<f32> = Matrix4::new(
-	1.0, 0.0, 0.0, 0.0,
-	0.0, 1.0, 0.0, 0.0,
-	0.0, 0.0, 0.5, 0.5,
-	0.0, 0.0, 0.0, 1.0,
-);
-
-struct Transform {
-	pos: Point2<f32>,   // position on screen
-	screen_aspect: f32, // width / height. Screen aspect ratio.
-	aspect: f32,        // width / height. Sprite aspect ratio.
-	scale: f32,         // if scale = 1, this sprite will be as tall as the screen.
-	rotate: Deg<f32>,   // Around this object's center, in degrees measured ccw from vertical
-}
-
-impl Transform {
-	/// Build a matrix that corresponds to this transformation.
-	fn to_matrix(&self) -> Matrix4<f32> {
-		// Note that our mesh starts centered at (0, 0).
-		// This is essential---we do not want scale and rotation
-		// changing our sprite's position!
-
-		// Apply sprite aspect ratio, preserving height.
-		// This must be done *before* rotation.
-		let sprite_aspect = Matrix4::from_nonuniform_scale(self.aspect, 1.0, 1.0);
-
-		// Apply provided scale
-		let scale = Matrix4::from_scale(self.scale);
-
-		// Apply rotation
-		let rotate = Matrix4::from_angle_z(self.rotate);
-
-		// Apply screen aspect ratio, again preserving height.
-		// This must be done AFTER rotation... think about it!
-		let screen_aspect = Matrix4::from_nonuniform_scale(1.0 / self.screen_aspect, 1.0, 1.0);
-
-		// After finishing all op, translate.
-		// This must be done last, all other operations
-		// require us to be at (0, 0).
-		let translate = Matrix4::from_translation(Vector3 {
-			x: self.pos.x,
-			y: self.pos.y,
-			z: 0.0,
-		});
-
-		// Order matters!
-		// The rightmost matrix is applied first.
-		return OPENGL_TO_WGPU_MATRIX * translate * screen_aspect * rotate * scale * sprite_aspect;
-	}
-}
-
-// Datatype for vertex buffer
-#[repr(C)]
-#[derive(Copy, Clone, Debug, bytemuck::Pod, bytemuck::Zeroable)]
-struct Vertex {
-	position: [f32; 3],
-	tex_coords: [f32; 2],
-}
-
-impl Vertex {
-	fn desc() -> wgpu::VertexBufferLayout<'static> {
-		wgpu::VertexBufferLayout {
-			array_stride: mem::size_of::<Vertex>() as wgpu::BufferAddress,
-			step_mode: wgpu::VertexStepMode::Vertex,
-			attributes: &[
-				wgpu::VertexAttribute {
-					offset: 0,
-					shader_location: 0,
-					format: wgpu::VertexFormat::Float32x3,
-				},
-				wgpu::VertexAttribute {
-					offset: mem::size_of::<[f32; 3]>() as wgpu::BufferAddress,
-					shader_location: 1,
-					format: wgpu::VertexFormat::Float32x2,
-				},
-			],
-		}
-	}
-}
-
-// 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 VERTICES: &[Vertex] = &[
-	Vertex {
-		position: [-1.0, 1.0, 0.0],
-		tex_coords: [0.0, 0.0],
-	},
-	Vertex {
-		position: [1.0, 1.0, 0.0],
-		tex_coords: [1.0, 0.0],
-	},
-	Vertex {
-		position: [1.0, -1.0, 0.0],
-		tex_coords: [1.0, 1.0],
-	},
-	Vertex {
-		position: [-1.0, -1.0, 0.0],
-		tex_coords: [0.0, 1.0],
-	},
-];
-
-const INDICES: &[u16] = &[0, 3, 2, 0, 2, 1];
-
 impl GPUState {
 	// We can draw at most this many sprites on the screen.
 	// TODO: compile-time option
@@ -305,7 +131,7 @@ impl GPUState {
 				vertex: wgpu::VertexState {
 					module: &shader,
 					entry_point: "vertex_shader_main",
-					buffers: &[Vertex::desc(), InstanceRaw::desc()],
+					buffers: &[Vertex::desc(), SpriteInstance::desc()],
 				},
 				fragment: Some(wgpu::FragmentState {
 					module: &shader,
@@ -339,20 +165,20 @@ impl GPUState {
 
 		let vertex_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
 			label: Some("vertex buffer"),
-			contents: bytemuck::cast_slice(VERTICES),
+			contents: bytemuck::cast_slice(SPRITE_MESH_VERTICES),
 			usage: wgpu::BufferUsages::VERTEX,
 		});
 
 		let index_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
 			label: Some("vertex index buffer"),
-			contents: bytemuck::cast_slice(INDICES),
+			contents: bytemuck::cast_slice(SPRITE_MESH_INDICES),
 			usage: wgpu::BufferUsages::INDEX,
 		});
 
 		let instance_buffer = device.create_buffer(&wgpu::BufferDescriptor {
 			label: Some("instance buffer"),
 			usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
-			size: InstanceRaw::get_size() * Self::SPRITE_LIMIT,
+			size: SpriteInstance::SIZE * Self::SPRITE_LIMIT,
 			mapped_at_creation: false,
 		});
 
@@ -386,7 +212,11 @@ impl GPUState {
 
 	pub fn update(&mut self) {}
 
-	pub fn render(&mut self, sprites: &Vec<Sprite>) -> Result<(), wgpu::SurfaceError> {
+	pub fn render(
+		&mut self,
+		sprites: &Vec<Sprite>,
+		camera: Camera,
+	) -> Result<(), wgpu::SurfaceError> {
 		let output = self.surface.get_current_texture()?;
 		let view = output
 			.texture
@@ -423,22 +253,49 @@ impl GPUState {
 		// (it may not be square!)
 		let screen_aspect = self.size.width as f32 / self.size.height as f32;
 
-		// TODO: warning when too many sprites are drawn.
+		// Game coordinates (relative to camera) of ne and sw corners of screen.
-		let mut instances: Vec<Instance> = Vec::new();
+		// Used to skip off-screen sprites.
+		let clip_ne = Point2::from((-1.0, 1.0)) * camera.zoom;
+		let clip_sw = Point2::from((1.0, -1.0)) * camera.zoom;
+
+		let mut instances: Vec<SpriteInstance> = Vec::new();
 		for s in sprites {
-			// Compute position on screen,
+			let pos = s.pos - camera.pos.to_vec();
-			// using logical pixels
-			let screen_pos: Point2<f32> = (s.position - s.camera.pos.to_vec()) / s.camera.zoom;
 			let texture = self.texture_array.get_texture(&s.name[..]);
 
-			instances.push(Instance {
+			// Game dimensions of this sprite post-scale
+			// We really need height / 2 to check if we're on the screen,
+			// but we omit the division so we get a small "margin"
+			// and so we can re-use this value.
+			let height = texture.height * s.scale;
+			let width = height * texture.aspect;
+
+			// Sprite scale is relative to the sprite's defined height,
+			// so we apply both factors here
+
+			// Don't compute matrices for sprites that are off the screen
+			if pos.x < clip_ne.x - width
+				|| pos.y > clip_ne.y + height
+				|| pos.x > clip_sw.x + width
+				|| pos.y < clip_sw.y - height
+			{
+				println!("skip {}", s.name);
+				continue;
+			}
+
+			let scale = height / camera.zoom;
+			let screen_pos: Point2<f32> = pos / camera.zoom;
+
+			instances.push(SpriteInstance {
 				transform: Transform {
 					pos: screen_pos,
 					aspect: texture.aspect,
 					screen_aspect,
-					scale: s.scale * (texture.height / s.camera.zoom),
 					rotate: s.angle,
-				},
+					scale,
+				}
+				.to_matrix()
+				.into(),
 				texture_index: texture.index,
 			})
 		}
@@ -450,19 +307,19 @@ impl GPUState {
 		}
 
 		// Write new sprite data to buffer
-		let instance_data: Vec<_> = instances.iter().map(Instance::to_raw).collect();
+		self.queue
-		self.queue.write_buffer(
+			.write_buffer(&self.instance_buffer, 0, bytemuck::cast_slice(&instances));
-			&self.instance_buffer,
-			0,
-			bytemuck::cast_slice(&instance_data),
-		);
 
 		render_pass.set_pipeline(&self.render_pipeline);
 		render_pass.set_bind_group(0, &self.texture_array.bind_group, &[]);
 		render_pass.set_vertex_buffer(0, self.vertex_buffer.slice(..));
 		render_pass.set_vertex_buffer(1, self.instance_buffer.slice(..));
 		render_pass.set_index_buffer(self.index_buffer.slice(..), wgpu::IndexFormat::Uint16);
-		render_pass.draw_indexed(0..INDICES.len() as u32, 0, 0..instances.len() as _);
+		render_pass.draw_indexed(
+			0..SPRITE_MESH_INDICES.len() as u32,
+			0,
+			0..instances.len() as _,
+		);
 
 		// begin_render_pass borrows encoder mutably, so we can't call finish()
 		// without dropping this variable.

src/render/mod.rs

@@ -1,6 +1,54 @@
-mod gpu;
+mod bufferdata;
+mod gpustate;
 mod rawtexture;
 mod texturearray;
+mod util;
 
-pub use gpu::GPUState;
+pub use gpustate::GPUState;
 pub use texturearray::Texture;
+
+use self::bufferdata::Vertex;
+use cgmath::Matrix4;
+
+/// API correction matrix.
+/// cgmath uses OpenGL's matrix format, which
+/// needs to be converted to wgpu's matrix format.
+#[rustfmt::skip]
+const OPENGL_TO_WGPU_MATRIX: Matrix4<f32> = Matrix4::new(
+	1.0, 0.0, 0.0, 0.0,
+	0.0, 1.0, 0.0, 0.0,
+	0.0, 0.0, 0.5, 0.5,
+	0.0, 0.0, 0.0, 1.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_MESH_VERTICES: &[Vertex] = &[
+	Vertex {
+		position: [-1.0, 1.0, 0.0],
+		texture_coords: [0.0, 0.0],
+	},
+	Vertex {
+		position: [1.0, 1.0, 0.0],
+		texture_coords: [1.0, 0.0],
+	},
+	Vertex {
+		position: [1.0, -1.0, 0.0],
+		texture_coords: [1.0, 1.0],
+	},
+	Vertex {
+		position: [-1.0, -1.0, 0.0],
+		texture_coords: [0.0, 1.0],
+	},
+];
+
+const SPRITE_MESH_INDICES: &[u16] = &[0, 3, 2, 0, 2, 1];

src/render/util.rs

@@ -0,0 +1,53 @@
+use cgmath::{Deg, Matrix4, Point2, Vector3};
+
+use super::OPENGL_TO_WGPU_MATRIX;
+
+// Represents a sprite tranformation.
+//
+// This produces a single matrix we can apply to a brand-new sprite instance
+// to put it in the right place, at the right angle, with the right scale.
+pub struct Transform {
+	pub pos: Point2<f32>,   // position on screen
+	pub screen_aspect: f32, // width / height. Screen aspect ratio.
+	pub aspect: f32,        // width / height. Sprite aspect ratio.
+	pub scale: f32,         // if scale = 1, this sprite will be as tall as the screen.
+	pub rotate: Deg<f32>,   // Around this object's center, in degrees measured ccw from vertical
+}
+
+impl Transform {
+	/// Build the matrix that corresponds to this transformation.
+	pub fn to_matrix(&self) -> Matrix4<f32> {
+		// Note that our mesh starts centered at (0, 0).
+		// This is essential---we do not want scale and rotation
+		// changing our sprite's position!
+
+		// Apply sprite aspect ratio, preserving height.
+		// This must be done *before* rotation.
+		//
+		// We apply the provided scale here as well as a minor optimization
+		let sprite_aspect_and_scale =
+			Matrix4::from_nonuniform_scale(self.aspect * self.scale, self.scale, 1.0);
+
+		// Apply rotation
+		let rotate = Matrix4::from_angle_z(self.rotate);
+
+		// Apply screen aspect ratio, again preserving height.
+		// This must be done AFTER rotation... think about it!
+		let screen_aspect = Matrix4::from_nonuniform_scale(1.0 / self.screen_aspect, 1.0, 1.0);
+
+		// After finishing all op, translate.
+		// This must be done last, all other operations
+		// require us to be at (0, 0).
+		let translate = Matrix4::from_translation(Vector3 {
+			x: self.pos.x,
+			y: self.pos.y,
+			z: 0.0,
+		});
+
+		// Order matters!
+		// The rightmost matrix is applied first.
+		return OPENGL_TO_WGPU_MATRIX
+			* translate * screen_aspect
+			* rotate * sprite_aspect_and_scale;
+	}
+}

src/ship.rs

@@ -1,10 +1,6 @@
 use cgmath::Point2;
 
-use crate::physics::Pfloat;
+use crate::{physics::Pfloat, physics::PhysBody, Sprite, Spriteable};
-use crate::physics::PhysBody;
-use crate::Camera;
-use crate::Sprite;
-use crate::Spriteable;
 
 pub enum ShipKind {
 	Gypsum,
@@ -33,10 +29,9 @@ impl Ship {
 }
 
 impl Spriteable for Ship {
-	fn sprite(&self, camera: Camera) -> Sprite {
+	fn sprite(&self) -> Sprite {
 		return Sprite {
-			position: self.body.pos,
+			pos: self.body.pos,
-			camera: camera,
 			name: self.kind.sprite().to_owned(),
 			angle: self.body.angle,
 			scale: 1.0,

src/system.rs

@@ -1,4 +1,4 @@
-use crate::{physics::Polar, Camera, Doodad, Sprite, Spriteable};
+use crate::{physics::Polar, Doodad, Sprite, Spriteable};
 use cgmath::Deg;
 
 pub struct System {
@@ -32,7 +32,7 @@ impl System {
 		return s;
 	}
 
-	pub fn sprites(&self, camera: Camera) -> Vec<Sprite> {
+	pub fn sprites(&self) -> Vec<Sprite> {
-		return self.bodies.iter().map(|x| x.sprite(camera)).collect();
+		return self.bodies.iter().map(|x| x.sprite()).collect();
 	}
 }