Add the ability to interpret the same buffer with different Voxel traits #4
Conversation
examples-crate/Cargo.toml
Outdated
| name = "reinterpret" | ||
| path = "reinterpret/main.rs" |
There was a problem hiding this comment.
Could use a more descriptive name.
There was a problem hiding this comment.
What do you propose?
There was a problem hiding this comment.
Hmmmm... maybe multiple_views. Or mirror the name of the function and call it with_cast.
src/lib.rs
Outdated
| fn is_empty(&self) -> bool { | ||
| self.0.is_empty() | ||
| } | ||
| fn is_opaque(&self) -> bool { | ||
| self.0.is_opaque() | ||
| } |
There was a problem hiding this comment.
I don't know much about inlining, so I'll leave that decision to someone else.
There was a problem hiding this comment.
General guidance is here: https://nnethercote.github.io/perf-book/inlining.html
My policy has been to #[inline] very short functions that might get called in a tight loop. It's just a suggestion to the compiler, and the compiler might decide it's a bad idea to inline a particular function, and I'm OK trusting the compiler's decision in most cases.
There was a problem hiding this comment.
So I think inlining these trait methods makes sense. Pushing and popping a stack frame in either of these functions would be bad for performance.
src/simple.rs
Outdated
| @@ -17,6 +18,31 @@ pub fn visible_block_faces<T, S>( | |||
| ) where | |||
| T: Voxel, | |||
| S: Shape<u32, 3>, | |||
| { | |||
| visible_block_faces_with_cast::<_, IdentityVoxel<T>, _>( | |||
There was a problem hiding this comment.
I'm not sure "cast" is the proper term for this. Maybe _with_voxel_view or something.
|
I would merge the first patch, but I don't think we need a new example (with bevy and all) for this. Feel free to add a doc test for the new function though. |
bonsairobo
force-pushed
the
main
branch
2 times, most recently
from
4ff9dcb
to
427c085
Compare
dcz-self
force-pushed
the
reinterpret
branch
from
6f8b7e2
to
73b51a3
Compare
|
Rebased. I skipped the doctest, however. |
dcz-self
force-pushed
the
reinterpret
branch
from
73b51a3
to
08c3980
Compare
Fixes #3 .
I don't really like it a lot, with the references' lifetimes messing up the newtypes. At least in practice when voxels mostly impllement Copy, this only means a lifetimed From is needed (see example).