[Bf-codereview] Partial Visibility (Sculpt) (issue 5695043)

[brechtvanlommel at gmail.com]

Code looks solid to me, couldn't spot bugs, just some code style and
naming comments. Making this hiding work at the grid level is really
quite complex .. but it's implemented now.


http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE_multires.h
File source/blender/blenkernel/BKE_multires.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE_multires.h#newcode47
source/blender/blenkernel/BKE_multires.h:47: void
multires_mark_as_modified(struct Object *ob, enum
MultiresModifiedFlags);
Second parameter here has no name.

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE_multires.h#newcode81
source/blender/blenkernel/BKE_multires.h:81: void
multires_create_grid_hidden(struct GridHidden *gh, unsigned level);
Just a minor note, I'd stick to int instead of using unsigned, since
that's already used pretty much everywhere else, and prefer not to have
them mixed.

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE_subsurf.h
File source/blender/blenkernel/BKE_subsurf.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE_subsurf.h#newcode65
source/blender/blenkernel/BKE_subsurf.h:65: unsigned
ccg_gridsize(unsigned level);
Why not put this in CCGSubSurf.h and use ccgSubSurf_ prefix? Also, this
one uses unsigned while the other uses int.

http://codereview.appspot.com/5695043/diff/4001/source/blender/editors/sculpt_paint/paint_hide.c
File source/blender/editors/sculpt_paint/paint_hide.c (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/editors/sculpt_paint/paint_hide.c#newcode407
source/blender/editors/sculpt_paint/paint_hide.c:407: ot->name =
"Partial Visibility";
This is a bit of a strange name for an operator. It's effectively just
show/hide for sculpt mode right, maybe just name it like that?

http://codereview.appspot.com/5695043/diff/4001/source/blender/makesdna/DNA_meshdata_types.h
File source/blender/makesdna/DNA_meshdata_types.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/makesdna/DNA_meshdata_types.h#newcode225
source/blender/makesdna/DNA_meshdata_types.h:225: typedef struct
GridHidden {
I was expecting this data to be in the same layer as MDisps? With this
being a separate layer I fear that e.g. the levels might get out of
sync, and you sort of have to keep track that when one is modified the
other is too.

But I guess it's needed because this might get used later of things for
cases where you might not have MDisps?

http://codereview.appspot.com/5695043/