[Bf-codereview] Sculpt masking (issue 6135048)

NicholasBishop at gmail.com NicholasBishop at gmail.com
Thu May 10 23:26:53 CEST 2012


Thanks for reviewing :)

> 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?

On my ad hoc testing I didn't find performance to be noticeably
different, but on the other hand I don't have a good automated way of
verifying this. No performance drop reported by testers either.

> 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.

The functions are BLI_INLINE (except for the CCGKey initialization
functions), so that should be OK.

> 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?

Yep, figured it was an OK tradeoff to make masks always present since
they'll probably be a frequently used part of the sculpt workflow.


> 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.

Changes made, and masking committed as revisions 46509 - 46532.

Thanks,
-Nicholas

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


More information about the Bf-codereview mailing list