Add ShapeVars helper type; use it in rendering + meshing
#190
Conversation
|
cc @waywardmonkeys for review; I can't tag you directly until you accept the invite and join the project as a collaborator 😄 |
fidget/src/render/config.rs
Outdated
| @@ -158,7 +167,16 @@ impl VoxelRenderConfig { | |||
| &self, | |||
| shape: Shape<F>, | |||
| ) -> (Vec<u32>, Vec<[u8; 3]>) { | |||
| crate::render::render3d::<F>(shape, self) | |||
| self.run_with_vars::<F>(shape, &Default::default()) | |||
There was a problem hiding this comment.
Sometimes you're using ShapeVars::default() and sometimes Default::default(). Can it always just be ShapeVars::default() for consistency and readability?
There was a problem hiding this comment.
Good point – I started using ShapeVars::new() instead, since it's slightly shorter.
CHANGELOG.md
Outdated
| functions for shapes with supplementary variables | ||
| - Change `ShapeBulkEval::eval_v` to take **single** variables (i.e. `x`, `y`, | ||
| `z` vary but each variables are contant). Add `ShapeBulkEval::eval_vs` if | ||
| `x`, `y`, `z` and variables are _all_ changing through the slices. |
There was a problem hiding this comment.
This part is just confusing to me, but maybe because it is late at night. But maybe expand a bit somewhere? (The comments on eval_vs didn't help me either.)
There was a problem hiding this comment.
Added some more comments to the eval_v and eval_vs docstrings
| /// Note that this cannot store `X`, `Y`, `Z` variables (which are passed in as | ||
| /// first-class arguments); it only stores [`Var::V`] values (identified by | ||
| /// their inner [`VarIndex`]). | ||
| pub struct ShapeVars<F>(HashMap<VarIndex, F>); |
There was a problem hiding this comment.
Is this using a HashMap an intermediate step along the way to using a Vec / smallvec?
There was a problem hiding this comment.
Perhaps! I'm inclined to leave it until someone starts noticing it gating performance, which I suspect is a long ways off.
Fixes #182 and #183
See the CHANGELOG diff for details