I'd like to remove face_set_
too, but I think sculpt_face_set_edit
-> edit_init
here and sculpt_face_set_init_exec
Some further thoughts after the last round of review.
Minor nit - the comment below makes me think that we should compare distances[i]
to FLT_MAX
instead
Is adding a clamp to [0.0, 1.0] here worthwhile?
Something that I thought of based on your comment about the initialize vs modify comment on these spans - what do you think about moving the initialization of the r_factors
Vector
here and the r_distances
Vector
later to somewhere outside of these functions?
Thanks for the review!
For the
r_
prefixes, I don't actually think they apply here. I've always viewed those as a way to express that the values were initialized or otherwise…
Just my 2 cents - I think them being redundant is fine, to me the asserts at this level provide two benefits:
- Gives more contextual information on this line with the sizes being unequal than…
Just a first pass on the code so far for high level / stylistic stuff, haven't gone super in depth into either of the brush algorithms to compare to the existing code.
translations
-> r_translations
here and elsewhere in the file
positions
-> r_positions
here and elsewhere in the file
factors
-> r_factors
, this and elsewhere in the other methods.
Minor nit, I'm personally not a fan of utils
as a suffix, it tends to lead to the file / namespace / class being a dumping ground of loosely related concepts instead of something more cohesive. That being said I don't think I have a good name for this right now - maybe something to do with factor
and displacement
, since that seems to be the main type of output of these functions?
Since factors
comes up a fair bit in this file, I think it's worth defining what it is somewhere in the main header.
How about we add a new namespace here for brushes?