This can smooth any float, float3, or Color4f attribute on
the point domain, and can use either a constant interpolation
factor or another float attribute for the interpolation factor.
Probably need to set default attribute in/out to "position".
Details
Details
- Reviewers
Hans Goudey (HooglyBoogly)
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- smooth (branched from master)
- Build Status
Buildable 13996 Build 13996: arc lint + arc unit
Event Timeline
Comment Actions
Unfortunately the patch requires updating for current master after recent refactoring to the attribute API (rB5cf6f570c65d), so I didn't get to test it.
I agree with Jacques about DefaultMixer, though I'm also not sure if it will have to be tweaked to use it here.
Generally this seems quite useful, even as a nice base for the functionality on other attribute domains in the future : )
| source/blender/makesrna/intern/rna_nodetree.c | ||
|---|---|---|
| 9630 | Extra semicolon | |
| source/blender/nodes/geometry/nodes/node_geo_mesh_smooth.cc | ||
| 23–24 | Pretty sure these includes are unused? Same with MEM_guardedalloc.h? | |
| 48 | Should also have uiLayoutSetPropSep(layout, true); | |
| 134 | It seems like it shouldn't be necessary to declare a temporary array here. | |
| 149 | This could be separated from the main loop. In other words, instead of checking if iter == 0 on every iteration of the hot loop, split this out to a separate loop, maybe a separate function even. | |
| 193 | Don't forget bke::geometry_set_realize_instances, that's what allows the node to work on object instances from the object info or collection info nodes. | |
| 210 | Asking about the input attribute type of the result attribute is a warning that something a bit fishy is going on. I've recently added attribute_get_meta_data for this purpose. | |
Comment Actions
I made these changes and will shortly upload a new diff.
| source/blender/nodes/geometry/nodes/node_geo_mesh_smooth.cc | ||
|---|---|---|
| 134 | I think I need a temporary array, because the output is an interpolation between the original value and the smoothed value. | |
| 149 | This code went away when I switched to using a mixer. It means we sum the edge weights each iteration rather than only once (which is what this code accomplished) but seems a fair tradeoff for code reuse. | |
| 210 | I admit to being a little lost here, basically copying other nodes, like the attribute compare and attribute math ones. If you have an example of how to use attribute_get_meta_data or can just tell me how to change this code, I'd appreciate it. | |
| 212 | This code is gone now. | |
Comment Actions
Works well, just some small comments and a note about other domains besides "point".
I'm running into issues smoothing a vertex color. I wonder if this node needs more explicit handling for other attribute domains.
Something fishy is going on anyway-- BLI_assert(input.size() == totvert); maybe means that the result domain should always be the point domain. (Actually I didn't test with a debug build, that assert probably would have triggered.
| source/blender/nodes/geometry/nodes/node_geo_mesh_smooth.cc | ||
|---|---|---|
| 34 | I think it's more consistent to have the result last, maybe this input can be first after geometry? | |
| 70 | The mesh argument can be a reference | |
| 81 | This is nitpicky, but I much prefer range-based for loops in C++ code, for (const int iter : IndexRange(iterations)) { | |
| 94–97 | I guess it's important to save the result in every iteration rather than saving it once with the factor at the end? | |
| 134 | Right, indeed, my bad. | |
| 210 | In the current version of the node this looks fine. | |
Comment Actions
Hi, just an idea, but what if we combine iterations and factor into one continuous Strength slider? The end user probably only cares about the blur amount anyway, so with the factor set to 0.5 (probably the best default), integer part (+1) of the final slider would give iterations, the remainder would be like factor of the last iteration.
This should nicely illustrate the idea:
Possible pseudocode:
strength = strength - 0.001 # so when user says 4.0, it's not calculating extra iteration with 0 weight, there are probably way better ways iter = math.floor(strength) + 1 extra = max(strength, 0) % 1 # max() because of the negative delta
It would work something like this (GIF, don't know if it will play...)
Another idea (admittedly a bit more crazy...) - it would be interesting to have a mode (Strength / Distance dropdown?) that sort of normalizes the blur amount to real distance units, maybe look at average neighbor edge length and divide the input distance by it. Possibly multiply by some correction factor based on the internal factor. That number would then be the blur strength per vertex (could be an issue?). Something like this I imagine:
strength = correction_fac * dist_input / avg_neighbor_edge_len
Of course this would be problematic for dense meshes - make default distance small or based on vert count?
BTW how are you weighting the vertices? I'm just curious, I know about classic average, 1/d (or d**2 ?) and some cotangent weighting... The c/c++ is scaring me :)
Anyway, I'm really happy someone is making progress on this, cheers!