refactor(render): docs/comments and error handling for missing textures

8 files changed, 155 insertions, 37 deletions

diff --git a/stockton-render/src/draw/buffer.rs b/stockton-render/src/draw/buffer.rs
index 006a37b..cbe56f4 100644
--- a/stockton-render/src/draw/buffer.rs
+++ b/stockton-render/src/draw/buffer.rs
@@ -29,6 +29,8 @@ use hal::{
 use crate::error::CreationError;
 use crate::types::*;
 
+/// Create a buffer of the given specifications, allocating more device memory.
+// TODO: Use a different memory allocator?
 pub(crate) fn create_buffer(device: &mut Device,
 	adapter: &Adapter,
 	usage: Usage,
@@ -59,20 +61,38 @@ pub(crate) fn create_buffer(device: &mut Device,
 	Ok((buffer, memory))
 }
 
+/// A buffer that can be modified by the CPU
 pub trait ModifiableBuffer: IndexMut<usize> {
+	/// Get a handle to the underlying GPU buffer
 	fn get_buffer<'a>(&'a mut self) -> &'a Buffer;
+
+	/// Commit all changes to GPU memory, returning a handle to the GPU buffer
 	fn commit<'a>(&'a mut self, device: &Device,
 		command_queue: &mut CommandQueue, 
 		command_pool: &mut CommandPool) -> &'a Buffer;
 }
 
+/// A GPU buffer that is written to using a staging buffer
 pub struct StagedBuffer<'a, T: Sized> {
+	/// CPU-visible buffer
 	staged_buffer: ManuallyDrop<Buffer>,
+
+	/// CPU-visible memory
 	staged_memory: ManuallyDrop<Memory>,
+
+	/// GPU Buffer
 	buffer: ManuallyDrop<Buffer>,
+
+	/// GPU Memory
 	memory: ManuallyDrop<Memory>,
+
+	/// Where staged buffer is mapped in CPU memory
 	staged_mapped_memory: &'a mut [T],
+
+	/// If staged memory has been changed since last `commit`
 	staged_is_dirty: bool,
+
+	/// The highest index in the buffer that's been written to.
 	pub highest_used: usize
 }
 
@@ -80,9 +100,13 @@ pub struct StagedBuffer<'a, T: Sized> {
 impl<'a, T: Sized> StagedBuffer<'a, T> {
 	/// size is the size in T
 	pub fn new(device: &mut Device, adapter: &Adapter, usage: Usage, size: u64) -> Result<Self, CreationError> {
-
+		// Convert size to bytes
 		let size_bytes = size * size_of::<T>() as u64;
+
+		// Get CPU-visible buffer
 		let (staged_buffer, staged_memory) = create_buffer(device, adapter, Usage::TRANSFER_SRC, Properties::CPU_VISIBLE, size_bytes)?;
+		
+		// Get GPU Buffer
 		let (buffer, memory) = create_buffer(device, adapter, Usage::TRANSFER_DST | usage, Properties::DEVICE_LOCAL, size_bytes)?;
 
 		// Map it somewhere and get a slice to that memory
@@ -103,6 +127,7 @@ impl<'a, T: Sized> StagedBuffer<'a, T> {
 		})
 	}
 
+	/// Call this before dropping
 	pub(crate) fn deactivate(mut self, device: &mut Device) {
 		unsafe { 
 			device.unmap_memory(&self.staged_memory);
@@ -124,6 +149,7 @@ impl <'a, T: Sized> ModifiableBuffer for StagedBuffer<'a, T> {
 	fn commit<'b>(&'b mut self, device: &Device,
 		command_queue: &mut CommandQueue, 
 		command_pool: &mut CommandPool) -> &'b Buffer {
+		// Only commit if there's changes to commit.
 		if self.staged_is_dirty {
 
 			// Flush mapped memory to ensure the staged buffer is filled
diff --git a/stockton-render/src/draw/context.rs b/stockton-render/src/draw/context.rs
index dc7703d..eacfc44 100644
--- a/stockton-render/src/draw/context.rs
+++ b/stockton-render/src/draw/context.rs
@@ -14,8 +14,8 @@
 // with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Deals with all the Vulkan/HAL details.
-//! In the end, this takes in vertices and renders them to a window.
-//! You'll need something else to actually generate the vertices though.
+//! In the end, this takes in a depth-sorted list of faces and a map file and renders them.
+//! You'll need something else to actually find/sort the faces though.
 
 use std::{
 	mem::{ManuallyDrop, size_of},
@@ -72,48 +72,87 @@ const FRAGMENT_SOURCE: &str = include_str!("./data/stockton.frag");
 pub struct UVPoint (pub Vector3, pub i32, pub Vector2);
 
 /// Contains all the hal related stuff.
-/// In the end, this takes some 3D points and puts it on the screen.
+/// In the end, this takes in a depth-sorted list of faces and a map file and renders them.
 // TODO: Settings for clear colour, buffer sizes, etc
 pub struct RenderingContext<'a> {
 	// Parents for most of these things
+
+	/// Vulkan Instance
 	instance: ManuallyDrop<back::Instance>,
+
+	/// Device we're using
 	device: ManuallyDrop<Device>,
+
+	/// Adapter we're using
 	adapter: Adapter,
 
 	// Render destination
+
+	/// Surface to draw to
 	surface: ManuallyDrop<Surface>,
+
+	/// Swapchain we're targeting
 	swapchain: ManuallyDrop<Swapchain>,
+
+	/// Viewport of surface
 	viewport: hal::pso::Viewport,
 	
+	/// The imageviews in our swapchain
 	imageviews: Vec<ImageView>,
+
+	/// The framebuffers of imageviews in our swapchain
 	framebuffers: Vec<Framebuffer>,
+
+	/// The frame we will draw to next
 	current_frame: usize,
+
+	/// The number of frames in our swapchain, ie max pre-rendered frames possible
 	frames_in_flight: usize,
 	
 	// Sync objects
 	// TODO: Collect these together?
+
+	/// Triggered when the image is ready to draw to
 	get_image: Vec<Semaphore>,
+
+	/// Triggered when rendering is done
 	render_complete: Vec<Semaphore>,
+
+	/// Triggered when the image is on screen
 	present_complete: Vec<Fence>,
 
 	// Pipeline
+
+	/// Our main render pass
 	renderpass: ManuallyDrop<RenderPass>,
+
+	/// The layout of our main graphics pipeline
 	pipeline_layout: ManuallyDrop<PipelineLayout>,
+
+	/// Our main graphics pipeline
 	pipeline: ManuallyDrop<GraphicsPipeline>,
 
 	// Command pool and buffers
+
+	/// The command pool used for our buffers
 	cmd_pool: ManuallyDrop<CommandPool>,
+
+	/// The buffers used to draw to our frames
 	cmd_buffers: Vec<CommandBuffer>,
+
+	/// The queue group our buffers belong to
 	queue_group: QueueGroup,
 
-	// Texture store
+	/// Texture store
 	texture_store: ManuallyDrop<TextureStore>,
 
-	// Vertex and index buffers
-	// These are both staged
+	/// (Staged) Vertex Buffer
 	pub vert_buffer: ManuallyDrop<StagedBuffer<'a, UVPoint>>,
+
+	/// (Staged) Index Buffer
 	pub index_buffer: ManuallyDrop<StagedBuffer<'a, (u16, u16, u16)>>,
 
+	/// Our camera settings
 	camera: WorkingCamera
 }
 
@@ -653,14 +692,17 @@ impl<'a> RenderingContext<'a> {
 			use hal::command::{SubpassContents, CommandBufferFlags, ClearValue, ClearColor};
 			use hal::pso::ShaderStageFlags;
 
+			// Command buffer to use
 			let buffer = &mut self.cmd_buffers[image_index];
+
+			// Colour to clear window to
 			let clear_values = [ClearValue {
 				color: ClearColor {
 					float32: [0.0, 0.0, 0.0, 1.0]
 				}
 			}];
 
-			// Commit from staging buffers
+			// Get references to our buffers
 			let (vbufs, ibuf) = {
 				let vbufref: &<back::Backend as hal::Backend>::Buffer = self.vert_buffer.get_buffer();
 
@@ -672,6 +714,7 @@ impl<'a> RenderingContext<'a> {
 
 			buffer.begin_primary(CommandBufferFlags::EMPTY);
 			{
+				// Main render pass / pipeline
 				buffer.begin_render_pass(
 					&self.renderpass,
 					&self.framebuffers[image_index],
@@ -681,6 +724,7 @@ impl<'a> RenderingContext<'a> {
 				);
 				buffer.bind_graphics_pipeline(&self.pipeline);
 
+				// VP Matrix
 				let vp = self.camera.get_matrix().as_slice();
 				let vp = std::mem::transmute::<&[f32], &[u32]>(vp);
 
@@ -690,6 +734,7 @@ impl<'a> RenderingContext<'a> {
 					0,
 					vp);
 
+				// Bind buffers
 				buffer.bind_vertex_buffers(0, vbufs);
 				buffer.bind_index_buffer(IndexBufferView {
 					buffer: ibuf,
@@ -697,6 +742,7 @@ impl<'a> RenderingContext<'a> {
 					index_type: hal::IndexType::U16
 				});
 
+				// Iterate over faces, copying them in and drawing groups that use the same texture chunk all at once.
 				let mut current_chunk = file.get_face(0).texture_idx as usize / 8;
 				let mut chunk_start = 0;
 
@@ -705,6 +751,7 @@ impl<'a> RenderingContext<'a> {
 
 				for face in faces.into_iter().map(|idx| file.get_face(*idx)) {
 					if current_chunk != face.texture_idx as usize / 8 {
+						// Last index was last of group, so draw it all.
 						let mut descriptor_sets: ArrayVec<[_; 1]> = ArrayVec::new();
 						descriptor_sets.push(self.texture_store.get_chunk_descriptor_set(current_chunk));
 
@@ -717,11 +764,13 @@ impl<'a> RenderingContext<'a> {
 
 						buffer.draw_indexed(chunk_start as u32 * 3..(curr_idx_idx as u32 * 3) + 1, 0, 0..1);
 						
+						// Next group of same-chunked faces starts here.
 						chunk_start = curr_idx_idx;
 						current_chunk = face.texture_idx as usize / 8;
 					}
 
 					if face.face_type == FaceType::Polygon || face.face_type == FaceType::Mesh {
+						// 2 layers of indirection
 						let base = face.vertices_idx.start;
 
 						for idx in face.meshverts_idx.clone().step_by(3) {
@@ -752,10 +801,12 @@ impl<'a> RenderingContext<'a> {
 					}
 
 					if curr_vert_idx >= INITIAL_VERT_SIZE.try_into().unwrap() || curr_idx_idx >= INITIAL_INDEX_SIZE.try_into().unwrap() {
+						println!("out of vertex buffer space!");
 						break;
 					}
 				}
 
+				// Draw the final group of chunks
 				let mut descriptor_sets: ArrayVec<[_; 1]> = ArrayVec::new();
 				descriptor_sets.push(self.texture_store.get_chunk_descriptor_set(current_chunk));
 				buffer.bind_graphics_descriptor_sets(
@@ -828,7 +879,7 @@ impl<'a> RenderingContext<'a> {
 
 impl<'a> core::ops::Drop for RenderingContext<'a> {
 	fn drop(&mut self) {
-		// TODO: Probably missing some destroy stuff
+		// TODO: Destroy shader modules
 		self.device.wait_idle().unwrap();
 
 		unsafe {
diff --git a/stockton-render/src/draw/texture/chunk.rs b/stockton-render/src/draw/texture/chunk.rs
index 9356274..22541a5 100644
--- a/stockton-render/src/draw/texture/chunk.rs
+++ b/stockton-render/src/draw/texture/chunk.rs
@@ -44,6 +44,8 @@ pub struct TextureChunk {
 }
 
 impl TextureChunk {
+	/// Create a new texture chunk and load in the textures specified by `range` from `file` using `resolver`
+	/// Can error if the descriptor pool is too small or if a texture isn't found
 	pub fn new<T: HasTextures, R: TextureResolver>(device: &mut Device,
 		adapter: &mut Adapter,
 		command_queue: &mut CommandQueue,
@@ -53,6 +55,7 @@ impl TextureChunk {
 		file: &T, range: Range<u32>,
 		resolver: &mut R) -> Result<TextureChunk, error::CreationError> {
 
+		// 
 		let descriptor_set = unsafe {
 			pool.allocate_set(&layout).map_err(|e| {
 				println!("{:?}", e);
@@ -71,10 +74,16 @@ impl TextureChunk {
 		for tex_idx in range {
 			debug!("Loading tex {}", local_idx + 1);
 			let tex = file.get_texture(tex_idx);
-			let img = resolver.resolve(tex);
-			store.put_texture(img, local_idx,
-				device, adapter,
-				command_queue, command_pool).unwrap();
+			if let Some(img) = resolver.resolve(tex) {
+				store.put_texture(img, local_idx,
+					device, adapter,
+					command_queue, command_pool).unwrap();
+			} else {
+				// Texture not found. For now, tear everything down.
+				store.deactivate(device);
+
+				return Err(error::CreationError::BadDataError);
+			}
 
 			local_idx += 1;
 		}
diff --git a/stockton-render/src/draw/texture/image.rs b/stockton-render/src/draw/texture/image.rs
index fec84a2..583c2d9 100644
--- a/stockton-render/src/draw/texture/image.rs
+++ b/stockton-render/src/draw/texture/image.rs
@@ -13,12 +13,15 @@
 // You should have received a copy of the GNU General Public License along
 // with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-use core::ptr::copy_nonoverlapping;
-use std::iter::once;
-use core::mem::size_of;
+use core::{
+	ptr::copy_nonoverlapping,
+	mem::{size_of, ManuallyDrop}
+};
+use std::{
+	iter::once,
+	convert::TryInto
+};
 use image::RgbaImage;
-use draw::buffer::create_buffer;
-use core::mem::ManuallyDrop;
 use hal::{
 	MemoryTypeId,
 	buffer::Usage as BufUsage,
@@ -28,18 +31,25 @@ use hal::{
 	memory::{Properties as MemProperties, Dependencies as MemDependencies, Segment},
 	prelude::*,
 };
-use std::convert::TryInto;
+
 use crate::types::*;
+use draw::buffer::create_buffer;
 
 /// The size of each pixel in an image
 const PIXEL_SIZE: usize = size_of::<image::Rgba<u8>>();
 
-
 /// Holds an image that's loaded into GPU memory and can be sampled from
 pub struct LoadedImage {
+	/// The GPU Image handle
 	image: ManuallyDrop<Image>,
+
+	/// The full view of the image
 	pub image_view: ManuallyDrop<ImageView>,
+
+	/// A sampler for the image
 	pub sampler: ManuallyDrop<Sampler>,
+
+	/// The memory backing the image
 	memory: ManuallyDrop<Memory>
 }
 
@@ -52,7 +62,7 @@ impl LoadedImage {
 		let initial_row_size = PIXEL_SIZE * (img.width() as usize);
 		let limits = adapter.physical_device.limits();
 		let row_alignment_mask = limits.optimal_buffer_copy_pitch_alignment as u32 - 1;
-
+		
 		let row_size = ((initial_row_size as u32 + row_alignment_mask) & !row_alignment_mask) as usize;
 		debug_assert!(row_size as usize >= initial_row_size);
 
diff --git a/stockton-render/src/draw/texture/loader.rs b/stockton-render/src/draw/texture/loader.rs
index b3aa3ae..483fb7d 100644
--- a/stockton-render/src/draw/texture/loader.rs
+++ b/stockton-render/src/draw/texture/loader.rs
@@ -34,7 +34,7 @@ use crate::types::*;
 /// Stores all loaded textures in GPU memory.
 /// When rendering, the descriptor sets are bound to the buffer
 /// The descriptor set layout should have the same count of textures as this does.
-/// Note that it's possible not all descriptors are actually initialised images
+/// All descriptors will be properly initialised images.
 pub struct TextureStore {
 	descriptor_pool: ManuallyDrop<DescriptorPool>,
 	pub(crate) descriptor_set_layout: ManuallyDrop<DescriptorSetLayout>,
@@ -42,11 +42,12 @@ pub struct TextureStore {
 }
 
 impl TextureStore {
+	/// Create a new texture store for the given file, loading all textures from it.
 	pub fn new<T: HasTextures>(device: &mut Device,
 		adapter: &mut Adapter,
 		command_queue: &mut CommandQueue,
 		command_pool: &mut CommandPool, file: &T) -> Result<TextureStore, error::CreationError> {
-		// Figure out how many textures in this file
+		// Figure out how many textures in this file / how many chunks needed
 		let size = file.textures_iter().count();
 		let num_chunks = {
 			let mut x = size / CHUNK_SIZE;
@@ -57,6 +58,7 @@ impl TextureStore {
 		};
 		let rounded_size = num_chunks * CHUNK_SIZE;
 
+		// Descriptor pool, where we get our sets from
 		let mut descriptor_pool = unsafe {
 			use hal::pso::{DescriptorRangeDesc, DescriptorType, DescriptorPoolCreateFlags, ImageDescriptorType};
 
@@ -83,7 +85,7 @@ impl TextureStore {
 			})?
 		};
 
-		// Descriptor set layout
+		// Layout of our descriptor sets
 		let mut descriptor_set_layout = unsafe {
 			use hal::pso::{DescriptorSetLayoutBinding, DescriptorType, ShaderStageFlags, ImageDescriptorType};
 
@@ -112,11 +114,11 @@ impl TextureStore {
 			)
 		}.map_err(|_| error::CreationError::OutOfMemoryError)?;
 
-		// Set up all our chunks
-		debug!("Starting to load textures...");
-
+		// TODO: Proper way to set up resolver
 		let mut resolver = BasicFSResolver::new(Path::new("."));
 
+		// Create texture chunks
+		debug!("Starting to load textures...");
 		let mut chunks = Vec::with_capacity(num_chunks);
 		for i in 0..num_chunks {
 			let range = {
@@ -128,7 +130,14 @@ impl TextureStore {
 			};
 			debug!("Chunk {} / {} covering {:?}", i + 1, num_chunks, range);
 
-			chunks.push(TextureChunk::new(device, adapter, command_queue, command_pool, &mut descriptor_pool, &mut descriptor_set_layout, file, range, &mut resolver)?);
+			chunks.push(
+				TextureChunk::new(
+					device, adapter, command_queue,
+					command_pool, &mut descriptor_pool,
+					&mut descriptor_set_layout, file,
+					range, &mut resolver
+				)?
+			);
 		}
 
 		debug!("All textures loaded.");
@@ -140,6 +149,7 @@ impl TextureStore {
 		})
 	}
 
+	/// Call this before dropping
 	pub fn deactivate(mut self, device: &mut Device) -> () {
 		unsafe {
 			use core::ptr::read;
@@ -155,10 +165,12 @@ impl TextureStore {
 		}
 	}
 
+	/// Get number of chunks being used
 	pub fn get_n_chunks(&self) -> usize {
 		self.chunks.len()
 	}
 
+	/// Get the descriptor set for a given chunk
 	pub fn get_chunk_descriptor_set<'a>(&'a self, idx: usize) -> &'a DescriptorSet {
 		&self.chunks[idx].descriptor_set
 	}
diff --git a/stockton-render/src/draw/texture/mod.rs b/stockton-render/src/draw/texture/mod.rs
index 896e1d2..9951eeb 100644
--- a/stockton-render/src/draw/texture/mod.rs
+++ b/stockton-render/src/draw/texture/mod.rs
@@ -13,6 +13,8 @@
 // You should have received a copy of the GNU General Public License along
 // with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+//! Everything related to loading textures into GPU memory
+
 mod resolver;
 mod image;
 mod chunk;
diff --git a/stockton-render/src/draw/texture/resolver.rs b/stockton-render/src/draw/texture/resolver.rs
index 66bce9e..5867171 100644
--- a/stockton-render/src/draw/texture/resolver.rs
+++ b/stockton-render/src/draw/texture/resolver.rs
@@ -26,9 +26,11 @@ use std::path::Path;
 
 /// An object that can be used to resolve a texture from a BSP File
 pub trait TextureResolver {
-	fn resolve(&mut self, texture: &Texture) -> RgbaImage;
+	/// Get the given texture, or None if it's corrupt/not there.
+	fn resolve(&mut self, texture: &Texture) -> Option<RgbaImage>;
 }
 
+/// A basic filesystem resolver which expects no file extension and guesses the image format
 pub struct BasicFSResolver<'a> {
 	path: &'a Path
 }
@@ -42,13 +44,17 @@ impl<'a> BasicFSResolver<'a> {
 }
 
 impl<'a> TextureResolver for BasicFSResolver<'a> {
-	fn resolve(&mut self, tex: &Texture) -> RgbaImage {
+	fn resolve(&mut self, tex: &Texture) -> Option<RgbaImage> {
 		let path = self.path.join(&tex.name);
-		println!("Loading texture from {:?}", path);
 
-		Reader::open(path).unwrap()
-			.with_guessed_format().unwrap()
-			.decode().unwrap()
-			.into_rgba()
+		if let Ok(file) = Reader::open(path) {
+			if let Ok(guessed) = file.with_guessed_format() {
+				if let Ok(decoded) = guessed.decode() {
+					return Some(decoded.into_rgba());
+				}
+			}
+		}
+
+		None
 	}
 }
\ No newline at end of file
diff --git a/stockton-render/src/error.rs b/stockton-render/src/error.rs
index b029975..5ab0822 100644
--- a/stockton-render/src/error.rs
+++ b/stockton-render/src/error.rs
@@ -20,7 +20,7 @@
 /// 	- Hardware - No suitable card usually
 /// 	- Sanity - Things that probably aren't true, likely indicating a deeper issue.
 ///				These aren't guaranteed sanity issues, but they are weird issues.
-/// 	- Runtime - Things caused by runtime conditions, usually resource constraints.
+/// 	- Runtime - Things caused by runtime conditions, usually resource constraints. Could also be caused by corrupt files
 #[derive(Debug)]
 pub enum CreationError {
 	WindowError,
@@ -41,7 +41,9 @@ pub enum CreationError {
 	BufferNoMemory,
 	
 	SwapchainError (hal::window::CreationError),
-	ImageViewError (hal::image::ViewCreationError)
+	ImageViewError (hal::image::ViewCreationError),
+
+	BadDataError
 }
 
 /// An error encountered when rendering.