Regression: 'Object Info' 'Random' always gives the same seed for instanced bezier curves #101270

New Issue

Duarte Farrajota Ramos · 2022-09-22T13:30:35+02:00

2022-09-22 13:30:35 +02:00

System Information
Operating system: Windows-10-10.0.19044-SP0 64 Bits
Graphics card: Quadro P4000/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 516.40
Graphics card: NVidia RTX 3090

Blender Version
Broken: version: 3.0, caused by:

b9febb54a4 (Eevee only)
1bc655c5aa (Cycles)
Worked: 2.93

Short description of error
Input > Object Info > Random shader node generates the same seed for bezier curves objects when instanced as collection from an empty.

Exact steps for others to reproduce the error

Add bezier curve to default collection "Collection"
Add some Bevel depth
Switch to Rendered or Material Preview shading mode
Add a material to the curve
Add an Input > Object Info node to the material
Connect the Random socket to the Principled BSDF shader Base Color input
In the 3D View instance the Collection with Shift + A Add > Collection Instance > Collection
Repeat the process a few times or duplicate the collection instance

Notice how the instanced bezier curves always has the same color as the "original", both in Material Preview, Cycles or EEVEE.
Making instances real, converting the bezier curve to mesh, or duplicating the curve directly without instancing through an empty, brings back the expected random colors.
Mesh objects don't display the same issue, instancing from geometry nodes works as expected.

In the image below both the cube and the bezier circle have a material with random colors made with Input > Object Info > Random shader node.
The bezier curves however always display a yellow color unless converted to mesh.

ObjectInfo_Random_Curves.blend

3.4 alpha displays same behavior, 2.93 is the last working official release, so it was introduced some time in 3.0.

**System Information** Operating system: Windows-10-10.0.19044-SP0 64 Bits Graphics card: Quadro P4000/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 516.40 Graphics card: NVidia RTX 3090 **Blender Version** Broken: version: 3.0, caused by: - b9febb54a4 (Eevee only) - 1bc655c5aa (Cycles) Worked: 2.93 **Short description of error** *Input > Object Info > Random* shader node generates the same seed for bezier curves objects when instanced as collection from an empty. **Exact steps for others to reproduce the error** - Add bezier curve to default collection "Collection" - Add some Bevel depth - Switch to *Rendered* or *Material Preview* shading mode - Add a material to the curve - Add an Input > Object Info node to the material - Connect the *Random* socket to the *Principled BSDF* shader *Base Color* input - In the 3D View instance the Collection with `Shift` + `A` *Add > Collection Instance > Collection* - Repeat the process a few times or duplicate the collection instance Notice how the instanced bezier curves always has the same color as the "original", both in Material Preview, Cycles or EEVEE. Making instances real, converting the bezier curve to mesh, or duplicating the curve directly without instancing through an empty, brings back the expected random colors. Mesh objects don't display the same issue, instancing from geometry nodes works as expected. In the image below both the cube and the bezier circle have a material with random colors made with *Input > Object Info > Random* shader node. The bezier curves however always display a yellow color unless converted to mesh. ![Bug_Random.gif](https://archive.blender.org/developer/F13553386/Bug_Random.gif) [ObjectInfo_Random_Curves.blend](https://archive.blender.org/developer/F13553430/ObjectInfo_Random_Curves.blend) 3.4 alpha displays same behavior, 2.93 is the last working official release, so it was introduced some time in 3.0.

Duarte Farrajota Ramos commented

2022-09-22 13:30:35 +02:00

Added subscriber: @DuarteRamos

Germano Cavalcante commented

2022-09-23 16:18:46 +02:00

Added subscriber: @mano-wii

Germano Cavalcante commented

2022-09-23 16:18:46 +02:00

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

Germano Cavalcante commented

2022-09-23 16:18:46 +02:00

I can confirm the problem. It was introduced in 3.0 sometime before 276eebb274
Strange it hasn't been reported before.

I can confirm the problem. It was introduced in 3.0 sometime before 276eebb274 Strange it hasn't been reported before.

Duarte Farrajota Ramos commented

2022-09-28 11:12:39 +02:00

This also seems to affect Cycles renders, should it be tagged with #render_cycles as well?

Germano Cavalcante commented

2022-09-28 15:33:14 +02:00

It takes a bisect to actually identify the area, but as it's also seen in Cycles, it must be a #Core or #Modeling issue.

Germano Cavalcante commented

2022-09-28 21:28:20 +02:00

Added subscriber: @HooglyBoogly

Germano Cavalcante commented

2022-09-28 21:28:20 +02:00

Caused by b9febb54a4
CC @HooglyBoogly

Caused by b9febb54a4 CC @HooglyBoogly

Germano Cavalcante changed title from ~~Object Info Random always gives the same seed for instanced bezier curves~~ to Regression: 'Object Info' 'Random' always gives the same seed for instanced bezier curves

2022-09-28 21:29:35 +02:00

Lukas Stockner commented

2022-10-15 02:13:00 +02:00

Added subscriber: @LukasStockner

Lukas Stockner commented

2022-10-15 02:13:00 +02:00

From what I can tell, this is actually caused by 3211c80a31.

From what I can tell, this is actually caused by 3211c80a31.

Lukas Stockner commented

2022-10-15 02:56:40 +02:00

The problem appears to be that make_dupli, which assigns the random IDs, mixes in the name of the object referenced by the dupli context. However, when the evaluated geometry's make_dupli is called, the context is set to the object itself, so this step no longer happens.

I guess the random_id of the evaluated geometry's dupli should be set to that of the base dupli?

For reference, this patch fixes the random IDs (but probably breaks 100s of other things, I have no idea how this actually works):

diff --git a/source/blender/blenkernel/intern/object_dupli.cc b/source/blender/blenkernel/intern/object_dupli.cc
index 96da99af97e..5885b6853e7 100644
--- a/source/blender/blenkernel/intern/object_dupli.cc
+++ b/source/blender/blenkernel/intern/object_dupli.cc
@@ -842,10 +842,12 @@ static void make_duplis_geometry_set_impl(const DupliContext *ctx,
                                           bool geometry_set_is_instance,
                                           bool use_new_curves_type)
 {
+  DupliObject* orig = (DupliObject*) ctx->duplilist->last;
   int component_index = 0;
   if (ctx->object->type != OB_MESH || geometry_set_is_instance) {
     if (const Mesh *mesh = geometry_set.get_mesh_for_read()) {
-      make_dupli(ctx, ctx->object, &mesh->id, parent_transform, component_index++);
+      DupliObject* dupli = make_dupli(ctx, ctx->object, &mesh->id, parent_transform, component_index++);
+      if (orig) dupli->random_id = orig->random_id;
     }
   }
   if (ctx->object->type != OB_VOLUME || geometry_set_is_instance) {

In any case, this is definitely not a problem on the rendering side.

The problem appears to be that `make_dupli`, which assigns the random IDs, mixes in the name of the object referenced by the dupli context. However, when the evaluated geometry's `make_dupli` is called, the context is set to the object itself, so this step no longer happens. I guess the `random_id` of the evaluated geometry's dupli should be set to that of the base dupli? For reference, this patch fixes the random IDs (but probably breaks 100s of other things, I have no idea how this actually works): ``` diff --git a/source/blender/blenkernel/intern/object_dupli.cc b/source/blender/blenkernel/intern/object_dupli.cc index 96da99af97e..5885b6853e7 100644 --- a/source/blender/blenkernel/intern/object_dupli.cc +++ b/source/blender/blenkernel/intern/object_dupli.cc @@ -842,10 +842,12 @@ static void make_duplis_geometry_set_impl(const DupliContext *ctx, bool geometry_set_is_instance, bool use_new_curves_type) { + DupliObject* orig = (DupliObject*) ctx->duplilist->last; int component_index = 0; if (ctx->object->type != OB_MESH || geometry_set_is_instance) { if (const Mesh *mesh = geometry_set.get_mesh_for_read()) { - make_dupli(ctx, ctx->object, &mesh->id, parent_transform, component_index++); + DupliObject* dupli = make_dupli(ctx, ctx->object, &mesh->id, parent_transform, component_index++); + if (orig) dupli->random_id = orig->random_id; } } if (ctx->object->type != OB_VOLUME || geometry_set_is_instance) { ``` In any case, this is definitely not a problem on the rendering side.

Jeroen Bakker commented

2022-11-08 10:53:17 +01:00

Added subscriber: @Jeroen-Bakker

Jeroen Bakker commented

2022-11-08 10:53:17 +01:00

Removing eevee project tag as cause is in other area (GeometryNodes/Modeling) to keep bug list manageable. Please don't hesitate to add it back or ask us directly when needed help from our side.

Brecht Van Lommel commented

2022-11-09 18:10:04 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2022-11-09 18:10:04 +01:00

@HooglyBoogly, I'm assuming you will work on this, or do you want the render module to help out?

Hans Goudey commented

2022-11-09 22:17:44 +01:00

I've looked into this a few times by now, along with related issues #100706 and #99088. Working with this code it makes me feel so stupid, it's just so hard to piece anything logical together.
I tried again today but my approach (holding the "parent" object in the context and using it in some cases) didn't end up working. I can look again tomorrow, for now I'm at my limits with this area.

The existing code doesn't handle nested instancing for meshes. If I make the cube a "vertex dupli" of a new single-vert object in the collection, all of the cubes have the same color.

Come to think of it, I don't really get the logic behind the "correct" behavior. The collection instance is instancing a single collection-- logically it would make more sense if each instance of a single object had the same ID.
I guess the answer is that the random ID isn't per-original object, it's per-instance? That logic doesn't seem to handle nested instancing though. Is it always supposed to use the ID of the top-most object?

I think we would get a great benefit from rewriting this area. I don't know how possible that is without changing behavior though. But there has to be a better way...

I've looked into this a few times by now, along with related issues #100706 and #99088. Working with this code it makes me feel so stupid, it's just so hard to piece anything logical together. I tried again today but my approach (holding the "parent" object in the context and using it in some cases) didn't end up working. I can look again tomorrow, for now I'm at my limits with this area. The existing code doesn't handle nested instancing for meshes. If I make the cube a "vertex dupli" of a new single-vert object in the collection, all of the cubes have the same color. --- Come to think of it, I don't really get the logic behind the "correct" behavior. The collection instance is instancing a single collection-- logically it would make more sense if each instance of a single object had the same ID. I guess the answer is that the random ID isn't per-original object, it's per-instance? That logic doesn't seem to handle nested instancing though. Is it always supposed to use the ID of the top-most object? --- I think we would get a great benefit from rewriting this area. I don't know how possible that is without changing behavior though. But there has to be a better way...

Brecht Van Lommel commented

2022-11-10 17:41:47 +01:00

The random ID should be per instance, but indeed seems to be broken for nested instancing in various cases. The point is to be able to add randomization in shading of object instances.

I think the root object name, instance object name and persistent ID together should generate a unique random ID per instance. The intermediate parent names are not needed as far as I can tell.

So this patch seems like it works, but have not tested extensively:

diff --git a/source/blender/blenkernel/intern/object_dupli.cc b/source/blender/blenkernel/intern/object_dupli.cc
index 5efd44c..e98dc08 100644
--- a/source/blender/blenkernel/intern/object_dupli.cc
+++ b/source/blender/blenkernel/intern/object_dupli.cc
@@ -87,6 +87,9 @@ struct DupliContext {
   Object *obedit;
 
   Scene *scene;
+  /** Root parent object at the scene level. */
+  Object *root_object;
+  /** Immediate parent object in the context. */
   Object *object;
   float space_mat[4][4];
   /**
@@ -138,6 +141,7 @@ static void init_context(DupliContext *r_ctx,
   r_ctx->scene = scene;
   r_ctx->collection = nullptr;
 
+  r_ctx->root_object = ob;
   r_ctx->object = ob;
   r_ctx->obedit = OBEDIT_FROM_OBACT(ob);
   r_ctx->instance_stack = &instance_stack;
@@ -277,8 +281,8 @@ static DupliObject *make_dupli(const DupliContext *ctx,
     dob->random_id = BLI_hash_int_2d(dob->random_id, 0);
   }
 
-  if (ctx->object != ob) {
-    dob->random_id ^= BLI_hash_int(BLI_hash_string(ctx->object->id.name + 2));
+  if (ctx->root_object != ob) {
+    dob->random_id ^= BLI_hash_int(BLI_hash_string(ctx->root_object->id.name + 2));
   }
 
   return dob;

Is that the approach you tried and found issues with, or was it something different?

The random ID should be per instance, but indeed seems to be broken for nested instancing in various cases. The point is to be able to add randomization in shading of object instances. I think the root object name, instance object name and persistent ID together should generate a unique random ID per instance. The intermediate parent names are not needed as far as I can tell. So this patch seems like it works, but have not tested extensively: ``` diff --git a/source/blender/blenkernel/intern/object_dupli.cc b/source/blender/blenkernel/intern/object_dupli.cc index 5efd44c..e98dc08 100644 --- a/source/blender/blenkernel/intern/object_dupli.cc +++ b/source/blender/blenkernel/intern/object_dupli.cc @@ -87,6 +87,9 @@ struct DupliContext { Object *obedit; Scene *scene; + /** Root parent object at the scene level. */ + Object *root_object; + /** Immediate parent object in the context. */ Object *object; float space_mat[4][4]; /** @@ -138,6 +141,7 @@ static void init_context(DupliContext *r_ctx, r_ctx->scene = scene; r_ctx->collection = nullptr; + r_ctx->root_object = ob; r_ctx->object = ob; r_ctx->obedit = OBEDIT_FROM_OBACT(ob); r_ctx->instance_stack = &instance_stack; @@ -277,8 +281,8 @@ static DupliObject *make_dupli(const DupliContext *ctx, dob->random_id = BLI_hash_int_2d(dob->random_id, 0); } - if (ctx->object != ob) { - dob->random_id ^= BLI_hash_int(BLI_hash_string(ctx->object->id.name + 2)); + if (ctx->root_object != ob) { + dob->random_id ^= BLI_hash_int(BLI_hash_string(ctx->root_object->id.name + 2)); } return dob; ``` Is that the approach you tried and found issues with, or was it something different?

Hans Goudey commented

2022-11-11 00:33:26 +01:00

I like your fix, it seems to make sense. It takes advantage of the fact that the random ID was already broken for nested instancing to avoid adding too much complexity.
Mixing in the root object ID is probably fairly safe too, since it won't introduce variation per instance.

The intermediate parent names are not needed as far as I can tell.

Okay, I think that makes sense, since the persistent ID should handle variation at intermediate levels already. It definitely makes the problem simpler.

Is that the approach you tried and found issues with, or was it something different?

I was trying to use the next object up in the instance tree. Determining that when each level has a different context was a bit tricky, and I didn't get it to work. This seems much simpler.

I like your fix, it seems to make sense. It takes advantage of the fact that the random ID was already broken for nested instancing to avoid adding too much complexity. Mixing in the root object ID is probably fairly safe too, since it won't introduce variation per instance. >The intermediate parent names are not needed as far as I can tell. Okay, I think that makes sense, since the persistent ID should handle variation at intermediate levels already. It definitely makes the problem simpler. >Is that the approach you tried and found issues with, or was it something different? I was trying to use the next object up in the instance tree. Determining that when each level has a different context was a bit tricky, and I didn't get it to work. This seems much simpler.

blender-admin commented

2022-11-11 12:54:01 +01:00

This issue was referenced by 4f22e6178e

This issue was referenced by 4f22e6178e62eee448b597cc14ee8b8fa2a63612

blender-admin commented

2022-11-11 12:54:01 +01:00

This issue was referenced by 80f5a5a8aa

This issue was referenced by 80f5a5a8aa12594fb724e9ba45185e13c914f7dd

Brecht Van Lommel commented

2022-11-11 14:45:56 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Brecht Van Lommel closed this issue