Release 0.6.2

This is a hotfix for some dodgy vertex offset code. It still needs improving, but this fixes the root cause of the issue.

Author: 17cupsofcoffee

CHANGELOG.md

@@ -6,6 +6,13 @@ From 0.4.0 onwards, all breaking changes will be explicitly labelled, to make it
 
 This project adheres to Semantic Versioning.
 
+## [0.6.2] - 2021-03-15
+
+### Fixed
+
+* `VertexBuffer::set_data` was mistakenly measuring its offset in individual floats, rather than vertices. This was inconsistent with `IndexBuffer`, and could potentially lead to corrupted data.
+    * I do not believe this was a memory safety issue, as all writes were still valid and aligned - they were just in the wrong place!
+
 ## [0.6.1] - 2021-03-15
 
 ### Added
@@ -737,7 +744,8 @@ for. This can be useful when implementing more complex animation behaviors. ([@V
 
 * Initial release!
 
-[Upcoming]: https://github.com/17cupsofcoffee/tetra/compare/0.6.1..HEAD
+[Upcoming]: https://github.com/17cupsofcoffee/tetra/compare/0.6.2..HEAD
+[0.6.2]: https://github.com/17cupsofcoffee/tetra/compare/0.6.1..0.6.2
 [0.6.1]: https://github.com/17cupsofcoffee/tetra/compare/0.6.0..0.6.1
 [0.6.0]: https://github.com/17cupsofcoffee/tetra/compare/0.5.8..0.6.0
 [0.5.8]: https://github.com/17cupsofcoffee/tetra/compare/0.5.7..0.5.8

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "tetra"
 description = "A simple 2D game framework written in Rust"
-version = "0.6.1"
+version = "0.6.2"
 edition = "2018"
 authors = ["Joe Clay <[email protected]>"]
 license = "MIT"

src/graphics.rs

@@ -86,7 +86,7 @@ impl GraphicsContext {
         window_width: i32,
         window_height: i32,
     ) -> Result<GraphicsContext> {
-        let vertex_buffer = device.new_vertex_buffer(MAX_VERTICES, 8, BufferUsage::Dynamic)?;
+        let vertex_buffer = device.new_vertex_buffer(MAX_VERTICES, BufferUsage::Dynamic)?;
         let index_buffer = device.new_index_buffer(MAX_INDICES, BufferUsage::Static)?;
 
         let indices: Vec<u32> = INDEX_ARRAY

src/graphics/mesh.rs

@@ -139,10 +139,9 @@ impl VertexBuffer {
         vertices: &[Vertex],
         usage: BufferUsage,
     ) -> Result<VertexBuffer> {
-        let buffer = ctx.device.new_vertex_buffer(vertices.len(), 8, usage)?;
+        let buffer = ctx.device.new_vertex_buffer(vertices.len(), usage)?;
 
-        ctx.device
-            .set_vertex_buffer_data(&buffer, bytemuck::cast_slice(vertices), 0);
+        ctx.device.set_vertex_buffer_data(&buffer, vertices, 0);
 
         Ok(VertexBuffer {
             handle: Rc::new(buffer),
@@ -156,7 +155,7 @@ impl VertexBuffer {
     /// Panics if the offset is out of bounds.
     pub fn set_data(&self, ctx: &mut Context, vertices: &[Vertex], offset: usize) {
         ctx.device
-            .set_vertex_buffer_data(&self.handle, bytemuck::cast_slice(vertices), offset);
+            .set_vertex_buffer_data(&self.handle, vertices, offset);
     }
 
     /// Creates a mesh using this buffer.

src/platform/device_gl.rs

@@ -1,19 +1,13 @@
 use std::cell::Cell;
-use std::mem;
 use std::rc::Rc;
 
 use glow::{Context as GlowContext, HasContext, PixelUnpackData};
 
 use crate::error::{Result, TetraError};
-use crate::graphics::mesh::{BufferUsage, VertexWinding};
+use crate::graphics::mesh::{BufferUsage, Vertex, VertexWinding};
 use crate::graphics::{BlendAlphaMode, BlendMode, FilterMode};
 use crate::math::{Mat2, Mat3, Mat4, Vec2, Vec3, Vec4};
 
-/// Utility function for calculating offsets/sizes.
-fn size<T>(elements: usize) -> i32 {
-    (elements * mem::size_of::<T>()) as i32
-}
-
 type BufferId = <GlowContext as HasContext>::Buffer;
 type ProgramId = <GlowContext as HasContext>::Program;
 type TextureId = <GlowContext as HasContext>::Texture;
@@ -149,7 +143,6 @@ impl GraphicsDevice {
     pub fn new_vertex_buffer(
         &mut self,
         count: usize,
-        stride: usize,
         usage: BufferUsage,
     ) -> Result<RawVertexBuffer> {
         unsafe {
@@ -163,16 +156,13 @@ impl GraphicsDevice {
                 state: Rc::clone(&self.state),
                 id,
                 count,
-                stride,
             };
 
             self.bind_vertex_buffer(Some(&buffer));
 
-            self.state.gl.buffer_data_size(
-                glow::ARRAY_BUFFER,
-                size::<f32>(buffer.size()),
-                usage.into(),
-            );
+            self.state
+                .gl
+                .buffer_data_size(glow::ARRAY_BUFFER, buffer.size() as i32, usage.into());
 
             Ok(buffer)
         }
@@ -181,13 +171,13 @@ impl GraphicsDevice {
     pub fn set_vertex_buffer_data(
         &mut self,
         buffer: &RawVertexBuffer,
-        data: &[f32],
+        data: &[Vertex],
         offset: usize,
     ) {
         self.bind_vertex_buffer(Some(buffer));
 
         assert!(
-            data.len() + offset <= buffer.size(),
+            data.len() + offset <= buffer.count(),
             "tried to write out of bounds buffer data"
         );
 
@@ -196,7 +186,7 @@ impl GraphicsDevice {
 
             self.state.gl.buffer_sub_data_u8_slice(
                 glow::ARRAY_BUFFER,
-                size::<f32>(offset),
+                (buffer.stride() * offset) as i32,
                 bytemuck::cast_slice(data),
             );
         }
@@ -220,7 +210,7 @@ impl GraphicsDevice {
 
             self.state.gl.buffer_data_size(
                 glow::ELEMENT_ARRAY_BUFFER,
-                size::<u32>(count),
+                buffer.size() as i32,
                 usage.into(),
             );
 
@@ -241,7 +231,7 @@ impl GraphicsDevice {
 
             self.state.gl.buffer_sub_data_u8_slice(
                 glow::ELEMENT_ARRAY_BUFFER,
-                size::<u32>(offset),
+                (buffer.stride() * offset) as i32,
                 bytemuck::cast_slice(data),
             );
         }
@@ -785,7 +775,7 @@ impl GraphicsDevice {
                 glow::TRIANGLES,
                 count as i32,
                 glow::UNSIGNED_INT,
-                size::<u32>(offset),
+                (index_buffer.stride() * offset) as i32,
             );
         }
     }
@@ -806,7 +796,7 @@ impl GraphicsDevice {
                             2,
                             glow::FLOAT,
                             false,
-                            size::<f32>(b.stride),
+                            b.stride() as i32,
                             0,
                         );
 
@@ -815,17 +805,17 @@ impl GraphicsDevice {
                             2,
                             glow::FLOAT,
                             false,
-                            size::<f32>(b.stride),
-                            size::<f32>(2),
+                            b.stride() as i32,
+                            8,
                         );
 
                         self.state.gl.vertex_attrib_pointer_f32(
                             2,
                             4,
                             glow::FLOAT,
                             false,
-                            size::<f32>(b.stride),
-                            size::<f32>(4),
+                            b.stride() as i32,
+                            16,
                         );
 
                         self.state.gl.enable_vertex_attrib_array(0);
@@ -1056,20 +1046,22 @@ pub struct RawVertexBuffer {
     id: BufferId,
 
     count: usize,
-    stride: usize,
 }
 
 impl RawVertexBuffer {
+    /// The number of vertices in the buffer.
     pub fn count(&self) -> usize {
         self.count
     }
 
+    // The size of each vertex, in bytes.
     pub fn stride(&self) -> usize {
-        self.stride
+        std::mem::size_of::<Vertex>()
     }
 
+    /// The size of the buffer, in bytes.
     pub fn size(&self) -> usize {
-        self.count * self.stride
+        self.count * self.stride()
     }
 }
 
@@ -1096,9 +1088,20 @@ pub struct RawIndexBuffer {
 }
 
 impl RawIndexBuffer {
+    /// The number of indices in the buffer.
     pub fn count(&self) -> usize {
         self.count
     }
+
+    /// The size of each index, in bytes.
+    pub fn stride(&self) -> usize {
+        std::mem::size_of::<u32>()
+    }
+
+    /// The size of the buffer, in bytes.
+    pub fn size(&self) -> usize {
+        self.count * self.stride()
+    }
 }
 
 impl Drop for RawIndexBuffer {