Page MenuHome
Differential D2150

GSoC 2016: Added PBVH based vertex and weight painting to Blender.
AbandonedPublic
Actions

Authored by Campbell Barton (campbellbarton) on Aug 11 2016, 4:52 AM.

Details

Reviewers
Bastien Montagne (mont29)
Julian Eisel (Severin)
Nathan V. Morrical (nathanvollmer)
Commits
rB0f00ede65cca: Fixed vertex selection being ignored.
rBdad3e97829e3: Merge branch 'master' into soc-2016-pbvh-painting
rB06de85843c26: Added initial PBVH creation for weight painting on toggle. PBVH is visible…
rBc3ce0d56a6fe: Got texture paint working with PBVH painting.
Summary

Previously, Blender was using OpenGL selection when painting vertices. Although convenient for painting visible vertices, this technique has several problems.

OpenGL selection:

  1. is unable to paint occluded vertices efficiently. Only vertices directly underneath the brush region can be selected.
  2. tests multiple selection rays against all vertices of a mesh. This is not only inefficient, it also fails at selecting vertices not caught by the ray.
  3. is unorganized. It is difficult to take advantage of vertex locality without searching through a large list of vertices.

The goal of this project was to rethink the way we handle vertex painting, and this commit does so by using Paint Bounding Volume Hierarchies (PBVH) and spherical based selection.

In addition to fixing the above OpenGL selection challenges, this commit adds the following features to both weight and vertex paint:

  • Mirrored painting and radial symmetry, like in sculpt mode.
  • Volume based splash prevention, which avoids painting vertices far away from the 3D brush location.
  • Normal based splash prevention, which avoids painting vertices with normals opposite the normal at the 3D brush location.
  • Blur mode now uses a nearest neighbor average.
  • Average mode, which averages the color/weight of the vertices within the brush
  • Smudge mode, which pulls the colors/weights along the direction of the brush
  • RGB^2 color blending, which gives a more accurate blend between two colors
  • multithreading support. (PBVH leaves are painted in parallel.)
  • Foreground/background color picker in vertex paint

Proposal:
https://wiki.blender.org/index.php/User:Bitinat2/GSoC_2016/Proposal

Developer Documentation:
https://wiki.blender.org/index.php/User:Bitinat2/GSoC_2016/CodeDocs

End User Documentation:
https://wiki.blender.org/index.php/User:Bitinat2/GSoC_2016/UserDocs

Working Branch Used for Development:
https://developer.blender.org/diffusion/B/browse/soc-2016-pbvh-painting/

Diff Detail

Repository
rB Blender
Branch
soc-2016-pbvh-painting
Build Status
Buildable 163
Build 163: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Bastien Montagne (mont29) added inline comments.Dec 16 2016, 9:46 AM
source/blender/blenkernel/BKE_pbvh.h
193

this is still not answered ;)

source/blender/editors/sculpt_paint/paint_image.c
1481–1486

OK, makes sense then. :)

Nathan V. Morrical (nathanvollmer) marked an inline comment as done.Dec 18 2016, 5:38 AM
Nathan V. Morrical (nathanvollmer) added inline comments.
source/blender/blenkernel/BKE_pbvh.h
193

The first part "BKE_pbvh_" is consistent with the other related methods. "_num_nodes" is the attribute being queried.

I should probably change this to "BKE_pbvh_get_num_nodes". I think that makes more sense.

source/blender/editors/sculpt_paint/paint_image.c
1451–1463

Done!

Nathan V. Morrical (nathanvollmer) updated this revision to Diff 8009.Dec 18 2016, 7:58 PM
Nathan V. Morrical (nathanvollmer) marked an inline comment as done.
  • Merge branch 'master' into soc-2016-pbvh-painting
  • Cleanup some more indentations...
  • Merge branch 'master' into soc-2016-pbvh-painting
  • Fixed multires bug.
  • Added a comment
Thomas Beck (plasmasolutions) added a subscriber: Thomas Beck (plasmasolutions).Jan 4 2017, 10:44 AM
Nathan V. Morrical (nathanvollmer) updated this revision to Diff 8106.Jan 10 2017, 6:48 AM
  • Fixed vertex selection being ignored.
Nathan V. Morrical (nathanvollmer) updated this object.Jan 10 2017, 6:57 AM
Bastien Montagne (mont29) accepted this revision.Jan 13 2017, 6:26 PM
Bastien Montagne (mont29) edited edge metadata.

OK, besides minor points noted below, I think this is good for master, even more since we can assume you’ll be around to fix potential issues in coming weeks?

Also, please do renaming as discussed above after the merge of main work (re « IMO, I think I like 'pbvh_session' the best here. sculpt_vpaint might further complicate things. We could also rename the 'SculptSession' struct to 'PBVHSession'. »).

Do not forget to add quick note about the changes in the Release Notes, with link to your wiki manual pages.

And congrats and thanks for your work! :D

PS: you should really try to configure your code editor (or IDE), those recurrent space/tab issues are a bit annoying in a review process, did you follow those instructions?

source/blender/editors/sculpt_paint/paint_vertex.c
2238–2240

mixing spaces and tabs again ;)

2274

mixing spaces and tabs again ;)

2304

should rather be MVert *v = &data->me->mvert[vertexIndex]; (same above and below of course).

2304–2306

mixing spaces and tabs again ;)

2306

Better use SELECT than literal here (defined in DNA_object_types.h at least, again same goes above and below).

2317

mixing spaces and tabs again ;)

This revision is now accepted and ready to land.Jan 13 2017, 6:26 PM
Bastien Montagne (mont29) added a comment.Jan 13 2017, 6:37 PM

Oh, and for the merge, don’t forget to use git merge --squash, we don’t want the whole history of the branch in master of course :)

Julian Eisel (Severin) added a subscriber: Julian Eisel (Severin).EditedJan 13 2017, 9:10 PM

Did some really basic testing and noticed the new weight painting behaves quite different compared to the old one. It applies the weight much less 'aggressively'. I tried to do the same stroke (same speed, same settings, same view, ...) twice, once in blender2.8 (don't have master checked out here right now) and once in master with the patch applied. This is blender2.8:

This is master with your patch:

Maybe it was planned to accept this as different behavior due to the new backend, or maybe we can do so now. Just want to make sure you're aware of this.

This is also appears to completely ignore CDDerivedMesh (didn't check this in code though).
To demonstrate this:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. The BVH still uses the normal mesh without modifiers (debug mode 14 confirms this). My blender2.8 build deforms the vertices to match the CDDM result (like the modifier show_on_cage option - guess this is Object.derivedDeform?).

Also managed to break weight painting completely:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Apply modifier
  • Enter Weight Paint mode
  • Weight painting doesn't work at all

Didn't test vertex painting but I guess it's suffering from similar issues.

source/blender/blenkernel/BKE_paint.h
207–216

It seems like SculptSession stores quite some data for single or only a few sculpt/paint modes. The attributes you added only apply to vertex and weight paint mode, others only apply to texture painting, others only to sculpting.
In this case I would really suggest using unions, to make clear what attributes are used for which modes, and that they are exclusive:

typedef struct SculptSession {
    ... /* shared vars */
    union {
        struct {
            ...
        } sculpt;
        struct {
           ...
        } texture_paint;
        ....
    } modes;
} SculptSession;

Access would make it really clear which mode a variable belongs to, e.g. sculpt.modes.weight_paint.max_weight. Maybe it's a bit too verbose, but I prefer to be explicit.

I guess this would require quite some changes, so if it's too much work I'm fine with just putting it on the ToDo. However I'd suggest adding the new attributes to nested structs at least.

source/blender/blenloader/intern/versioning_defaults.c
279–292

BLO_update_defaults_startup_blend is only executed for the default startup.blend, not for custom ones. This means the new brushes aren't added when the user launches Blender using a custom startup.blend. You should do a proper version patching in versioning_270.c.

source/blender/editors/sculpt_paint/paint_vertex.c
2319

C standards define operator precedence so that this is the same as if (i & (1 << 0)), however, I still strongly prefer to use the extra parentheses for readability (and buggy compilers ;).

Julian Eisel (Severin) requested changes to this revision.Jan 13 2017, 9:10 PM
Julian Eisel (Severin) added a reviewer: Julian Eisel (Severin).
This revision now requires changes to proceed.Jan 13 2017, 9:10 PM
Nathan V. Morrical (nathanvollmer) added a comment.Jan 14 2017, 12:15 AM
In D2150#57260, @Julian Eisel (Severin) wrote:

Did some really basic testing and noticed the new weight painting behaves quite different compared to the old one. It applies the weight much less 'aggressively'. I tried to do the same stroke (same speed, same settings, same view, ...) twice, once in blender2.8 (don't have master checked out here right now) and once in master with the patch applied. This is blender2.8:

This is master with your patch:

Maybe it was planned to accept this as different behavior due to the new backend, or maybe we can do so now. Just want to make sure you're aware of this.

This is a side effect of using spherical selection with splash prevention instead of cylindrical projection based selection. IMO this is an acceptable change, since the first isn't necessarily better than the second, and vise versa.

This is also appears to completely ignore CDDerivedMesh (didn't check this in code though).
To demonstrate this:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. The BVH still uses the normal mesh without modifiers (debug mode 14 confirms this). My blender2.8 build deforms the vertices to match the CDDM result (like the modifier show_on_cage option - guess this is Object.derivedDeform?).

Also managed to break weight painting completely:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Apply modifier
  • Enter Weight Paint mode
  • Weight painting doesn't work at all

Didn't test vertex painting but I guess it's suffering from similar issues.

I'm not entirely sure how to fix this, and could really use some help here...

First, it seems like subsurf uses a CCGDM, and not a CDDM. More specifically, ccgDM_getPBVH is called when using the subsurface modifier. (Your test doesn't show that CDDM is ignored, it shows that CCGDM is ignored, just for clarification.)

Second, this code uses the current Sculpt implementation of the PBVH. Current sculpt code determines if the modifier is multires, or subsurf. For multires, the ccgdm is used to create something called a grid_pbvh. For subsurface, sculpt just creates a PBVH using the base mesh. Should I be doing the same thing sculpt does here? Or should I create a grid_pbvh for both modifiers?

Third, I've been heavily relying on bvh->mloop, bvh->verts, and bvh->mpoly to do my painting. (see do_wpaint_brush_draw_task_cb_ex) These fields aren't set for a grid_pbvh, so I've been forced into only using the standard mesh pbvh to avoid a crash. If you're suggesting I use a grid approach instead, how can I access the verts, loops, and faces of a ccgDM mesh using a grid_PBVH/ccdgm? How can I map those derived vertices, loops, and faces to the base vertices, loops and faces? I've read the derived mesh docs found here: https://wiki.blender.org/index.php/Dev:Source/Modeling/DerivedMesh, and it isn't really all that helpful... Should I be reading something else?

Erick Tukuniata (erickblender) awarded a token.Jan 14 2017, 11:23 AM
Erick Tukuniata (erickblender) added a subscriber: Erick Tukuniata (erickblender).
Bastien Montagne (mont29) added a comment.Jan 14 2017, 11:57 AM

Yes, difference in brush 'visual strength' is expected indeed, especially with big brushes (this shall be maybe a bit more clearly advertised in doc/release notes, btw).

This is also appears to completely ignore CDDerivedMesh (didn't check this in code though).
To demonstrate this:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. The BVH still uses the normal mesh without modifiers (debug mode 14 confirms this). My blender2.8 build deforms the vertices to match the CDDM result (like the modifier show_on_cage option - guess this is Object.derivedDeform?).

First, it seems like subsurf uses a CCGDM, and not a CDDM. More specifically, ccgDM_getPBVH is called when using the subsurface modifier. (Your test doesn't show that CDDM is ignored, it shows that CCGDM is ignored, just for clarification.)

Second, this code uses the current Sculpt implementation of the PBVH. Current sculpt code determines if the modifier is multires, or subsurf. For multires, the ccgdm is used to create something called a grid_pbvh. For subsurface, sculpt just creates a PBVH using the base mesh. Should I be doing the same thing sculpt does here? Or should I create a grid_pbvh for both modifiers?

Yes, unlinke with sculpt, think in w/vpaint case you definitively want to use grid_pbvh in both cases.

Third, I've been heavily relying on bvh->mloop, bvh->verts, and bvh->mpoly to do my painting. (see do_wpaint_brush_draw_task_cb_ex) These fields aren't set for a grid_pbvh, so I've been forced into only using the standard mesh pbvh to avoid a crash. If you're suggesting I use a grid approach instead, how can I access the verts, loops, and faces of a ccgDM mesh using a grid_PBVH/ccdgm? How can I map those derived vertices, loops, and faces to the base vertices, loops and faces? I've read the derived mesh docs found here: https://wiki.blender.org/index.php/Dev:Source/Modeling/DerivedMesh, and it isn't really all that helpful... Should I be reading something else?

ccgdm.getPolyDataArray(dm, CD_ORIGINDEX) (and same for verts and edges) allow you to get a mapping from new faces/edges/verts to original ones, by indices (poly maps all subsurf faces to original ones, while only real original subsurfed edges and vertices are mapped, new generated ones get ORIGINDEX_NONE mapping index value). Note that the CCGDM faces are passed to BKE_pbvh_build_grids(), so think you can rebuild needed data from there? @Sergey Sharybin (sergey) shall have a better insight of ccgdm internals though, hence maybe better advices.

That way you should be able at least to paint original vertices on there subdivided location. Maybe an even smarter interpolation is possible too (painting each original vertex 'contributing' to a subdivided one proportionally, based on the 'grid distance' to them?), but that’s likely more involved and probably not in the scope of this patch.

Also managed to break weight painting completely:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Apply modifier
  • Enter Weight Paint mode
  • Weight painting doesn't work at all

Didn't test vertex painting but I guess it's suffering from similar issues.

I'm not entirely sure how to fix this, and could really use some help here...

Switching to Edit mode and back to Object mode (or WPaint) after applying the modifier is enough to get things working again, so I would not expect this to be a big issue, likely some tiny glitch (like bad flag left after the 'apply modifier' or some pointer not cleared or something like that).

source/blender/blenkernel/BKE_paint.h
207–216

This is nice idea, but needs to be done in separate commit imho, will be way too noisy and is not really related to patch itself.

Verbosity can be mitigated a bit, we can avoid long full names here (e.g. sculpt.mds.vpaint.max_weight, think we can accept contractions of 'in-between' unions/structs in that case).

Nathan V. Morrical (nathanvollmer) added a comment.Jan 15 2017, 9:33 AM
In D2150#57293, @Bastien Montagne (mont29) wrote:

Yes, difference in brush 'visual strength' is expected indeed, especially with big brushes (this shall be maybe a bit more clearly advertised in doc/release notes, btw).

I'll get that updated.

ccgdm.getPolyDataArray(dm, CD_ORIGINDEX) (and same for verts and edges) allow you to get a mapping from new faces/edges/verts to original ones, by indices (poly maps all subsurf faces to original ones, while only real original subsurfed edges and vertices are mapped, new generated ones get ORIGINDEX_NONE mapping index value). Note that the CCGDM faces are passed to BKE_pbvh_build_grids(), so think you can rebuild needed data from there? @Sergey Sharybin (sergey) shall have a better insight of ccgdm internals though, hence maybe better advices.

That clears things up a lot. Do all derived meshes behave this way? When two or more sub-surf/multires modifiers are on the modifier stack at the same time, would the higher derived mesh map to the base mesh directly, or would they map to the lower derived meshes recursively?

That way you should be able at least to paint original vertices on there subdivided location. Maybe an even smarter interpolation is possible too (painting each original vertex 'contributing' to a subdivided one proportionally, based on the 'grid distance' to them?), but that’s likely more involved and probably not in the scope of this patch.

I think the trickiest part here is determining what base vertex to interpolate the color onto. I could do a weighted average of all surrounding ORIGINDEX_NONE vertices onto the base vertex, although I'm not sure how many neighbors away from the original vertex I should traverse.

Also managed to break weight painting completely:

  • Default startup.blend
  • Add SubSurf modifier to default cube
  • Apply modifier
  • Enter Weight Paint mode
  • Weight painting doesn't work at all

Didn't test vertex painting but I guess it's suffering from similar issues.

I'm not entirely sure how to fix this, and could really use some help here...

Switching to Edit mode and back to Object mode (or WPaint) after applying the modifier is enough to get things working again, so I would not expect this to be a big issue, likely some tiny glitch (like bad flag left after the 'apply modifier' or some pointer not cleared or something like that).

I might have broke this when I forced the PBVH to use the base mesh for vwpaint instead of the ccgdm PBVH. I'll check it out.

Philip Witte (filwit) added a subscriber: Philip Witte (filwit).Jan 25 2017, 1:22 AM
Thomas Beck (plasmasolutions) awarded a token.Jan 31 2017, 8:22 PM
Erick Tukuniata (erickblender) added a comment.Feb 1 2017, 11:36 AM

Yo Nat,

I just pull your branch today a gave it some tests and i noticed that you solved the problem with masking but you introduced an other bug. So after sculpting in dyntopo, then starting to vertex paint if the face selection masking option is not activated we cannot paint on the whole mesh, we can only paint on some part.

Thanks for your work :)

Erick Tukuniata (erickblender) added subscribers: Campbell Barton (campbellbarton), Tom Musgrove (letterrip).EditedMar 21 2017, 9:08 AM

Yo @Nathan V. Morrical (nathanvollmer) are you still with us? Blender 2.79 is around the corner, please do something. @Bastien Montagne (mont29), @Campbell Barton (campbellbarton), @Tom Musgrove (letterrip) this branch is so close to be ready please guys do something, here we have the speed that Blender is missing in painting mode. Do something to make us happy :)

Campbell Barton (campbellbarton) added inline comments.Apr 5 2017, 8:23 AM
source/blender/blenkernel/BKE_paint.h
212

Why not make this unsigned int (*total_color)[3] avoids index * 3 all over.

source/blender/blenkernel/intern/paint.c
659–699

Note that we have MEM_SAFE_FREE macro that handles this common pattern.

source/blender/editors/sculpt_paint/paint_vertex.c
3168

Why use long? int should be fine here?

Nathan V. Morrical (nathanvollmer) updated this revision to Diff 8545.Apr 5 2017, 12:21 PM
Nathan V. Morrical (nathanvollmer) edited edge metadata.
Nathan V. Morrical (nathanvollmer) marked an inline comment as done.
  • fixed masking bug, & pbvh freeing bug. Still trying to figure out grids...
  • added union in sculpt session for modes. Sculpt fields could use some tidying, but that might be outside the scope of this branch. Swapped out some free patterns with MEM_SAFE_FREE. Swapped a long out for an int, since there's no size difference.
  • Merge branch 'master' into soc-2016-pbvh-painting
Erick Tukuniata (erickblender) added a comment.Apr 5 2017, 6:48 PM

Compiled the latest changes here are some feedback :

1.Thanks for fixing the mask bug, but there a small problem, when painting the brush is bleeding over the masked area, here is in the master

and here our branch
in some extend the bleeding is good but it looks like it is too much, i can cope with it.

  1. One of Severin review

" - Default startup.blend

  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. "

Now the branch is ignoring weight painting all together. Your 18/12/2016 implementation was handling this case well.

Again thank you, hope to see this in master :)

Nathan V. Morrical (nathanvollmer) added a comment.Apr 5 2017, 8:38 PM
In D2150#60951, @Erick Tukuniata (erickblender) wrote:

Compiled the latest changes here are some feedback :

1.Thanks for fixing the mask bug, but there a small problem, when painting the brush is bleeding over the masked area, here is in the master

and here our branch
in some extend the bleeding is good but it looks like it is too much, i can cope with it.

Probably would be best if this behavior matched the original. I think I can figure this out before the end of the day.

  1. One of Severin review

" - Default startup.blend

  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. "

Now the branch is ignoring weight painting all together. Your 18/12/2016 implementation was handling this case well.

The 18/12/16 implementation didn't use the subdivided vertex locations to paint with, which made painting--say, a cube--hard to do, since the subdivided vertices drift a lot. Currently working with mont on this, and I made some decent headway last night.

Thanks for testing!

Campbell Barton (campbellbarton) added a comment.Apr 10 2017, 7:53 AM

While reviewing this patch noticed:

  • Original mesh vertex coords are used when they shouldn't be.
  • Smudge in weight paint mode ignores vertex selection.
source/blender/makesrna/intern/rna_brush.c
98

What's the difference between smear/smudge/blur?

Campbell Barton (campbellbarton) updated this revision to Diff 8583.Apr 10 2017, 7:54 AM
  • Merge branch 'master' into soc-2016-pbvh-painting
  • De-duplicate vpaint symmetry draw functions
  • Cleanup: remove redundant nested header
  • Cleanup: minor syntax/unused var
  • Cleanup: add OB_MODE_ALL_SCULPT
  • Cleanup: move inline ViewContext out of vpaint structs
  • Cleanup: remove redundant headers-in-headers
  • Cleanup: headers
  • Cleanup: move WPaintData, VPaintData, WeightPaintInfo back into paint_vertex.c
  • Cleanup: variable names, const correctness, formatting
  • Cleanup: move BKE_pbvh_vertex_iter_end into same scope as BKE_pbvh_vertex_iter_begin
  • Cleanup: Avoid redundant/accidental struct copying
Campbell Barton (campbellbarton) updated this revision to Diff 8588.Apr 10 2017, 11:53 AM
  • Use 'unsigned int[3]' array for total color
Nathan V. Morrical (nathanvollmer) updated this revision to Diff 8610.Apr 12 2017, 12:21 PM
Nathan V. Morrical (nathanvollmer) marked an inline comment as done.
  • Added support for multires for both vertex and weight paint. Fixed a masking bug.
  • Merge branch 'soc-2016-pbvh-painting' of git.blender.org:blender into soc-2016-pbvh-painting
  • Merge branch 'master' into soc-2016-pbvh-painting
Nathan V. Morrical (nathanvollmer) marked 17 inline comments as done.Apr 12 2017, 12:47 PM
In D2150#60951, @Erick Tukuniata (erickblender) wrote:

Compiled the latest changes here are some feedback :

1.Thanks for fixing the mask bug, but there a small problem, when painting the brush is bleeding over the masked area, here is in the master

and here our branch
in some extend the bleeding is good but it looks like it is too much, i can cope with it.

Probably would be best if this behavior matched the original. I think I can figure this out before the end of the day.

  1. One of Severin review

" - Default startup.blend

  • Add SubSurf modifier to default cube
  • Enter Weight Paint mode
  • Try drawing. You have to draw over the un-deformed vertices of the cube to affect the weight. "

Now the branch is ignoring weight painting all together. Your 18/12/2016 implementation was handling this case well.

The 18/12/16 implementation didn't use the subdivided vertex locations to paint with, which made painting--say, a cube--hard to do, since the subdivided vertices drift a lot. Currently working with mont on this, and I made some decent headway last night.

Thanks for testing!

source/blender/makesrna/intern/rna_brush.c
98

I show the difference in my user documentation: https://wiki.blender.org/index.php/User:Bitinat2/GSoC_2016/UserDocs

Nathan V. Morrical (nathanvollmer) updated this revision to Diff 8611.Apr 12 2017, 12:48 PM
Nathan V. Morrical (nathanvollmer) marked an inline comment as done.
  • surrounded symmetry axis checks with parenthesis just in case
  • Severin suggested I move new brushes to versioning_270.c
Nathan V. Morrical (nathanvollmer) marked 3 inline comments as done.Apr 12 2017, 12:57 PM
Campbell Barton (campbellbarton) updated this revision to Diff 8616.Apr 12 2017, 6:34 PM
  • Fix accidental nested loop in vertex paint smudge
  • Direction may be zero length even when not first-evaluated
  • Remove unnecessary loop over polygon-loops
  • Don't create vertex groups when reading weights
  • Move view_dot > 0.0 checks to first possible location
  • Make vertex index assignment shorter
  • Move SculptBrushTest initialization out of loop
  • Fix crash when use_group_restrict is disabled
  • Merge branch 'master' into soc-2016-pbvh-painting
Campbell Barton (campbellbarton) updated this revision to Diff 8618.Apr 12 2017, 6:56 PM
  • Rename Smudge to Smear since we already call this smear in image editor
Campbell Barton (campbellbarton) added inline comments.Apr 12 2017, 8:55 PM
source/blender/editors/sculpt_paint/paint_vertex.c
2280–2287

This is causing problems - in a simple case once the weights are 1.0, you can't paint them to a lower weight. (They get locked at 1.0)

Double checked and this wasn't caused by my recent changed here.

Erick Tukuniata (erickblender) added a comment.EditedApr 12 2017, 9:31 PM

Feedback with the latest update 1 bug found:

  • Ctrl+z undoes brush settings, it breaks consistency with master.

Almost forgot to say thanks for the bug fixing and mask behavior is the same as in master now :)

Jacob Merrill (blueprintrandom) awarded a token.Apr 16 2017, 9:57 PM
Jacob Merrill (blueprintrandom) added a subscriber: Jacob Merrill (blueprintrandom).Apr 16 2017, 9:59 PM
This comment was removed by Campbell Barton (campbellbarton).
Bastien Montagne (mont29) added inline comments.Apr 26 2017, 9:41 AM
source/blender/editors/sculpt_paint/paint_vertex.c
2280–2287

This whole block cannot work indeed, it’s merely assuming we are only doing "positive" mix painting…

Thing is, I'm not sure we can have simple solution here, I tried just accumulating applied 'alpha' (see P470), but this will not work correctly either of course, the operations are commutative here.

Imho the only way to do it would be to store both original weights and accumulated alpha, clamping the later to brush strength, and re-apply paint operation with accumulated alpha over original weight every time. Which is basically what wpaint_blend() does in current master, for what I can say - though in a slightly less smart way, since it recomputes 'max' value every time...

Will try to implement my idea if nobody has better one ;)

Howard Trickey (howardt) added a subscriber: Howard Trickey (howardt).May 27 2017, 9:06 PM
This revision now requires changes to proceed.May 27 2017, 9:06 PM
Howard Trickey (howardt) added a comment.May 27 2017, 9:08 PM

What is the status of this patch? Is it likely to get applied in the 2.8 branch any time soon? Since a GSoC 2017 student that I am mentoring is also doing Vertex Paint improvements, it would be a pity not to be able to build on top of this.

Bastien Montagne (mont29) added a comment.May 27 2017, 10:33 PM

Eeek, have too much things piling up these days…

Basically, idea is to merge this in master, but after we branch out 2.79 release. And we'd rather have 2.79 branched out before GSoC starts (at least that was plan), will raise topic on meeting this sunday.

This patch is nearly ready for master, only remains a few details to be tackled.

Darshan Kadu (darshan) mentioned this in rB29645a583842: applied patch D2150.id8618.diff and fixed versioning_270.c.Jun 3 2017, 10:24 AM
Nathan V. Morrical (nathanvollmer) added inline comments.Jun 5 2017, 4:33 PM
source/blender/editors/sculpt_paint/paint_vertex.c
2280–2287

I believe "max_weight" was intended to be the endpoint of an interpolation between the current color and the final color. Say the current color is .5, and after painting becomes .25. The max_weight would be .25, and after several iterations, the weight would ideally converge to that value. A concurrent color value could be .00, rising instead of falling to .25.

We could do something like

/* max weight starts out uninitialized as a negative number, since the final interpolation val cannot be negative. */
if (ss->modes.vwpaint.max_weight[v_index] < 0) {
    /* If uninitialized, set to be the final value in interpolation */
    ss->modes.vwpaint.max_weight[v_index] = brush_strength + weight_curr;
    CLAMP(ss->modes.vwpaint.max_weight[v_index] ,0.0f, 1.0f);
}

/* We're done if the current weight matches the final value (Might be a better way to do this...) */
if (fabs(weight_curr - ss->modes.vwpaint.max_weight[v_index]) > .0001) {
    continue;
}

/* Prevent change in alpha from overshooting the final interpolation value. 
    Note, max_weight could be less than weight_curr, so we take abs to find magnitude.  */
CLAMP(final_alpha, 0.0, fabs(ss->modes.vwpaint.max_weight[v_index] - weight_curr));

Thoughts?

Can't seem to lock weights at 1.0 with current code... I must be doing something wrong...

Howard Trickey (howardt) mentioned this in T52909: Vertex Paint GSoC starting point.Sep 26 2017, 3:15 PM
Campbell Barton (campbellbarton) updated this revision to Diff 9314.Sep 27 2017, 1:37 PM
  • Merge branch 'master' into soc-2016-pbvh-painting
  • Use BLI_math rounding function, replace sqrtl w/ sqrtf
  • Cleanup: use shorter uchar, uint type names
  • Merge branch 'master' into soc-2016-pbvh-painting
  • Resolve bug w/ non-airbrush weight-paint blending
  • Cleanup: split vert/weight paint into separate members
  • Avoid redundant vert->loop->vert topology lookup
  • Cleanup: move accumulation structs out of sculpt-session
  • Cleanup: unused function
Campbell Barton (campbellbarton) mentioned this in rB4f616c93f7cb: Vertex/Weight Paint: Use PBVH for painting.Sep 27 2017, 5:35 PM
Campbell Barton (campbellbarton) commandeered this revision.Sep 27 2017, 5:37 PM
Campbell Barton (campbellbarton) added a reviewer: Nathan V. Morrical (nathanvollmer).

Committed rB4f616c93f7cb8c8c8e038bc0c949c931242971c2

Erick Tukuniata (erickblender) added a comment.Sep 27 2017, 8:37 PM

Thanks Cambo for this commit. You are such a good man :)
Note that the day that you will have a hug out of nowhere, it will be me.

Nathan V. Morrical (nathanvollmer) edited edge metadata.Sep 27 2017, 9:46 PM

@Campbell Barton (campbellbarton) Thanks for the help!

source/blender/blenkernel/intern/DerivedMesh.c
2623

I don't think this flag makes much sense. What semantic is being added by including "ALL_" in "OB_MODE_ALL_SCULPT"? Why was this change necessary?

Campbell Barton (campbellbarton) added a comment.Sep 28 2017, 5:54 AM

yw eric :)

@Nathan V. Morrical (nathanvollmer) OB_MODE_ALL_SCULPT is to match all modes that use sculpt data. Without this we need to duplicate that information wherever mode checks are done.

At the moment it's not really needed, but later on these kinds of checks can get out of sync.

Campbell Barton (campbellbarton) abandoned this revision.Oct 4 2017, 2:10 PM

This is in master now, closing.

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
45 lines
source/
blender/
blenkernel/
13 lines
2 lines
intern/
2 lines
5 lines
22 lines
8 lines
5 lines
blenloader/
intern/
24 lines
editors/
sculpt_paint/
19 lines
105 lines
1 line
1611 lines
243 lines
214 lines
1 line
makesdna/
4 lines
3 lines
makesrna/
intern/
2 lines
9 lines

Diff 7430

View Options

release/scripts/startup/bl_ui/space_view3d_toolbar.py

Loading...
View Options

source/blender/blenkernel/BKE_paint.h

Loading...
View Options

source/blender/blenkernel/BKE_pbvh.h

Loading...
View Options

source/blender/blenkernel/intern/DerivedMesh.c

Loading...
View Options

source/blender/blenkernel/intern/cdderivedmesh.c

Loading...
View Options

source/blender/blenkernel/intern/paint.c

Loading...
View Options

source/blender/blenkernel/intern/pbvh.c

Loading...
View Options

source/blender/blenkernel/intern/subsurf_ccg.c

Loading...
View Options

source/blender/blenloader/intern/versioning_defaults.c

Loading...
View Options

source/blender/editors/sculpt_paint/paint_image.c

Loading...
View Options

source/blender/editors/sculpt_paint/paint_intern.h

Loading...
View Options

source/blender/editors/sculpt_paint/paint_ops.c

Loading...
View Options

source/blender/editors/sculpt_paint/paint_vertex.c

Loading...
View Options

source/blender/editors/sculpt_paint/sculpt.c

Loading...
View Options

source/blender/editors/sculpt_paint/sculpt_intern.h

Loading...
View Options

source/blender/editors/sculpt_paint/sculpt_undo.c

Loading...
View Options

source/blender/makesdna/DNA_brush_types.h

Loading...
View Options

source/blender/makesdna/DNA_scene_types.h

Loading...
View Options

source/blender/makesrna/intern/rna_brush.c

Loading...
View Options

source/blender/makesrna/intern/rna_sculpt_paint.c

Loading...