Fix #131218: Cycles: Race condition when syncing motion blur
The issue here is that motion_steps handling is a bit complex, and the parallel synchronization of geometry does not play well with it. The obvious result of this was a crash related to the main thread checking attributes while the geometry sync was changing them, but there was also another race condition that could result in ending up with the wrong motion_steps. Specific changes: - Change place where `motion_steps` is set to avoid concurrent access - Change the default `motion_steps` to zero, since they won't be explicitly set if there's no motion now - Don't skip `motion_steps` copy in `sync_X` since it's no longer set in `sync_object` and we need to transfer the value in case it was set to 3 by the velocity code since that's no longer the default Pull Request: https://projects.blender.org/blender/blender/pulls/133669
This commit is contained in:
@@ -1062,9 +1062,7 @@ void BlenderSync::sync_hair(BL::Depsgraph b_depsgraph, BObjectInfo &b_ob_info, H
|
||||
|
||||
for (const SocketType &socket : new_hair.type->inputs) {
|
||||
/* Those sockets are updated in sync_object, so do not modify them. */
|
||||
if (socket.name == "use_motion_blur" || socket.name == "motion_steps" ||
|
||||
socket.name == "used_shaders")
|
||||
{
|
||||
if (socket.name == "use_motion_blur" || socket.name == "used_shaders") {
|
||||
continue;
|
||||
}
|
||||
hair->set_value(socket, new_hair, socket);
|
||||
|
||||
@@ -209,6 +209,20 @@ void BlenderSync::sync_geometry_motion(BL::Depsgraph &b_depsgraph,
|
||||
return;
|
||||
}
|
||||
|
||||
/* If the geometry already has motion blur from a velocity attribute, don't
|
||||
* set the geometry motion steps again.
|
||||
*
|
||||
* Otherwise, setting geometry motion steps is done here to avoid concurrency issues.
|
||||
* - It can't be done earlier in sync_object_motion_init because sync_geometry
|
||||
* runs in parallel, and has_motion_blur would check attributes while
|
||||
* sync_geometry is potentially creating the attribute from velocity.
|
||||
* - It needs to happen before the parallel motion sync that happens right after
|
||||
* this, because that can create the attribute from neighboring frames.
|
||||
* Copying the motion steps from the object here solves this. */
|
||||
if (!geom->has_motion_blur()) {
|
||||
geom->set_motion_steps(object->get_motion().size());
|
||||
}
|
||||
|
||||
/* Find time matching motion step required by geometry. */
|
||||
const int motion_step = geom->motion_step(motion_time);
|
||||
if (motion_step < 0) {
|
||||
|
||||
@@ -1167,9 +1167,7 @@ void BlenderSync::sync_mesh(BL::Depsgraph b_depsgraph, BObjectInfo &b_ob_info, M
|
||||
|
||||
for (const SocketType &socket : new_mesh.type->inputs) {
|
||||
/* Those sockets are updated in sync_object, so do not modify them. */
|
||||
if (socket.name == "use_motion_blur" || socket.name == "motion_steps" ||
|
||||
socket.name == "used_shaders")
|
||||
{
|
||||
if (socket.name == "use_motion_blur" || socket.name == "used_shaders") {
|
||||
continue;
|
||||
}
|
||||
mesh->set_value(socket, new_mesh, socket);
|
||||
|
||||
@@ -131,12 +131,6 @@ void BlenderSync::sync_object_motion_init(BL::Object &b_parent, BL::Object &b_ob
|
||||
|
||||
geom->set_use_motion_blur(use_motion_blur);
|
||||
|
||||
if (!geom->has_motion_blur()) {
|
||||
/* Only set motion steps if geometry doesn't already have
|
||||
* motion blur from a velocity attribute. */
|
||||
geom->set_motion_steps(motion_steps);
|
||||
}
|
||||
|
||||
motion.resize(motion_steps, transform_empty());
|
||||
|
||||
if (motion_steps) {
|
||||
|
||||
@@ -212,9 +212,7 @@ void BlenderSync::sync_pointcloud(PointCloud *pointcloud, BObjectInfo &b_ob_info
|
||||
/* Update original sockets. */
|
||||
for (const SocketType &socket : new_pointcloud.type->inputs) {
|
||||
/* Those sockets are updated in sync_object, so do not modify them. */
|
||||
if (socket.name == "use_motion_blur" || socket.name == "motion_steps" ||
|
||||
socket.name == "used_shaders")
|
||||
{
|
||||
if (socket.name == "use_motion_blur" || socket.name == "used_shaders") {
|
||||
continue;
|
||||
}
|
||||
pointcloud->set_value(socket, new_pointcloud, socket);
|
||||
|
||||
@@ -40,7 +40,7 @@ NODE_ABSTRACT_DEFINE(Geometry)
|
||||
{
|
||||
NodeType *type = NodeType::add("geometry_base", nullptr);
|
||||
|
||||
SOCKET_UINT(motion_steps, "Motion Steps", 3);
|
||||
SOCKET_UINT(motion_steps, "Motion Steps", 0);
|
||||
SOCKET_BOOLEAN(use_motion_blur, "Use Motion Blur", false);
|
||||
SOCKET_NODE_ARRAY(used_shaders, "Shaders", Shader::get_node_type());
|
||||
|
||||
|
||||
Reference in New Issue
Block a user