Details

Reviewers

Bastien Montagne (mont29)
Luca Rood (LucaRood)

Group Reviewers

Physics

Commits

rBS4c164487bc15: Add "Gravitation" option to "Force" type force fields
rB4c164487bc15: Add "Gravitation" option to "Force" type force fields

Summary

The current falloff function does not have an accurate 1/r^power characteristic, because of (1.0f-mindist) added to the radius.
The deviations from the 1/r^power function are greater with smaller distances.

The difference can be seen in the attached .blend file, where particles orbit the center of a force field with a 1/r² gradient: The particles take the path of a ( stationary ) ellipse, whereas with the current falloff function the particle tracks have a cycloid-like shape.

ForceFieldDemo1.blend638 KBDownload

( play animation to see the effect)
Here is a screenshot of the demo file with the corrected power function:

Diff Detail

Event Timeline

Andreas Erlbacher (AndreasE) updated this revision to Diff 7923.Dec 1 2016, 8:48 PM

Andreas Erlbacher (AndreasE) retitled this revision from to Force Field power falloff function not accurate.

Andreas Erlbacher (AndreasE) updated this object.

Andreas Erlbacher (AndreasE) added a project: Physics.

Andreas Erlbacher (AndreasE) added a reviewer: Physics.

@Luca Rood (LucaRood) mind double-checking that? Looks good to me, but…

Bastien Montagne (mont29) mentioned this in T50251: Force Field: Power falloff function calculation error.Dec 16 2016, 6:32 PM

Well, this is a delicate subject, and both approaches are valid, depending on the point of view.

The way Blender currently does it, ensures that within mindist the influence is exactly one, and it decreases exponentially from there. This ensures the strength of the force field never goes above the value set by the user.

With the approach in this patch, a factor of less than 1.0 results in forces greater than those set by the user (approaching infinity as factor approaches 0.0), after applying the falloff function. Also, with this approach, when mindist > 0.0 all forces within mindist will be equal to the forces exactly at mindist, which differs from the behavior currently expected, where forces within mindist will always have a factor of 1.0.

One could argue that the latter is more physically accurate when used with a force field of type "Force", as can indeed be observed in the orbiting particle example. However, the falloff options are not really meant to simulate any kind of physical behavior, but rather just provide an intuitive way to control influences. Also, this might be confusing to the user, as changing mindist will affect the strength of the forces within that distance, which one would probably not usually expect.

Furthermore, this falloff function only really makes sense for force fields of type "Force", while for other types, such as "Wind", this would really not make sense at all.

I think it might be a worthwhile change, to enable better orbiting simulations, but should only be implemented for type "Force", so the falloff function for type "Force" would have to be separated from the other types, to keep those unchanged. And even then, this might be better suited for 2.8, as it introduces major breaking changes in force field behavior.

As an alternative (and probably better) solution, this could be implemented as a separate type of force field (perhaps called "Gravitation"?), which would have this kind of falloff hard coded into the force calculation, or maybe add this as a "Simulate Gravitation" option to the existing "Force" force field. This way the falloff options are kept as an intuitive influence control, but orbiting simulations are still possible.

source/blender/blenkernel/intern/effect.c
480 ↗	(On Diff #7923)	Be careful with functions that approach infinity, when `fac == 0.0f` this will return `inf`. Something like this check before your return would fix that: if (fac < FLT_EPSILON) return FLT_MAX;

This revision now requires changes to proceed.Dec 16 2016, 11:29 PM

Silly mistake in my inline comment :P

source/blender/blenkernel/intern/effect.c
480 ↗	(On Diff #7923)	I just realized that returning `FLT_MAX` won't work either, because it would result in `inf` anyway when multiplying any force greater than 1.0. However, when the point affected by the force is at the same location as the force (i.e. `fac == 0.0`), there shouldn't really be any force at all. So this would actually be the right fix: if (fac < FLT_EPSILON) return 0.0f;

Funny I haven't seen the easy solution before: When the minimum distance is 1.0 the falloff function is exactly ~1/r² with the unmodified falloff function ( at least for distances greater than 1). So it shall be sufficient to put just a note in the UI text like this.

Indeed, that does partially solve the issue, however, only when your mass never goes within the 1.0 mindist, otherwise, it will encounter a constant force, which will change its orbit (perhaps we should take the force field object's scale into account, for the falloff stuff, so that one can change the actual scale of the distances).

Regarding your tool-tip, I think it is a bit unclear, and makes it seem like one should always use minimum distance of 1.0 when using the falloff power. Perhaps change it to "real gravitational falloff = 2, with minimum distance = 1.0"?

Although, I actually think it is a bit hacky to use the falloff options, as they are now, to simulate gravitation, and I think that even the "real gravitational falloff =2" tool-tip probably shouldn't have been added in the first place, and is rather a misinterpretation of the intention behind the falloff options (it is quite misleading, and doesn't even make sense at all for a force field of any type other than "Force"). I would rather have a proper dedicated gravitational falloff option specifically in the "Force" force field's options (independent of the existing falloff options), which wouldn't have any of that mindist stuff. And then the "real gravitation" tool-tip could actually be removed from the generic falloff options.

@Bastien Montagne (mont29), any thoughts?

Yes, technically a separate type of force field for gravitation would definitely make more sense - if there is a demand from users - because it would be a bit more work than changing just a few lines.
I am just beginning learn to learn Blender, and to have a look into the source code, but I can see how far I will get... ;-)

Like I said above, this doesn't have to be a separate force field type, but rather just a "gravitational falloff" option in the "Force" force field. This is a small enough feature, that it wouldn't really require user demand. Also, I think this is a better option, than misleading the user about the function of the current falloff options (which is already the case, with the mention of gravity in the tool-tip).

Adding this extra falloff option would indeed be just a few lines of code, and is quite trivial. If you are looking to learn more about the Blender code base, this is a great way to start :)
In any case, I think the mention of gravity should be removed from the tool-tip, regardless of having this "gravitational falloff" option.

However, I'd like to get @Bastien Montagne (mont29)'s opinion on this.

This diff adds a "Gravitation"-option to the force field falloff, which multiplies the force by 1/r² ( The original falloff function is still available). However, it is still missing the properties of a true gravitational force, because the masses are not taken into consideration: several objects with different masses do not work properly.
Adding the mass dependency would certainly go beyond the simple "force"-field type and make a new force field type necessary - so I am not sure if its worth implementing...

Looking at the code, this seems good :) (just a few little comments inline).
However, I didn't test, because the patch doesn't apply. I suspect that your editor is replacing tabs with spaces, messing up the diff.
Also, please use tabs for indentation, except in Python (refer to https://wiki.blender.org/index.php/Dev:Doc/New_Developer_Advice#Note_on_Configuration)

Regarding object masses, don't worry about that, as the orbiting objects generally have much lower mass than what they orbit around, thus making this parameter basically negligible. Also, in Blender we don't have true two-way gravitational relationships, which further makes this distinction irrelevant.

Lastly, don't forget to remove the note about gravitational falloff in the falloff_power tooltip, as that is now irrelevant with this new feature.

release/scripts/startup/bl_ui/properties_physics_common.py
279–281 ↗	(On Diff #8287)	You probably don't need this split nor the column, you can add the prop directly to `layout`. But actually, it's probably best to add this prop to the existing leftmost column defined above.
source/blender/blenkernel/intern/effect.c
852 ↗	(On Diff #8287)	Why do you need this assignment?
857 ↗	(On Diff #8287)	Best to use `powf` when dealing with floats. Also, wrong indentation, and missed a space after the comma.
source/blender/makesrna/intern/rna_object_force.c
1398	Best not to abbreviate Python properties too much, you can just use "use_gravity_falloff" here. Same goes for the UI text below.

Corrections

The tabs should be ok now ( my method of submitting the diff by copy/paste created the error).

Regarding the object masses: Suppose a (smaller) object orbits the fixed central object with a gravitational force field. With a given orbit size, the speed of the orbiting shall be independent of the mass of the orbiting object.
With the current force field in Blender, the force is independent of the mass, but the acceleration is dependent - therefore objects with different masses have different orbits. However, as long as all the orbiting objects have the same mass, it does not matter.

source/blender/blenkernel/intern/effect.c
852 ↗	(On Diff #8287)	Actually we can skip the assignment ( I thought about speed optimisation - if I had used `fac` also in line 856 - but it wouldn't be faster)

I think this is all good now.
Just noting that it is patching fine, except for DNA_object_force.h (still has whitespace issues, but was just one line, so I patched it manually for testing...)

But anyway, it is best to find a reliable way of submitting diffs... You might want to look into arcanist (https://wiki.blender.org/index.php/Dev:Doc/Tools/Code_Review#Use_Arcanist), though I actually use this little script which takes a piped input, and copies it to the clipboard: http://pasteall.org/262230/bash (runs on Linux and requires xclip).

About the masses, you are indeed correct. Though I think this implementation is good enough for now, we can figure out proper gravitational relations later, for 2.8 perhaps.

@Bastien Montagne (mont29), any objections?

This revision is now accepted and ready to land.Feb 23 2017, 5:17 AM

LGTM, besides nit picky comments below about UI messages (which can be addressed while committing the patch, no need to do another update just for that of course).

source/blender/makesrna/intern/rna_object_force.c
1278	It’s better not to have any tip, than having a tip saying exactly the same thing as the label… so should just be `""` as tip here.
1399	We use English Title Case for our labels (hence, `"Gravity Falloff"`).

Closed by commit rB4c164487bc15: Add "Gravitation" option to "Force" type force fields (authored by Luca Rood (LucaRood)). · Explain WhyFeb 23 2017, 11:26 PM

This revision was automatically updated to reflect the committed changes.

Alexander Gavrilov (angavrilov) mentioned this in D8075: Force Fields: implement new true power and custom falloff options..Jun 19 2020, 1:03 PM

Force Field power falloff function not accurate
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 8014

source/blender/makesrna/intern/rna_object_force.c

Force Field power falloff function not accurateClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 8014

source/blender/makesrna/intern/rna_object_force.c

Force Field power falloff function not accurate
ClosedPublic
Actions

Revision Contents
Changeset List