leod / posh

Type-Safe Graphics Programming with Functional Shaders
136 stars 2 forks source link

Fix overhead in `VertexBuffer<B>::set` #124

Closed leod closed 1 year ago

leod commented 1 year ago

This PR is all about enabling this diff in VertexBuffer<B>::set, which closes #75.

     pub fn set(&self, data: &[B::Gl]) {
-        // FIXME: Get rid of this conversion + allocation.
-        let data: Vec<_> = data.iter().map(AsStd140::as_std140).collect();
-
         self.raw.set(&data);
     }

Depending on the system, this can save milliseconds of time per frame when there are tens of thousands of vertices.

The solution in this PR is to make Block<Gl> implement bytemuck::Pod. It does this by providing the new types gl::{,I,U,B}Vec{2,3,4} and gl::Mat{2,3,4}. These serve as data that can be put into Block<Gl>; they are not intended to be used directly. As a result, we are also able to make the glam dependency of posh optional.

I'm okay with the solution in this PR. There is a nice symmetry between the types in gl and sl. The downside is that we now require users to do .into() on data that is put into Block<Gl> structs.

Other solutions I tried:

  1. Provide something like data: &[B::Pod] to VertexBuffer<B>::set. The problem with this is that then user-defined storage needs to do weird stuff like Vec<<MyVertex<Gl> as Block<Gl>>::Pod>.
  2. Stay with glam for Block<Gl>. This did not work out since e.g. glam::Vec4 has a 16-byte alignment, which induces padding bytes in the Block<Gl> type. However, structs with padding cannot derive(bytemuck::Pod). We could require users never to create Block<Gl> structs with padding, but that seems like an unnecessary restriction.
  3. Use mint for Block<Gl>. This did not work out since mint does not provide implementations for bytemuck::Pod. Also, we need to represent boolean vectors with u32 fields, and that might be more awkward to setup with mint (though I'm not sure on this point).