Partial Visibility (Sculpt)

This patch re-introduces the partial visibility feature for sculpt mode.

If you use git, here is a tar of the individual patches:
http://wiki.blender.org/uploads/a/a9/Partial-visibility-03.tar.bz2

Documentation:
http://wiki.blender.org/index.php/User:Nicholasbishop/PartialVisibility

[nicholasbishop, 2012-02-24 22:34:37]

Update to apply cleanly against latest trunk

[nicholasbishop, 2012-02-24 22:35:33]

On 2012/02/24 22:34:37, nicholasbishop wrote:
> Update to apply cleanly against latest trunk

Updated git patches:
http://wiki.blender.org/uploads/0/0b/Partial-visibility-01.tar.bz2

[brechtvl, 2012-02-29 13:08:05]

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...
File source/blender/blenkernel/BKE_multires.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE...
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...
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...
File source/blender/blenkernel/BKE_subsurf.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/blenkernel/BKE...
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...
File source/blender/editors/sculpt_paint/paint_hide.c (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/editors/sculpt...
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_m...
File source/blender/makesdna/DNA_meshdata_types.h (right):

http://codereview.appspot.com/5695043/diff/4001/source/blender/makesdna/DNA_m...
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?

[nicholasbishop, 2012-02-29 21:28:52]

Update from review

[nicholasbishop, 2012-02-29 21:39:08]

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.

[nicholasbishop, 2012-03-01 01:01:19]

Fix more signedness differences

[nicholasbishop, 2012-03-14 06:34:28]

Committed as r44864 to r44872.

Committed code combines MDisps and GridHidden structs as suggested.