[Bf-codereview] Sculpt masking (issue 6135048)

brechtvanlommel at gmail.com brechtvanlommel at gmail.com
Thu May 10 13:42:58 CEST 2012


Well, I can't find much wrong with this. The changes are clear and make
sense. It's incredibly useful to have the whole commit history for
review, it makes things much easier to understand.

Turning DMGridData into something more flexible is fine with me. I
wonder a bit about performance with those CCG_elem_* accessor functions
used everywhere, is there any performance regression sculpting high
resolution meshes?

If there is a significant performance regression, I guess inlining these
functions and put CCGKey on the stack so the compiler can keep the used
members in registers might help.

Also it seems the mask is now created everytime sculpting is used,
increasing memory usage a bit. I guess it doesn't take up too much
memory since it's just one float?

LGTM to go into trunk now.


http://codereview.appspot.com/6135048/diff/20003/source/blender/blenkernel/BKE_ccg.h
File source/blender/blenkernel/BKE_ccg.h (right):

http://codereview.appspot.com/6135048/diff/20003/source/blender/blenkernel/BKE_ccg.h#newcode36
source/blender/blenkernel/BKE_ccg.h:36: #include <stdio.h>
Just always include it, no need to check for NDEBUG.

http://codereview.appspot.com/6135048/diff/20003/source/blender/gpu/intern/gpu_buffers.c
File source/blender/gpu/intern/gpu_buffers.c (right):

http://codereview.appspot.com/6135048/diff/20003/source/blender/gpu/intern/gpu_buffers.c#newcode1273
source/blender/gpu/intern/gpu_buffers.c:1273: drivers (Gallium/Radeon)
*/
I'd remove the XXX, for me that indicates something broken. It makes
sense that it helps to align struct members.

http://codereview.appspot.com/6135048/


More information about the Bf-codereview mailing list