Cycles: add support for motion bluring of dynamic meshes.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Jun 21 2016, 9:27 PM.

Details

Summary

This adds support for motion bluring of dynamic meshes (such as meshes from liquid simulation) based on the patch provided by @Daniel Genrich (genscher) in T47069.

The trick here is to properly interpolate motion blur data based on the velocity vector of the current (center) frame unlike for static meshes motion blur where data is interpolated between frames, which will most likely fail for dynamic meshes. To achieve this, for now, the patch makes the very weak assumption that there's only gonna be two motion times (which
is the case in Cycles currently). A better approach is left for discussion.

Also the patch duplicates a bit of code from sync_mesh_motion. Would need to check on de-duplicating this.

Diff Detail

Repository
rB Blender
Kévin Dietrich (kevindietrich) retitled this revision from to Cycles: adds support for motion bluring of dynamic meshes..Jun 21 2016, 9:27 PM
Kévin Dietrich (kevindietrich) updated this object.
Kévin Dietrich (kevindietrich) retitled this revision from Cycles: adds support for motion bluring of dynamic meshes. to Cycles: add support for motion bluring of dynamic meshes..Jun 21 2016, 9:28 PM

Hey,

you did awesome work, Kevin!

The idea behind the code duplicaiton was that all "deform" modifier/physics could be supported in this function later.
But you are right. If this is only used for fluids, it would be simplier to de-dup this.

UPDATE:
You could commit the changes from this file, since they don't effect Cycles and are only for the elbeem module:
source/blender/makesrna/intern/rna_fluidsim.c

I don't have fast internet atm, so I can't do it myself, sorry! :-)

Thanks @Daniel Genrich (genscher).

I haven't mentioned this, but the reason I'm looking into this patch is for my current work on Alembic, where you might have files with per frame velocity information. I have some liquid and fracture caches from Houdini and Realflow for instance. So the code is most likely gonna be used for other things than Elbeem meshes.

Here is a test with an Alembic file from Houdini (through a mesh cache modifier). Motion blur kinda fails, I don't know why and where yet, but it is a start:

Brecht Van Lommel (brecht) requested changes to this revision.Jul 2 2016, 9:36 PM

Great to see this tackled, some comments.

intern/cycles/blender/blender_mesh.cpp
1143

Can this actually happen? I would expect us to disable motion export entirely in that case, or at least handle it the same way in both the fluid and non-fluid cases.

intern/cycles/blender/blender_object.cpp
336

From what I can tell other times would work correctly, but it would just be wasting memory since it's only doing linear interpolation of the velocities anyway, right?

I think there is an issue here though, if we have fluid velocities then it should not call sync_mesh_motion_velocity() for times other than -1.0 and 1.0. Otherwise we can end up with fluid velocities at some times and motion from other modifiers at other times.

Perhaps it would be good to just call sync_mesh_motion(), and then move the logic for checking if we are dealing with a fluid modifier into that function and skip or use the fluid velocities as needed there. With some luck we can also deduplicate more code that way.

intern/cycles/blender/blender_sync.h
168

This seems to serve no purpose? Meshes get inserted but it's never read or cleared.

This revision now requires changes to proceed.Jul 2 2016, 9:36 PM
intern/cycles/blender/blender_mesh.cpp
1143

It's mostly a paranoid check, since it's used to interpolate the velocity in the loop below.

intern/cycles/blender/blender_object.cpp
336

Other times would work correctly, though memory usage wasn't what I have in mind here. It's more related to the TODO noted in blender_mesh.cpp, about only assuming that two times are supported. To get motion blur information Cycles changes frame and loads the object which will not work for fluid meshes, so we have to interpolate the data in the same frame. In that respect this check is a bit bogus since all the work will be done at time -1.0, there's no need to check for time 1.0.

I'll merge the two functions together and we'll go from there.

intern/cycles/blender/blender_sync.h
168

Yes, it doesn't have any purpose. It's from some failed test I made, it should have been removed.

  • Merge sync_mesh_motion_velocity into sync_mesh_motion.

Looks good to me now.

This revision is now accepted and ready to land.Jul 4 2016, 6:38 PM

There is a couple of issues though, one noted inline.

The patch works mostly fine:

Though when the velocity is too high, it goes crazy:

The max velocity in the second image is <3.869598, 3.098900, 1.874617>, whilst in the first one it is <0.143458, 0.108711, -0.732028>. To be honest the time scale of this simulation is quite high (to get interesting data to test the patch) so I don't think it will be that much of an issue on real simulations, done properly, but it is still an issue. Anyone's got any idea what could be done here to prevent such cases, something that doesn't feel arbitrary like multiplying by some magic number?

intern/cycles/blender/blender_mesh.cpp
1035

This is wrong, motion_time will always be -1.0f, to make it work properly, and to be future proof, I think we could use the motion_times set, then we will not have to worry about the code only assuming 2 motion times. What do you think here?

To get the result in the first attached image, I manually set motion_time to 1.0f at the end of the loop, which isn't nice.

Brecht Van Lommel (brecht) requested changes to this revision.Jul 5 2016, 1:21 AM

What makes the velocities go crazy, is the velocity field "noisy" which gives these kinds of spikes? Would it need some kind of smoothing to be used for rendering? Would it look better with a higher mesh resolution?

Or is it an issue with the specific way we are exporting or rendering things in Cycles, and rendering the same image with Blender Internal and vector blur in the compositor gives good results?

This revision now requires changes to proceed.Jul 5 2016, 1:21 AM
intern/cycles/blender/blender_mesh.cpp
1035

I don't understand what you propose to change, perhaps the code for the change would help me understand it.

Thinking about this more it's not clear to me how -1.0f or 1.0f are working at all, I think in those cases we are evaluating different frames and the mesh topology can be totally different. Should we be exporting the vertex velocities immediately along with the vertex positions, instead of doing this as part of regular motion blur export?

Technically, such velocity data should be scaled using the simulation time step and the size/dimension of the object (or voxel size for grided simulations). This should already have been done by the fluid solver: https://developer.blender.org/diffusion/B/browse/master/intern/elbeem/intern/ntl_blenderdumper.cpp;39ae324918176a1a94de0c6be4466fdffa2fe711$151

With the same file and a higher resolution, the problem is more visible (I doubled the resolution and the max velocity almost doubled from <3.869598, 3.098900, 1.874617> to <6.785783, 7.264922, 2.286141>). Maybe we should sill scale the velocity.

Then as I said, it's not your typical fluid simulation, the liquid drastically change from one frame to the other, here's two consecutive frames, the first one being the one with the crazy velocities from above:


In a typical simulation, the liquid surface will not change so quickly, and the velocity field will be a lot smoother. The liquid solver has a weird, non conventional way of dealing with time scale, so I didn't bother too much with it. I think my setting is 5 times what it should be.

Using BI + compositing, the problem is the same, though you can get a decent looking blur by tweaking the Vector Blur node.

intern/cycles/blender/blender_mesh.cpp
1035

I'm usually bad at explaining, so bear with me :)

Yes, we should be exporting the velocities along with the vertex positions. And that's what is happening here. The interpolation for all motion times is done in one go (not by changing frame and using delta positions). We do that on the first time/frame, so there is no risk to interpolate with a different mesh, and the velocities coincide with the vertices. Thus the check above (line 962) for the time_index (if time_index == 0, then it's the first frame, otherwise return from function since we already have the data).

In non fluid cases, data is copied as required on frame change (because the mesh topology is the same).

Currently the code does it somewhat properly: we do interpolate for all motion times, but since we do it on the first frame, motion_time is set to -1.0f and is never changed. The motion_time parameter is passed all the way up from sync_motion() (which loops over the all motion times and changes scene frame each iteration), so I would like to do something similar here:

foreach(float relative_time : motion_times) {
    float3 *mP = attr_mP->data_float3() + step*numverts;
    float3 *mN = (attr_mN) ? attr_mN->data_float3() + step*numverts : NULL;

    for (b_fluid_domain.fluid_mesh_vertices.begin(fvi), i = 0; fvi != b_fluid_domain.fluid_mesh_vertices.end(); ++fvi, ++i) {
        float3 nPos = P[i] + get_float3(fvi->velocity())*relative_time*shuttertime*0.5f;
        mP[i] = nPos;

        if(mN) {
             mN[i] = N[i];
        }
    }
}

This will ensure that we do the interpolation for the right number of motion_times with the right values, and all in one go (without frame change).

Note that motion_time is not used in the non-fluid case, it's used here to change the direction of velocity (-1.0 will go backward, 1.0 forward), so we have a somewhat nice interpolation in time/space.

Ok, thanks for clearing that up.

This example does look like an extreme case were a single velocity vector can't possibly give accurate results.

The proposed code change seems good to me.

Use motion_times set for interpolating with the right number of motion times, update comments.

Testing this I'm still getting incorrect velocities, b_fluid_domain.fluid_mesh_vertices.length() does not match the number of vertices.

The issue is that we are syncing in sync_mesh_motion at time_index == 0, which is the previous frame. We never sync motion on the current frame so doing this as part of sync_motion can't work.

I've got an update for the patch to fix this, after that it seems to be all working correct for me.

Fix velocities not being exported at the current frame, causing vertex number mismatch.

Skip unnecessary work without motion blur, cleanup some now unnecessary code.

This revision was automatically updated to reflect the committed changes.