In researching some optimizations for normal generation, it's easier to implement in C++ than C. Unit tests pass on my end.
Details
Diff Detail
- Repository
- rB Blender
Event Timeline
First of all, +1 to the idea of moving this file. I think the code could be much more readable with future changes to use Span, Array, etc.
And parallelization with parallel_foris usually much easier to read. But I think the approach of this patch (only doing minimal changes) is right (for now anyway).
- Part of the patch doesn't apply cleanly anymore using arc patch --nobranch --nocommit D11688
- Clang format should be applied to the file, it looks there are some formatting mistakes and too-long lines. (I really recommend setting up the editor to do this automatically!)
- Personally I would replace BLI_SMALLSTACK_DECLARE with blender::Vector with an inline buffer, like Vector<short *, 32> clnors; rather than adding a cast in the header.
- This patch could/should also change things that clang tidy cares about: NULL -> nullptr, removing typedef
Adding you as a reviewer Campbell because you've been working in this area recently and you should be aware of the patch probably. I don't think you'll need to check the changes in detail though, I'm happy to do that.
I suggest looking over the diff after you submit it, because there are some large changes here (moving and copying of functions) that aren't necessary to move this file to C++.
| source/blender/blenkernel/intern/mesh.c | ||
|---|---|---|
| 1291 ↗ | (On Diff #38732) | Why is this new code being added here? |
| source/blender/blenkernel/intern/mesh_tessellate.c | ||
| 157 ↗ | (On Diff #38732) | Why does this patch move this section from mesh_tessellate.c to mesh_evaluate.cc That doesn't seem necessary. |
Hi, please accept my apologies for the inconvenience, but something's messed up with git on my end. I've done a fresh grab and redone these changes and will put them up in a new review. Again, please accept my sincerest apologies for the inconvenience.