[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