Can confirm; actually unable to undo the mask at all in current main
Currently in a debug build for current main, this fails on the following assert after attempting to undo the mask.
`BLI_assert failed: source/blender/editors/sculpt_paint/sculpt_undo.cc:920,…
Oh, my mistake, I completely looked over the fact this wasn't inside the factor loop
I'm fine with that, but I think we should also do a cleanup commit of the other usages of it in the recent sculpt files for consistency's sake and so we don't rehash this conversation in the future
@mod_moder Is the reaction because of the mix of MutableSpan + const? I agree it reads weird, but for better or worse it's fairly common in the newly refactored code
Haven't done validation of the brushes just yet - do you have any performance changes measured for this brush?
Why do we pass Object in here if it's unused? Do you plan on making use of this in a future refactor?
nit: Move the function docstring to the header
Should this and orig_normals be part of TLS or otherwise initialized outside of the parallel loop?
We normally do BLI_NOINLINE before static void right?
Is it worth passing in a normalized version to this function from somewhere to avoid normalizing on every vert? It seems like a bit of a waste to explicitly make it non-normalized on line 228 only to revert it here.
Thanks @DanielBystedt for the detailed feedback! I'll close this PR then since this new behavior isn't desirable.