Maniphest T64896

Switching Grease Pencil material slot link to Object and back to Data corrupts material
Closed, Duplicate
Actions

Assigned To
None
Authored By
matc (matc)
May 20 2019, 8:15 PM
Tags
  • BF Blender
  • Grease Pencil
  • Dependency Graph
  • Tracker Curfew
Subscribers
Antonio Vazquez (antoniov)
Brecht Van Lommel (brecht)
matc (matc)
Philipp Oeser (lichtwerk)

Description

Blender Version
Broken: 9efe117535c6, master, 2018-05-20

Short description of error
For normal objects switching a material slot link to Object and back to Data will result in the material to reappear in the slot. For Grease Pencil objects the material in the slot will point to 0xcccc.... This causes blender to crash while rendering the viewport.

Exact steps for others to reproduce the error

  1. File -> New -> 2D Animation
  2. Switch Link of material slot to Object (Bottom right button in material slot selector.)
  3. Switch Link of same material slot back to Data
  4. Blender crashes on viewport render

Related Objects

Mentioned Here
T66293: GPencil: Redesign Blend modes and cleanup Drawing Engine
rB9efe117535c6: Normal UI: for all ops needing autosmooth on, enable it if needed.

Event Timeline

matc (matc) created this task.May 20 2019, 8:15 PM
Brecht Van Lommel (brecht) assigned this task to Antonio Vazquez (antoniov).May 29 2019, 2:45 AM
Brecht Van Lommel (brecht) lowered the priority of this task from 90 to 80.
Antonio Vazquez (antoniov) lowered the priority of this task from 80 to 50.May 29 2019, 9:54 AM
Antonio Vazquez (antoniov) added a subscriber: Brecht Van Lommel (brecht).EditedMay 29 2019, 10:36 AM

@Brecht Van Lommel (brecht) I have been doing debug and I have seen the problem is when retry the material.

The material is recovered using

give_current_material(ob, ob->actcol);

You can find the code in BKE_gpencil_object_material_get_from_brush()

IIRC you told me the function give_current_material() Is used to get the mat for Data and for Objet mode, but in this case, the function is returning NaN. I have checked and the Object parameter is correct and the ob->actcol=1, so not sure why this is not working... at least the function could return NULL intead of garbage data.

Any idea?

Antonio Vazquez (antoniov) added a comment.May 29 2019, 10:44 AM

I have seen this in the console, not sure if related or not:

find_node_operation: Failed for (MATERIAL_UPDATE, '')
add_relation(Material -> GP Data) - Could not find op_from (OperationKey(type: SHADING, component name: '', operation code: MATERIAL_UPDATE))
add_relation(Material -> GP Data) - Failed, but op_to (ComponentKey(GDStroke, GEOMETRY)) was ok
matc (matc) added a comment.Jun 1 2019, 12:43 AM

Could be that the problem is in DepsgraphNodeBuilder::build_object_data_geometry where only materials are handled which are returned by give_current_material. When setting a link to Object, then the material in ob->data is ignored. But I'm not sure how this wouldn't have caused problems with the default cube.

This would fix the problem:

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index 93e5dd14ae0..acdf08b7f32 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -1191,9 +1191,19 @@ void DepsgraphNodeBuilder::build_object_data_geometry(Object *object, bool is_ob
       function_bind(BKE_object_eval_uber_data, _1, scene_cow, object_cow));
   op_node->set_as_exit();
   /* Materials. */
-  if (object->totcol != 0) {
-    for (int a = 1; a <= object->totcol; a++) {
-      Material *ma = give_current_material(object, a);
+  if (object->totcol) {
+    for (int a = 0; a < object->totcol; a++) {
+      Material *ma = object->mat[a];
+      if (ma != NULL) {
+        build_material(ma);
+      }
+    }
+  }
+  short *totcolp = give_totcolp(object);
+  if (*totcolp) {
+    Material ***matarar = give_matarar(object);
+    for (int a = 0; a < *totcolp; a++) {
+      Material *ma = (*matarar)[a];
       if (ma != NULL) {
         build_material(ma);
       }
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index 54d5223497e..117b1fd6d19 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -1918,8 +1918,18 @@ void DepsgraphRelationBuilder::build_object_data_geometry(Object *object)
   }
   /* Materials. */
   if (object->totcol) {
-    for (int a = 1; a <= object->totcol; a++) {
-      Material *ma = give_current_material(object, a);
+    for (int a = 0; a < object->totcol; a++) {
+      Material *ma = object->mat[a];
+      if (ma != NULL) {
+        build_material(ma);
+      }
+    }
+  }
+  short *totcolp = give_totcolp(object);
+  if (*totcolp) {
+    Material ***matarar = give_matarar(object);
+    for (int a = 0; a < *totcolp; a++) {
+      Material *ma = (*matarar)[a];
       if (ma != NULL) {
         build_material(ma);
       }
Philipp Oeser (lichtwerk) added a project: Grease Pencil.Sep 13 2019, 1:23 PM
Antonio Vazquez (antoniov) added a subscriber: Philipp Oeser (lichtwerk).Sep 13 2019, 1:26 PM

@Philipp Oeser (lichtwerk) I don't know what to do here... maybe you need reassign to someone that knows how to fix it, because it looks something outside Gpencil code.

Philipp Oeser (lichtwerk) removed Antonio Vazquez (antoniov) as the assignee of this task.Sep 13 2019, 1:49 PM
Philipp Oeser (lichtwerk) added a project: Dependency Graph.
Philipp Oeser (lichtwerk) added a subscriber: Antonio Vazquez (antoniov).
Dalai Felinto (dfelinto) added a project: Tracker Curfew.Dec 23 2019, 4:34 PM
Antonio Vazquez (antoniov) closed this task as a duplicate of T63757: Grease Pencil Module.Jan 23 2020, 7:23 PM

Closed by error. This has been fixed in T66293