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

[NicholasBishop at gmail.com]

Thanks for reviewing, added a new patch with naming and signedness
issues fixed.

> 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.
CCGSubSurf.h is in blenkernel/intern rather than a normal BKE_*.h
header, figured I shouldn't use it for this function since it's called
elsewhere (editors, blenloader, gpu.)

It's really only needed because I wanted to store 'level' rather than
'gridsize' in GridHidden data (felt like I should've done that for
MDisps too, since it's easy to derive gridsize and similar data from
level, but not the other way around.) Could store gridsize too though?

> 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?
Changed to PAINT_OT_hide_show. Looks like the other hide/show uses have
separate operators, one for hide and one for reveal. Should we do that
here?

> 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?
Interesting point, I don't have a strong reason for keeping them
separate. Reckon I just figured it made sense to keep different kinds of
data separate, but not sure. I'll have another look at this.

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