From c4686c2992417545e7a05a6a40ee9f1a8bbf3b96 Mon Sep 17 00:00:00 2001 From: HampusM Date: Fri, 16 Aug 2024 22:10:00 +0200 Subject: perf(engine): create OpenGL objects as needed instead of each frame --- TODO.md | 2 +- engine/src/opengl/buffer.rs | 13 ++++ engine/src/opengl/vertex_array.rs | 11 +++ engine/src/renderer/opengl.rs | 156 +++++++++++++++++++++++++------------- engine/src/util.rs | 37 +++++++++ 5 files changed, 167 insertions(+), 52 deletions(-) diff --git a/TODO.md b/TODO.md index 0a3b8d9..ad62857 100644 --- a/TODO.md +++ b/TODO.md @@ -8,7 +8,7 @@ - [x] Add support for removing components from entities at runtime in ECS framework - [x] Add support for entity relationships in ECS framework - [x] Add component events (removal & addition) in ECS framework -- [ ] Make renderer not create new buffers each frame +- [x] Make renderer not create new buffers each frame - [ ] Multiple rendering passes - [ ] Make projection changable - [ ] Remove position field from Camera component diff --git a/engine/src/opengl/buffer.rs b/engine/src/opengl/buffer.rs index 68a75fb..2be7f12 100644 --- a/engine/src/opengl/buffer.rs +++ b/engine/src/opengl/buffer.rs @@ -39,6 +39,19 @@ impl Buffer { self.buf } + + /// Does a weak clone of this buffer. The buffer itself is NOT copied in any way this + /// function only copies the internal buffer ID. + /// + /// # Safety + /// The returned `Buffer` must not be dropped if another `Buffer` referencing the + /// same buffer ID is used later or if a [`VertexArray`] is used later. + /// + /// [`VertexArray`]: crate::opengl::vertex_array::VertexArray + pub unsafe fn clone_weak(&self) -> Self + { + Self { buf: self.buf, _pd: PhantomData } + } } impl Drop for Buffer diff --git a/engine/src/opengl/vertex_array.rs b/engine/src/opengl/vertex_array.rs index e1e1a15..da5d91e 100644 --- a/engine/src/opengl/vertex_array.rs +++ b/engine/src/opengl/vertex_array.rs @@ -125,6 +125,17 @@ impl VertexArray { unsafe { gl::BindVertexArray(self.array) } } + + /// Does a weak clone of this vertex array. The vertex array itself is NOT copied in + /// any way this function only copies the internal vertex array ID. + /// + /// # Safety + /// The returned `VertexArray` must not be dropped if another `VertexArray` + /// referencing the same vertex array ID is used later. + pub unsafe fn clone_unsafe(&self) -> Self + { + Self { array: self.array } + } } impl Drop for VertexArray diff --git a/engine/src/renderer/opengl.rs b/engine/src/renderer/opengl.rs index 911f0df..5c7435c 100644 --- a/engine/src/renderer/opengl.rs +++ b/engine/src/renderer/opengl.rs @@ -6,6 +6,7 @@ use std::ops::Deref; use std::process::abort; use cstr::cstr; +use ecs::actions::Actions; use ecs::component::local::Local; use ecs::query::options::{Not, With}; use ecs::sole::Single; @@ -44,10 +45,22 @@ use crate::projection::{new_perspective_matrix, Projection}; use crate::shader::Program as ShaderProgram; use crate::texture::{Id as TextureId, Texture}; use crate::transform::{Position, Scale}; +use crate::util::NeverDrop; use crate::vector::{Vec2, Vec3}; use crate::vertex::{AttributeComponentType, Vertex}; use crate::window::Window; +type RenderableEntity = ( + Mesh, + ShaderProgram, + Material, + Option, + Option, + Option, + Option, + Option, +); + #[derive(Debug, Default)] pub struct Extension {} @@ -59,7 +72,9 @@ impl ecs::extension::Extension for Extension collector.add_system( PresentEvent, - render.into_system().initialize((GlObjects::default(),)), + render + .into_system() + .initialize((GlobalGlObjects::default(),)), ); } } @@ -97,24 +112,14 @@ fn initialize(window: Single) } fn render( - query: Query< - ( - Mesh, - ShaderProgram, - Material, - Option, - Option, - Option, - Option, - ), - Not>, - >, + query: Query>>, point_light_query: Query<(PointLight,)>, directional_lights: Query<(DirectionalLight,)>, camera_query: Query<(Camera,)>, window: Single, global_light: Single, - mut gl_objects: Local, + mut gl_objects: Local, + mut actions: Actions, ) { let Some(camera) = camera_query.iter().find_map(|(camera,)| { @@ -136,15 +141,26 @@ fn render( let directional_lights = directional_lights.iter().collect::>(); - let GlObjects { + let GlobalGlObjects { shader_programs: gl_shader_programs, textures: gl_textures, } = &mut *gl_objects; clear_buffers(BufferClearMask::COLOR | BufferClearMask::DEPTH); - for (mesh, shader_program, material, material_flags, position, scale, draw_flags) in - &query + for ( + entity_index, + ( + mesh, + shader_program, + material, + material_flags, + position, + scale, + draw_flags, + gl_objects, + ), + ) in query.iter().enumerate() { let material_flags = material_flags .map(|material_flags| material_flags.clone()) @@ -154,6 +170,25 @@ fn render( .entry(shader_program.u64_hash()) .or_insert_with(|| create_gl_shader_program(&shader_program).unwrap()); + let new_gl_objects; + + let gl_objects = match gl_objects.as_deref() { + Some(gl_objects) => gl_objects, + None => { + // TODO: Account for when meshes are changed + let gl_objects = GlObjects::new(&*mesh); + + new_gl_objects = Some(gl_objects.clone()); + + actions.add_components( + query.entity_uid(entity_index).unwrap(), + (gl_objects,), + ); + + &*new_gl_objects.unwrap() + } + }; + apply_transformation_matrices( Transformation { position: position.map(|pos| *pos).unwrap_or_default().position, @@ -202,7 +237,7 @@ fn render( ) } - draw_mesh(&mesh); + draw_mesh(&gl_objects); if draw_flags.is_some() { let default_polygon_mode_config = PolygonModeConfig::default(); @@ -216,7 +251,7 @@ fn render( } #[derive(Debug, Default, Component)] -struct GlObjects +struct GlobalGlObjects { shader_programs: HashMap, textures: HashMap, @@ -244,16 +279,12 @@ fn initialize_debug() set_debug_message_control(None, None, None, &[], MessageIdsAction::Disable); } -fn draw_mesh(mesh: &Mesh) +fn draw_mesh(gl_objects: &GlObjects) { - // TODO: Creating a new vertex buffer each draw is really dumb and slow this - // should be rethinked - let renderable = Renderable::new(mesh.vertices(), mesh.indices()); + gl_objects.vertex_arr.bind(); - renderable.vertex_arr.bind(); - - if let Some(index_info) = &renderable.index_info { - VertexArray::draw_elements(PrimitiveKind::Triangles, 0, index_info.cnt); + if gl_objects.index_buffer.is_some() { + VertexArray::draw_elements(PrimitiveKind::Triangles, 0, gl_objects.index_cnt); } else { VertexArray::draw_arrays(PrimitiveKind::Triangles, 0, 3); } @@ -302,26 +333,36 @@ fn create_gl_shader_program( Ok(gl_shader_program) } -#[derive(Debug)] -struct Renderable +#[derive(Debug, Component)] +struct GlObjects { - vertex_arr: VertexArray, - /// Vertex and index buffer has to live as long as the vertex array _vertex_buffer: Buffer, - index_info: Option, + index_buffer: Option>, + index_cnt: u32, + + vertex_arr: VertexArray, } -impl Renderable +impl GlObjects { - fn new(vertices: &[Vertex], indices: Option<&[u32]>) -> Self + #[cfg_attr(feature = "debug", tracing::instrument(skip_all))] + fn new(mesh: &Mesh) -> Self { + #[cfg(feature = "debug")] + tracing::trace!( + "Creating vertex array, vertex buffer{}", + if mesh.indices().is_some() { + " and index buffer" + } else { + "" + } + ); + let mut vertex_arr = VertexArray::new(); let mut vertex_buffer = Buffer::new(); - let mut index_info = None; - - vertex_buffer.store(vertices, BufferUsage::Static); + vertex_buffer.store(mesh.vertices(), BufferUsage::Static); vertex_arr.bind_vertex_buffer(0, &vertex_buffer, 0); @@ -344,25 +385,45 @@ impl Renderable offset += attrib.component_size * attrib.component_cnt as u32; } - if let Some(indices) = indices { + if let Some(indices) = mesh.indices() { let mut index_buffer = Buffer::new(); index_buffer.store(indices, BufferUsage::Static); vertex_arr.bind_element_buffer(&index_buffer); - index_info = Some(IndexInfo { - _buffer: index_buffer, - cnt: indices.len().try_into().unwrap(), - }); + return Self { + _vertex_buffer: vertex_buffer, + index_buffer: Some(index_buffer), + index_cnt: indices.len().try_into().unwrap(), + vertex_arr, + }; } Self { - vertex_arr, _vertex_buffer: vertex_buffer, - index_info, + index_buffer: None, + index_cnt: 0, + vertex_arr, } } + + pub fn clone(&self) -> NeverDrop + { + NeverDrop::new(Self { + // SAFETY: The vertex buffer will never become dropped (NeverDrop ensures it) + _vertex_buffer: unsafe { self._vertex_buffer.clone_weak() }, + index_buffer: self + .index_buffer + .as_ref() + // SAFETY: The index buffer will never become dropped (NeverDrop ensures + // it) + .map(|index_buffer| unsafe { index_buffer.clone_weak() }), + index_cnt: self.index_cnt, + // SAFETY: The vertex array will never become dropped (NeverDrop ensures it) + vertex_arr: unsafe { self.vertex_arr.clone_unsafe() }, + }) + } } fn apply_transformation_matrices( @@ -647,13 +708,6 @@ fn opengl_debug_message_cb( }; } -#[derive(Debug)] -struct IndexInfo -{ - _buffer: Buffer, - cnt: u32, -} - #[derive(Debug)] struct Transformation { diff --git a/engine/src/util.rs b/engine/src/util.rs index 532bdee..8e93982 100644 --- a/engine/src/util.rs +++ b/engine/src/util.rs @@ -9,6 +9,9 @@ macro_rules! try_option { }; } +use std::mem::ManuallyDrop; +use std::ops::{Deref, DerefMut}; + pub(crate) use try_option; macro_rules! or { @@ -93,3 +96,37 @@ macro_rules! builder { } pub(crate) use builder; + +/// Wrapper that ensures the contained value will never be dropped. +#[derive(Debug)] +pub struct NeverDrop +{ + value: ManuallyDrop, +} + +impl NeverDrop +{ + #[must_use] + pub fn new(value: Value) -> Self + { + Self { value: ManuallyDrop::new(value) } + } +} + +impl Deref for NeverDrop +{ + type Target = Value; + + fn deref(&self) -> &Self::Target + { + &*self.value + } +} + +impl DerefMut for NeverDrop +{ + fn deref_mut(&mut self) -> &mut Self::Target + { + &mut *self.value + } +} -- cgit v1.2.3-18-g5258