Intermittent failure of cycles texture_space_mesh_modifier test #90476

New Issue

Campbell Barton · 2021-08-06T10:29:58+02:00

Campbell Barton commented

2021-08-06 10:29:58 +02:00

This report is to keep track of texture_space_mesh_modifier test failing, this happens very occasionally (1 in 50..100 times for example), this bisects to 27da305a40.

{F10270322, size=full}

To run the test until failure, note that removing all files in tests/render/mesh/ besides tests/render/mesh/texture_space_mesh_modifier.blend speeds up the test.

ctest -C Release -VV -O Test.log -R cycles_mesh_cpu --repeat-until-fail 9999

Thanks to @LazyDodo for bisecting & providing image + command to redo.

Findings:

Neither ASAN or Valgrind report errors with uninitialized memory use.
Calling DEG_id_tag_update(&ob->id, ID_RECALC_COPY_ON_WRITE) for all objects on file-read works around the problem.
This patch P2311, works around the problem (effectively reverting 27da305a40).
In the error cases, when cycles accesses the mesh it's ME_AUTOSPACE_EVALUATED flag is not set.

This report is to keep track of `texture_space_mesh_modifier` test failing, this happens very occasionally (1 in 50..100 times for example), this bisects to 27da305a40. {[F10270322](https://archive.blender.org/developer/F10270322/Clipboard_-_August_4__2021_2_27_PM.png), size=full} To run the test until failure, note that removing all files in `tests/render/mesh/` besides `tests/render/mesh/texture_space_mesh_modifier.blend` speeds up the test. `ctest -C Release -VV -O Test.log -R cycles_mesh_cpu --repeat-until-fail 9999` *Thanks to @LazyDodo for bisecting & providing image + command to redo.* --- **Findings:** - Neither ASAN or Valgrind report errors with uninitialized memory use. - Calling `DEG_id_tag_update(&ob->id, ID_RECALC_COPY_ON_WRITE)` for all objects on file-read works around the problem. - This patch [P2311](https://archive.blender.org/developer/P2311.txt), works around the problem (effectively reverting 27da305a40). - In the error cases, when cycles accesses the mesh it's `ME_AUTOSPACE_EVALUATED` flag is not set.

Campbell Barton commented

2021-08-06 10:29:58 +02:00

Added subscribers: @LazyDodo, @ideasman42

Brecht Van Lommel commented

2021-08-06 12:05:09 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2021-08-06 12:05:09 +02:00

Thanks for tracking down the cause.

The mesh datablock part of the depsgraph looks like this:

Copying of the texture space location and size from original to evaluated mesh happens in BKE_mesh_copy_parameters as part of PARAMETERS_EVAL
Recomputation of the texture space location and size on the evaluated mesh happens in BKE_mesh_eval_geometry as part of GEOMETRY_EVAL

I see no dependency between PARAMETERS_EVAL and GEOMETRY_EVAL, so they can randomly run in any order and overwrite each other.

Thanks for tracking down the cause. The mesh datablock part of the depsgraph looks like this: ![texture_space_mesh_depsgraph.png](https://archive.blender.org/developer/F10270568/texture_space_mesh_depsgraph.png) * Copying of the texture space location and size from original to evaluated mesh happens in `BKE_mesh_copy_parameters` as part of `PARAMETERS_EVAL` * Recomputation of the texture space location and size on the evaluated mesh happens in `BKE_mesh_eval_geometry` as part of `GEOMETRY_EVAL` I see no dependency between `PARAMETERS_EVAL` and `GEOMETRY_EVAL`, so they can randomly run in any order and overwrite each other.

Campbell Barton commented

2021-08-06 12:17:32 +02:00

Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated

Edit, this probably isn't the case after reading brecht's comments.

When this test fails, the me->texflag & ME_AUTOSPACE_EVALUATED flag is not set for the mesh cycles is accessing in Cycles create_mesh function.

Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated ---- Edit, this probably isn't the case after reading brecht's comments. When this test fails, the `me->texflag & ME_AUTOSPACE_EVALUATED` flag is not set for the mesh cycles is accessing in Cycles `create_mesh` function.

Brecht Van Lommel commented

2021-08-06 12:20:49 +02:00

In #90476#1202804, @ideasman42 wrote:
Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated.

This seems unlikely, I can't imagine how the depsgraph evaluation and cycles export run in parallel. I think texflag gets overwritten when PARAMETERS_EVAL runs after GEOMETRY_EVAL.

> In #90476#1202804, @ideasman42 wrote: > Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated. This seems unlikely, I can't imagine how the depsgraph evaluation and cycles export run in parallel. I think `texflag` gets overwritten when `PARAMETERS_EVAL` runs after `GEOMETRY_EVAL`.

Campbell Barton commented

2021-08-06 12:49:24 +02:00

In #90476#1202807, @brecht wrote:

In #90476#1202804, @ideasman42 wrote:
Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated.

This seems unlikely, I can't imagine how the depsgraph evaluation and cycles export run in parallel. I think texflag gets overwritten when PARAMETERS_EVAL runs after GEOMETRY_EVAL.

This looks to be the case, this test patch fixes the problem.

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index c269ad16824..647bdc54a10 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -2214,7 +2214,14 @@ void DepsgraphRelationBuilder::build_object_data_geometry_datablock(ID *obdata)
   build_idproperties(obdata->properties);
   /* Animation. */
   build_animdata(obdata);
-  build_parameters(obdata);
+  // build_parameters(obdata);
+  /* TODO: don't inline `build_parameters`. */
+  OperationKey parameters_entry_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_ENTRY);
+  OperationKey parameters_eval_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_EVAL);
+  OperationKey parameters_exit_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_EXIT);
+  add_relation(parameters_entry_key, parameters_eval_key, "Entry -> Eval");
+  add_relation(parameters_eval_key, parameters_exit_key, "Entry -> Exit");
+
   /* ShapeKeys. */
   Key *key = BKE_key_from_id(obdata);
   if (key != nullptr) {
@@ -2224,6 +2231,8 @@ void DepsgraphRelationBuilder::build_object_data_geometry_datablock(ID *obdata)
   OperationKey obdata_geom_eval_key(obdata, NodeType::GEOMETRY, OperationCode::GEOMETRY_EVAL);
   OperationKey obdata_geom_done_key(obdata, NodeType::GEOMETRY, OperationCode::GEOMETRY_EVAL_DONE);
   add_relation(obdata_geom_eval_key, obdata_geom_done_key, "ObData Geom Eval Done");
+  add_relation(parameters_entry_key, obdata_geom_eval_key, "ObData Geom Params");
+
   /* Type-specific links. */
   const ID_Type id_type = GS(obdata->name);
   switch (id_type) {

> In #90476#1202807, @brecht wrote: >> In #90476#1202804, @ideasman42 wrote: >> Updated, this looks to be a race-condition between cycles and the depsgraph, where cycles may access the mesh before it's auto-texture space has been calculated. > > This seems unlikely, I can't imagine how the depsgraph evaluation and cycles export run in parallel. I think `texflag` gets overwritten when `PARAMETERS_EVAL` runs after `GEOMETRY_EVAL`. This looks to be the case, this test patch fixes the problem. ``` diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc index c269ad16824..647bdc54a10 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc @@ -2214,7 +2214,14 @@ void DepsgraphRelationBuilder::build_object_data_geometry_datablock(ID *obdata) build_idproperties(obdata->properties); /* Animation. */ build_animdata(obdata); - build_parameters(obdata); + // build_parameters(obdata); + /* TODO: don't inline `build_parameters`. */ + OperationKey parameters_entry_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_ENTRY); + OperationKey parameters_eval_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_EVAL); + OperationKey parameters_exit_key(obdata, NodeType::PARAMETERS, OperationCode::PARAMETERS_EXIT); + add_relation(parameters_entry_key, parameters_eval_key, "Entry -> Eval"); + add_relation(parameters_eval_key, parameters_exit_key, "Entry -> Exit"); + /* ShapeKeys. */ Key *key = BKE_key_from_id(obdata); if (key != nullptr) { @@ -2224,6 +2231,8 @@ void DepsgraphRelationBuilder::build_object_data_geometry_datablock(ID *obdata) OperationKey obdata_geom_eval_key(obdata, NodeType::GEOMETRY, OperationCode::GEOMETRY_EVAL); OperationKey obdata_geom_done_key(obdata, NodeType::GEOMETRY, OperationCode::GEOMETRY_EVAL_DONE); add_relation(obdata_geom_eval_key, obdata_geom_done_key, "ObData Geom Eval Done"); + add_relation(parameters_entry_key, obdata_geom_eval_key, "ObData Geom Params"); + /* Type-specific links. */ const ID_Type id_type = GS(obdata->name); switch (id_type) { ```

Campbell Barton changed title from ~~Intermittent of failure of cycles texture_space_mesh_modifier test~~ to Intermittent failure of cycles texture_space_mesh_modifier test

2021-08-06 13:01:42 +02:00

Brecht Van Lommel commented

2021-08-06 13:01:45 +02:00

I don't think depending on PARAMETERS_ENTRY is correct, since that's before PARAMETERS_EVAL?

Probably you can depend on the entire parameters component, like this:

ComponentKey parameters_key(obdata, NodeType::PARAMETERS);
add_relation(parameters_key, obdata_geom_eval_key, "ObData Geom Params");

I don't think depending on `PARAMETERS_ENTRY` is correct, since that's before `PARAMETERS_EVAL`? Probably you can depend on the entire parameters component, like this: ``` ComponentKey parameters_key(obdata, NodeType::PARAMETERS); add_relation(parameters_key, obdata_geom_eval_key, "ObData Geom Params"); ```

Brecht Van Lommel commented

2021-08-06 13:04:48 +02:00

By the way, we also have randomly failing Cycles displacement tests since a few weeks. They also involve these texture coordinates and a modifier, so it seems likely to be the same cause.

blender-admin commented

2021-08-06 13:32:19 +02:00

This issue was referenced by f4adb35f28

This issue was referenced by f4adb35f28ece4096f19489273601e5327a08e99

Brecht Van Lommel commented

2021-08-06 14:09:35 +02:00

Changed status from 'Needs Triage' to: 'Resolved'

Brecht Van Lommel closed this issue

2021-08-06 14:09:35 +02:00

Brecht Van Lommel self-assigned this 2021-08-06 14:09:35 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Intermittent failure of cycles texture_space_mesh_modifier test #90476