Randomize modifier
Needs ReviewPublic
Actions

Authored by Christian Hubert (alikendarfen) on May 2 2018, 12:35 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)

Summary

A modifier which randomizes vertices of a mesh.

What it does:

2 modes: per vertices (individual vertices are randomized) or per loose parts (each whole loose part is randomized).

Options for "per vertices":

Seed
Axis object (when used the axis of this object are considered instead of these of the modified object)
Random translations in X, Y or Z
Random "expansion" along normals
A vertex group can be used to influence the randomness

Options for "loose parts":

Seed
Axis object (when used the axis of this object are considered instead of these of the modified object)
Random translations in X, Y or Z. In this case each loose part is independently randomized
Random rotation around X, Y or Z

Diff Detail

Repository: rB Blender

Event Timeline

Christian Hubert (alikendarfen) created this revision.May 2 2018, 12:35 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)May 2 2018, 12:49 PM

I've done a very quick read-through of a part of the code, so by no means is this a complete review. I've just written down what I've noted so far.

There is no indication in the UI about which distribution is chosen for the randomness. For example, a uniform distribution will give different results than a gaussian distribution. I'm not saying that these should be added as choices (although that may be a nice idea too), but at least it should be clear to the user which distribution is used.

To me it's also not clear how the translation_avg_value and translation_range work together to form the final location of a vertex. How is a random translation chosen exactly?

This patch will go into Blender 2.8, so please update it so that it doesn't use DerivedMesh any more, but uses Mesh instead. See my and Mai's recent commits on the 2.8 branch, D3155, and T54737.

source/blender/makesrna/intern/rna_modifier.c
4914	The tooltip shouldn't simply repeat the information that is already in the UI, but should add more information. That way you can explain things that aren't already understood by the rest of the UI.
4939	The number of properties (and consequently the number of UI elements) can be reduced here. `use_object_axis=False` is semantically the same as `axis_object is None`, and `use_object_axis=True` corresponds to `axis_object is not None`. The Simple Deform modifier has a similar approach, where the axes can be defined by another object. Maybe take a look there?
4947	'middle' and 'average' are two different things ('middle' can refer to the median, whereas 'average' cannot). Consistent terminology makes the modifier easier to understand & use. I would also make sure that the singular/plural is clear; the property is called 'value' whereas the UI shows 'values'.

This revision now requires changes to proceed.May 2 2018, 1:05 PM

Hi Sybren,

About randomness distribution, is there any library that implements it already in Blender code (what I've used here is from "BLI_rand.h")?

Concerning translation_avg_value, I've hesitated with translation_middle_value. The final result is translation_avg_value + translation_range * (BLI_rng_get_float(rng) - 0.5f).

For the 2.8 branch, I'd be happy to start from it but I'm not able to setup a correct environment in order to have it compile and run. I'm really stuck with that, and from weeks now. Can that be some global incompatibility (VS2017, or other). Could you come back to me via email about it?

About "The number of properties (and consequently the number of UI elements) can be reduced here. use_object_axis=False is semantically the same as axis_object is None, and use_object_axis=True corresponds to axis_object is not None.",
I understand but that was intentional. The point is I feel more easy (in user point of view) to activate/deactivate a set value (the object) than adding/removing it. Is it matter of taste? For instance, array modifier does it the same way, but mirror does it the way you suggest.
But, ok, if you confirm that point, I will remove the check box. Just tell me.

OK for all other indications.

I will update the diff as soon the 2.8 branch can be correctly setup in my env.

Kind regards

Hi Sybren,

I use this in order to retrieve vertex group data:

	MDeformVert *vgroup = NULL;
	int vgroup_index = -1;
	modifier_get_vgroup_mesh(object, mesh, randomModifierData->vgroup, &vgroup, &vgroup_index);

But it seems to return nothing. Is that normal for now? (I've seen there is work in progress around that).

Thanks,
Christian

User interface clarification:

Better tooltips, labels.
I've kept the behavior concerning "axis object" (same principle as for array modifier)
Tooltip on seed indicates the random function used (uniform)
Changed the "avg" (average) value to "mid" (middle) + improvement in the tooltips

I've not found how to indicate X, Y, Z (Euler) for the rotation axis used in "loose parts" mode. So the values are not labeled for now.

Port to blender2.8 branch (Mesh instead of DerivedMesh + ModifierTypeInfo modifications).

I was not able to test the vertex group part as it seems the vg cannot be obtained from the original mesh
Some functions at the beginning of the MOD_randomize.c file are candidate to be shared (so moved to other existing files) but I've not do it for now because I feel it more secure for a first participation (but tell me if you want me to do it).

In D3195#75647, @Christian Hubert (alikendarfen) wrote:

For the 2.8 branch, I'd be happy to start from it but I'm not able to setup a correct environment in order to have it compile and run. I'm really stuck with that, and from weeks now. Can that be some global incompatibility (VS2017, or other). Could you come back to me via email about it?

hi, windows maintainer here, I don't want to hijack this review, since it'll cause too much noise, but 2017 should be no problem for 2.8, the most common issue is people re-using a master build folder for 2.8 (doesn't work, certain defaults have changed in cmake, and re-using the same build folder will not pick up on that) so, try deleting your build folder, if that doesn't work, post a more detailed problem description on devtalk.blender.org or bf-comitters and I'll get that sorted out for you.

@Ray Molenkamp (LazyDodo), thanks: it is working now

Manuel Grad (manitwo) added a subscriber: Manuel Grad (manitwo).May 27 2018, 10:25 AM