Page MenuHome

Fix T90666: Toggling motion blur while persistent data is enabled results in artifacts
ClosedPublic

Authored by Patrick Mours (pmoursnv) on Oct 7 2021, 5:21 PM.

Details

Summary

Enabling or disabling motion blur requires rebuilding the BVH of affected geometry and
uploading modified vertices to the device (since without motion blur the transform is
applied to the vertex positions, whereas with motion blur this is done during traversal).
Previously neither was happening when persistent data was enabled, since the relevant
node sockets were not tagged as modified after toggling motion blur.

The change to blender_object.cpp makes it so geom->set_use_motion_blur() is always
called (regardless of motion blur being toggled on or off), which will tag the geometry
as modified if that value changed and ensures the BVH is updated.
The change to geometry.cpp/mesh.cpp was necessary since after motion blur is disabled,
the transform is applied to the vertex positions of a mesh, but those changes were not
uploaded to the device. This is fixed now that they are tagged as modified.

Diff Detail

Repository
rB Blender
Branch
cycles_fix_persistent_data_motion_blur (branched from master)
Build Status
Buildable 17656
Build 17656: arc lint + arc unit

Event Timeline

Patrick Mours (pmoursnv) requested review of this revision.Oct 7 2021, 5:21 PM
Patrick Mours (pmoursnv) created this revision.

If I understand this correctly, the various x.tag_modified() are added because the use_motion_blur was modified in the exporter (blender_object.cpp) and we need to ensure that data is properly updated? If so, then, I think we should be able to simply update the proper device_update_flags in GeometryManager.device_update_preprocess for the device arrays to be tagged as modified appropriately. The changes in Mesh.apply_transform (tagging attributes and vertices as modified) could also be done there, so the logic is all centralized.

Ah, I missed GeometryManager.device_update_preprocess. That simplifies things a bit. It wasn't tagging the vertex array when a mesh data update was requested, hence them getting out of sync. Still tagging the arrays in Geometry.apply_transform directly though, since it seems wasteful to duplicate the logic that determines whether transform can be applied or not (see ObjectManager.apply_static_transforms).

This revision is now accepted and ready to land.Oct 8 2021, 4:35 PM

I observe this bug in 3.0.1