perf(engine): create OpenGL objects as needed instead of each frame

4 files changed, 166 insertions, 51 deletions

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<Item> Buffer<Item>
     {
         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<Item> Drop for Buffer<Item>
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<MaterialFlags>,
+    Option<Position>,
+    Option<Scale>,
+    Option<DrawFlags>,
+    Option<GlObjects>,
+);
+
 #[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<Window>)
 }
 
 fn render(
-    query: Query<
-        (
-            Mesh,
-            ShaderProgram,
-            Material,
-            Option<MaterialFlags>,
-            Option<Position>,
-            Option<Scale>,
-            Option<DrawFlags>,
-        ),
-        Not<With<NoDraw>>,
-    >,
+    query: Query<RenderableEntity, Not<With<NoDraw>>>,
     point_light_query: Query<(PointLight,)>,
     directional_lights: Query<(DirectionalLight,)>,
     camera_query: Query<(Camera,)>,
     window: Single<Window>,
     global_light: Single<GlobalLight>,
-    mut gl_objects: Local<GlObjects>,
+    mut gl_objects: Local<GlobalGlObjects>,
+    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::<Vec<_>>();
 
-    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<u64, GlShaderProgram>,
     textures: HashMap<TextureId, GlTexture>,
@@ -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<Vertex>,
-    index_info: Option<IndexInfo>,
+    index_buffer: Option<Buffer<u32>>,
+    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<Self>
+    {
+        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(
@@ -648,13 +709,6 @@ fn opengl_debug_message_cb(
 }
 
 #[derive(Debug)]
-struct IndexInfo
-{
-    _buffer: Buffer<u32>,
-    cnt: u32,
-}
-
-#[derive(Debug)]
 struct Transformation
 {
     position: Vec3<f32>,
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>
+{
+    value: ManuallyDrop<Value>,
+}
+
+impl<Value> NeverDrop<Value>
+{
+    #[must_use]
+    pub fn new(value: Value) -> Self
+    {
+        Self { value: ManuallyDrop::new(value) }
+    }
+}
+
+impl<Value> Deref for NeverDrop<Value>
+{
+    type Target = Value;
+
+    fn deref(&self) -> &Self::Target
+    {
+        &*self.value
+    }
+}
+
+impl<Value> DerefMut for NeverDrop<Value>
+{
+    fn deref_mut(&mut self) -> &mut Self::Target
+    {
+        &mut *self.value
+    }
+}