Add smallvec and use it to try and reduce indirection; Add indirection to avoid exploding my stack in debug; Add more tracing

Author: manpat

Cargo.toml

@@ -27,6 +27,8 @@ glutin = "0.32"
 glutin-winit = "0.5"
 raw-window-handle = "0.6"
 
+smallvec = "1.13.2"
+
 common = { git = "https://github.com/manpat/common-rs.git", version = ">=0.5.0", features=["interop", "serde"] }
 toy = { git = "https://github.com/manpat/toy-rs.git" }
 

toybox-gfx/Cargo.toml

@@ -9,6 +9,7 @@ anyhow.workspace = true
 common.workspace = true
 log.workspace = true
 tracing.workspace = true
+smallvec.workspace = true
 
 toybox-host.workspace = true
 toybox-vfs.workspace = true

toybox-gfx/src/bindings.rs

@@ -56,8 +56,8 @@ pub struct ImageBindDesc {
 #[derive(Debug, Default)]
 pub struct BindingDescription {
 	// TODO(pat.m): store unresolved named targets separately to resolved/explicit targets to simplify usage 
-	pub buffer_bindings: Vec<BufferBindDesc>,
-	pub image_bindings: Vec<ImageBindDesc>,
+	pub buffer_bindings: SmallVec<[BufferBindDesc; 4]>,
+	pub image_bindings: SmallVec<[ImageBindDesc; 4]>,
 
 	pub framebuffer: Option<FramebufferArgument>,
 }
@@ -171,79 +171,92 @@ impl BindingDescription {
 	pub fn bind(&self, core: &mut Core, resource_manager: &mut ResourceManager) {
 		let mut barrier_tracker = core.barrier_tracker();
 
-		for BufferBindDesc{target, source} in self.buffer_bindings.iter() {
-			let BufferArgument::Name{name, range} = *source
-				else { panic!("Unresolved buffer bind source") };
+		{
+			let _span = tracing::info_span!("bind buffers").entered();
 
-			let Some((index, indexed_target)) = target.to_raw_index().zip(target.to_indexed_buffer_target())
-				else { panic!("Unresolved buffer target") };
+			for BufferBindDesc{target, source} in self.buffer_bindings.iter() {
+				let BufferArgument::Name{name, range} = *source
+					else { panic!("Unresolved buffer bind source") };
 
-			match indexed_target {
-				// TODO(pat.m): this is pessimistic - but we need shader reflection to guarantee that an ssbo is bound
-				// as readonly.
-				IndexedBufferTarget::ShaderStorage => barrier_tracker.write_buffer(name, gl::SHADER_STORAGE_BARRIER_BIT),
-				IndexedBufferTarget::Uniform => barrier_tracker.read_buffer(name, gl::UNIFORM_BARRIER_BIT),
-			}
+				let Some((index, indexed_target)) = target.to_raw_index().zip(target.to_indexed_buffer_target())
+					else { panic!("Unresolved buffer target") };
+
+				match indexed_target {
+					// TODO(pat.m): this is pessimistic - but we need shader reflection to guarantee that an ssbo is bound
+					// as readonly.
+					IndexedBufferTarget::ShaderStorage => barrier_tracker.write_buffer(name, gl::SHADER_STORAGE_BARRIER_BIT),
+					IndexedBufferTarget::Uniform => barrier_tracker.read_buffer(name, gl::UNIFORM_BARRIER_BIT),
+				}
 
-			core.bind_indexed_buffer(indexed_target, index, name, range);
+				core.bind_indexed_buffer(indexed_target, index, name, range);
+			}
 		}
 
-		for ImageBindDesc{target, source, sampler} in self.image_bindings.iter() {
-			let ImageArgument::Name(image_name) = *source
-				else { panic!("Unresolved image bind source") };
+		{
+			let _span = tracing::info_span!("bind images").entered();
 
-			match *target {
-				ImageBindTarget::Sampled(unit) => {
-					barrier_tracker.read_image(image_name, gl::TEXTURE_FETCH_BARRIER_BIT);
+			for ImageBindDesc{target, source, sampler} in self.image_bindings.iter() {
+				let ImageArgument::Name(image_name) = *source
+					else { panic!("Unresolved image bind source") };
 
-					// TODO(pat.m): use default instead of panicking
-					let sampler_name = match sampler.expect("Sampled bind target missing sampler") {
-						SamplerArgument::Name(name) => name,
-						SamplerArgument::Common(sampler) => resource_manager.get_common_sampler(sampler),
-					};
+				match *target {
+					ImageBindTarget::Sampled(unit) => {
+						barrier_tracker.read_image(image_name, gl::TEXTURE_FETCH_BARRIER_BIT);
 
-					core.bind_sampler(unit, sampler_name);
-					core.bind_sampled_image(unit, image_name);
-				}
+						// TODO(pat.m): use default instead of panicking
+						let sampler_name = match sampler.expect("Sampled bind target missing sampler") {
+							SamplerArgument::Name(name) => name,
+							SamplerArgument::Common(sampler) => resource_manager.get_common_sampler(sampler),
+						};
 
-				ImageBindTarget::ReadonlyImage(unit) => {
-					barrier_tracker.read_image(image_name, gl::SHADER_IMAGE_ACCESS_BARRIER_BIT);
-					core.bind_image(unit, image_name);
-				}
+						core.bind_sampler(unit, sampler_name);
+						core.bind_sampled_image(unit, image_name);
+					}
 
-				ImageBindTarget::ReadWriteImage(unit) => {
-					barrier_tracker.write_image(image_name, gl::SHADER_IMAGE_ACCESS_BARRIER_BIT);
-					core.bind_image_rw(unit, image_name);
-				}
+					ImageBindTarget::ReadonlyImage(unit) => {
+						barrier_tracker.read_image(image_name, gl::SHADER_IMAGE_ACCESS_BARRIER_BIT);
+						core.bind_image(unit, image_name);
+					}
+
+					ImageBindTarget::ReadWriteImage(unit) => {
+						barrier_tracker.write_image(image_name, gl::SHADER_IMAGE_ACCESS_BARRIER_BIT);
+						core.bind_image_rw(unit, image_name);
+					}
 
-				_ => panic!("Unresolved image bind target"),
+					_ => panic!("Unresolved image bind target"),
+				}
 			}
 		}
 
-		// TODO(pat.m): the following should only be done for the Draw command really.
-		// it doesn't make sense to bind or emit barriers for e.g., Compute.
-
-		// Framebuffer should _ALWAYS_ be defined by this point.
-		// The global BindingDescription should specify Default
-		let framebuffer = self.framebuffer.as_ref()
-			.expect("Unresolved framebuffer")
-			.resolve_name(core, resource_manager);
-
-		if let Some(framebuffer_name) = framebuffer {
-			let framebuffer_info = core.get_framebuffer_info(framebuffer_name);
-			for attachment_image in framebuffer_info.attachments.values() {
-				// NOTE: only a read barrier since framebuffer writes are implicitly synchronised with later draw calls.
-				// We only need to make sure that if an image is _modified_ that a barrier is inserted before rendering to it.
-				barrier_tracker.read_image(*attachment_image, gl::FRAMEBUFFER_BARRIER_BIT);
+
+		{
+			let _span = tracing::info_span!("bind framebuffer").entered();
+
+			// TODO(pat.m): the following should only be done for the Draw command really.
+			// it doesn't make sense to bind or emit barriers for e.g., Compute.
+
+			// Framebuffer should _ALWAYS_ be defined by this point.
+			// The global BindingDescription should specify Default
+			let framebuffer = self.framebuffer.as_ref()
+				.expect("Unresolved framebuffer")
+				.resolve_name(core, resource_manager);
+
+			if let Some(framebuffer_name) = framebuffer {
+				let framebuffer_info = core.get_framebuffer_info(framebuffer_name);
+				for attachment_image in framebuffer_info.attachments.values() {
+					// NOTE: only a read barrier since framebuffer writes are implicitly synchronised with later draw calls.
+					// We only need to make sure that if an image is _modified_ that a barrier is inserted before rendering to it.
+					barrier_tracker.read_image(*attachment_image, gl::FRAMEBUFFER_BARRIER_BIT);
+				}
+
+				let framebuffer_size = core.get_framebuffer_size(framebuffer_name);
+				core.set_viewport(framebuffer_size);
+			} else {
+				core.set_viewport(core.backbuffer_size());
 			}
 
-			let framebuffer_size = core.get_framebuffer_size(framebuffer_name);
-			core.set_viewport(framebuffer_size);
-		} else {
-			core.set_viewport(core.backbuffer_size());
+			core.bind_framebuffer(framebuffer);
 		}
-
-		core.bind_framebuffer(framebuffer);
 	}
 }
 

toybox-gfx/src/command_group.rs

@@ -1,3 +1,4 @@
+use crate::prelude::*;
 use crate::bindings::*;
 use crate::command::{Command, compute, draw};
 use crate::resource_manager::{ShaderHandle, arguments::*};
@@ -33,7 +34,7 @@ pub enum FrameStage {
 pub struct CommandGroup {
 	pub stage: FrameStage,
 
-	pub commands: Vec<Command>,
+	pub commands: SmallVec<[Command; 16]>,
 
 	pub shared_bindings: BindingDescription,
 }
@@ -42,7 +43,7 @@ impl CommandGroup {
 	pub(crate) fn new(stage: FrameStage) -> CommandGroup {
 		CommandGroup {
 			stage,
-			commands: Vec::new(),
+			commands: SmallVec::new(),
 			shared_bindings: BindingDescription::new(),
 		}
 	}

toybox-gfx/src/core/image.rs

@@ -33,7 +33,7 @@ pub enum ImageType {
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub(in crate::core) struct ImageInfoInternal {
 	info: ImageInfo,
-	views: Vec<(ImageFormat, u32)>,
+	views: SmallVec<[(ImageFormat, u32); 2]>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]

toybox-gfx/src/lib.rs

@@ -24,6 +24,8 @@ pub mod prelude {
 	pub use crate::host::gl;
 	pub use crate::{ResourceName, BufferRangeExt};
 
+	pub use smallvec::SmallVec;
+
 	pub use toybox_vfs as vfs;
 	pub use common::math::*;
 }
@@ -49,7 +51,7 @@ impl System {
 
 impl System {
 	#[instrument(skip_all, name="gfxsys System::new")]
-	pub fn new(mut core: core::Core) -> anyhow::Result<System> {
+	pub fn new(mut core: core::Core) -> anyhow::Result<Box<System>> {
 		core.register_debug_hook();
 
 		let resource_manager = resource_manager::ResourceManager::new(&mut core)?;
@@ -63,11 +65,11 @@ impl System {
 			core.gl.Enable(gl::FRAMEBUFFER_SRGB);
 		}
 
-		Ok(System {
+		Ok(Box::new(System {
 			core,
 			resource_manager,
 			frame_encoder,
-		})
+		}))
 	}
 
 	pub fn resize(&mut self, new_size: common::Vec2i) {

toybox-host/src/lib.rs

@@ -44,7 +44,7 @@ pub type GlContext = glutin::context::PossiblyCurrentContext;
 
 
 pub fn start<F, H>(settings: Settings<'_>, start_hostee: F) -> anyhow::Result<()>
-	where F: FnOnce(&Host) -> anyhow::Result<H>
+	where F: FnOnce(&Host) -> anyhow::Result<Box<H>>
 		, H: HostedApp + 'static
 {
 	let _span = tracing::info_span!("host start").entered();
@@ -78,8 +78,8 @@ pub fn start<F, H>(settings: Settings<'_>, start_hostee: F) -> anyhow::Result<()
 		_span,
 	};
 
-	let mut app_host = ApplicationHost::Bootstrap(bootstrap_state, start_hostee);
-
+	// NOTE: Box to avoid issues with stack sizes if hostee is too large.
+	let mut app_host = Box::new(ApplicationHost::Bootstrap(bootstrap_state, start_hostee));
 	event_loop.run_app(&mut app_host)?;
 
 	Ok(())
@@ -92,11 +92,11 @@ enum ApplicationHost<F, H> {
 	#[default]
 	Empty,
 	Bootstrap(BootstrapState, F),
-	Hosting(Host, H),
+	Hosting(Host, Box<H>),
 }
 
 impl<F, H> ApplicationHandler for ApplicationHost<F, H>
-	where F: FnOnce(&Host) -> anyhow::Result<H>
+	where F: FnOnce(&Host) -> anyhow::Result<Box<H>>
 		, H: HostedApp + 'static
 {
 	fn resumed(&mut self, event_loop: &ActiveEventLoop) {

toybox/src/context.rs

@@ -1,7 +1,7 @@
 use crate::prelude::*;
 
 pub struct Context {
-	pub gfx: gfx::System,
+	pub gfx: Box<gfx::System>,
 	pub audio: audio::System,
 	pub input: input::System,
 	pub egui: egui::Context,

toybox/src/lib.rs

@@ -80,11 +80,11 @@ pub fn run_with_settings<F, A>(settings: host::Settings<'_>, start_app: F) -> an
 
 		let app = tracing::info_span!("app start").in_scope(|| start_app(&mut context))?;
 
-		Ok(HostedApp {
+		Ok(Box::new(HostedApp {
 			context,
 			debug_menu_state: debug::MenuState::default(),
 			app,
-		})
+		}))
 	})
 }