[Bf-blender-cvs] [b3f7eb4a078] modifier-panels-ui: Deduplicate code, fix crash moving panels quickly

Hans Goudey noreply at git.blender.org
Mon Apr 20 19:44:04 CEST 2020


Commit: b3f7eb4a07869359d421bf210622a9f5e2661a15
Author: Hans Goudey
Date:   Mon Apr 20 12:43:55 2020 -0500
Branches: modifier-panels-ui
https://developer.blender.org/rBb3f7eb4a07869359d421bf210622a9f5e2661a15

Deduplicate code, fix crash moving panels quickly

The method to check if the panel list represents data is now generalized.
Also actate EXIT panel state when freeing list panels when there is still
a handler attached.

===================================================================

M	source/blender/editors/include/UI_interface.h
M	source/blender/editors/interface/interface_panel.c
M	source/blender/editors/interface/interface_templates.c

===================================================================

diff --git a/source/blender/editors/include/UI_interface.h b/source/blender/editors/include/UI_interface.h
index 6540bcaa613..3a963a37c07 100644
--- a/source/blender/editors/include/UI_interface.h
+++ b/source/blender/editors/include/UI_interface.h
@@ -1694,19 +1694,26 @@ void UI_panel_category_draw_all(struct ARegion *region, const char *category_id_
 
 struct PanelType *UI_paneltype_find(int space_id, int region_id, const char *idname);
 
-/* Recreate list panels for representing a list. */
-struct Panel *UI_panel_add_recreate(struct ScrArea *sa,
-                                    struct ARegion *region,
-                                    struct ListBase *panels,
-                                    struct PanelType *panel_type,
-                                    int modifier_index);
-void UI_panel_set_list_index(struct Panel *panel, int i);
+/* Recreated list panels for representing a list. */
+struct Panel *UI_panel_add_list(struct ScrArea *sa,
+                                struct ARegion *region,
+                                struct ListBase *panels,
+                                struct PanelType *panel_type,
+                                int modifier_index);
 void UI_panel_delete(struct ARegion *region, struct ListBase *panels, struct Panel *panel);
-void UI_panels_free_recreate(struct ARegion *region);
+void UI_panels_free_list(struct bContext *C, struct ARegion *region);
+
 #define LIST_PANEL_UNIQUE_STR_LEN 4
 void UI_list_panel_unique_str(struct Panel *panel, char *r_name);
+
 void UI_panel_set_expand_from_list_data(const struct bContext *C, struct Panel *panel);
 
+typedef struct PanelType *(*uiListPanelTypeFromDataFunc)(struct ARegion *region,
+                                                         struct Link *data_link);
+bool UI_panel_list_matches_data(struct ARegion *region,
+                                struct ListBase *data,
+                                uiListPanelTypeFromDataFunc get_panel_type);
+
 /* Handlers
  *
  * Handlers that can be registered in regions, areas and windows for
diff --git a/source/blender/editors/interface/interface_panel.c b/source/blender/editors/interface/interface_panel.c
index 470aa905819..bd9e857fb55 100644
--- a/source/blender/editors/interface/interface_panel.c
+++ b/source/blender/editors/interface/interface_panel.c
@@ -311,7 +311,7 @@ static void get_panel_expand_flag(Panel *panel, short *flag, short *flag_index)
  *
  * \note This needs to iterate through all of the regions panels because the panel with changed
  * expansion could have been the subpanel of a list panel, meaning it might not know which
- * list item it corresponds to. HANS-TODO: Fix
+ * list item it corresponds to.
  */
 static void set_panels_list_data_expand_flag(const bContext *C, ARegion *region)
 {
@@ -559,7 +559,7 @@ static void ui_offset_panel_block(uiBlock *block)
  * Called in situations where panels need to be added dynamically rather than having only one panel
  * corresponding to each PanelType.
  */
-Panel *UI_panel_add_recreate(
+Panel *UI_panel_add_list(
     ScrArea *sa, ARegion *region, ListBase *panels, PanelType *panel_type, int list_index)
 {
   Panel *panel = MEM_callocN(sizeof(Panel), "list panel");
@@ -579,7 +579,7 @@ Panel *UI_panel_add_recreate(
    * function to creat them, as UI_panel_begin does other things we don't need to do. */
   for (LinkData *link = panel_type->children.first; link; link = link->next) {
     PanelType *child = link->data;
-    UI_panel_add_recreate(sa, region, &panel->children, child, list_index);
+    UI_panel_add_list(sa, region, &panel->children, child, list_index);
   }
 
   /* If we're adding a recreate list panel, make sure it's added to the end of the list. Check the
@@ -613,22 +613,6 @@ Panel *UI_panel_add_recreate(
   return panel;
 }
 
-void UI_panel_set_list_index(Panel *panel, int i)
-{
-  BLI_assert(panel->type != NULL);
-  if (panel->type->parent == NULL) {
-    BLI_assert(panel->type->flag & PNL_LIST);
-  }
-
-  panel->runtime.list_index = i;
-
-  Panel *child = panel->children.first;
-  while (child != NULL) {
-    UI_panel_set_list_index(child, i);
-    child = child->next;
-  }
-}
-
 void UI_list_panel_unique_str(Panel *panel, char *r_name)
 {
   snprintf(r_name, LIST_PANEL_UNIQUE_STR_LEN, "%d", panel->runtime.list_index);
@@ -646,7 +630,6 @@ static void panel_free_block(ARegion *region, Panel *panel)
 
   LISTBASE_FOREACH (uiBlock *, block, &region->uiblocks) {
     if (STREQ(block->name, block_name)) {
-      // printf("Removing panel block %s\n", block_name);
       BLI_remlink(&region->uiblocks, block);
       UI_block_free(NULL, block);
       break; /* Only delete one block for this panel. */
@@ -674,7 +657,7 @@ void UI_panel_delete(ARegion *region, ListBase *panels, Panel *panel)
   MEM_freeN(panel);
 }
 
-void UI_panels_free_recreate(ARegion *region)
+void UI_panels_free_list(bContext *C, ARegion *region)
 {
   /* Delete panels with the recreate flag. */
   ListBase *panels = &region->panels;
@@ -690,12 +673,63 @@ void UI_panels_free_recreate(ARegion *region)
 
     panel_next = panel->next;
     if (remove) {
+      if (panel->activedata != NULL) {
+        panel_activate_state(C, panel, PANEL_STATE_EXIT);
+      }
       UI_panel_delete(region, panels, panel);
     }
     panel = panel_next;
   }
 }
 
+/**
+ * Check if the list panels in the region's panel list corresponds to the list of data the panels
+ * represent. Returns false if the panels have been reordered or if the types from the list data
+ * don't match in any way.
+ *
+ * \param data: The list of data to check against the list panels.
+ * \param panel_type_func: Each type from the data list should have a corresponding panel type. For
+ * a mix of readabilty and generality, this can happen separately for each type of panel list.
+ */
+bool UI_panel_list_matches_data(ARegion *region,
+                                ListBase *data,
+                                uiListPanelTypeFromDataFunc panel_type_func)
+{
+  int data_len = BLI_listbase_count(data);
+  int i = 0;
+  Link *data_link = data->first;
+  Panel *panel = region->panels.first;
+  while (panel != NULL) {
+    if (panel->type != NULL && panel->type->flag & PNL_LIST) {
+      /* The panels were reordered by drag and drop. */
+      if (panel->flag & PNL_LIST_ORDER_CHANGED) {
+        return false;
+      }
+
+      /* We reached the last contraint before the last list panel. */
+      if (data_link == NULL) {
+        return false;
+      }
+
+      /* The types of the corresponding panel and constraint don't match. */
+      if (panel_type_func(region, data_link) != panel->type) {
+        return false;
+      }
+
+      data_link = data_link->next;
+      i++;
+    }
+    panel = panel->next;
+  }
+
+  /* If we didn't make it to the last list item, the panel list isn't complete. */
+  if (i != data_len) {
+    return false;
+  }
+
+  return true;
+}
+
 /**************************** drawing *******************************/
 
 /* triangle 'icon' for panel header */
@@ -2943,10 +2977,6 @@ static void panel_activate_state(const bContext *C, Panel *panel, uiHandlePanelS
   }
 
   if (state == PANEL_STATE_EXIT) {
-    // if (data->is_drag_drop) {
-    //   reorder_recreate_panel_list(C, region, panel);
-    // }
-
     MEM_freeN(data);
     panel->activedata = NULL;
 
diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c
index 27ef2265599..958399579d3 100644
--- a/source/blender/editors/interface/interface_templates.c
+++ b/source/blender/editors/interface/interface_templates.c
@@ -1821,9 +1821,11 @@ void uiTemplatePathBuilder(uiLayout *layout,
 
 #define ERROR_LIBDATA_MESSAGE TIP_("Can't edit external library data")
 
-static PanelType *panel_type_from_modifier_type(ARegion *region, ModifierType type)
+static PanelType *panel_type_from_modifier(ARegion *region, Link *md_link)
 {
   ARegionType *region_type = region->type;
+  ModifierData *md = (ModifierData *)md_link;
+  ModifierType type = md->type;
   const ModifierTypeInfo *mti = modifierType_getInfo(type);
 
   /* Get the name of the modifier's panel type which was defined when the panel was registered. */
@@ -1836,61 +1838,23 @@ static PanelType *panel_type_from_modifier_type(ARegion *region, ModifierType ty
 
 void uiTemplateModifiers(uiLayout *UNUSED(layout), bContext *C)
 {
-  /* HANS-TODO: Fix crash when panel's modifier type doesn't match. */
-
   ScrArea *sa = CTX_wm_area(C);
   ARegion *region = CTX_wm_region(C);
   Object *ob = CTX_data_active_object(C);
+  ListBase *modifiers = &ob->modifiers;
 
-  /* Check if the current modifier list corresponds to list panels, or if the panels were
-   * reordered. */
-  bool modifiers_changed = false;
-  int modifiers_len = BLI_listbase_count(&ob->modifiers);
-  int i = 0;
-  ModifierData *md = ob->modifiers.first;
-  Panel *panel = region->panels.first;
-  while (panel != NULL) {
-    if (panel->type != NULL && panel->type->flag & PNL_LIST) {
-      /* The panels were reordered by drag and drop. */
-      if (panel->flag & PNL_LIST_ORDER_CHANGED) {
-        modifiers_changed = true;
-        break;
-      }
-
-      /* We reached the last modifier before the last list panel. */
-      if (md == NULL) {
-        modifiers_changed = true;
-        break;
-      }
-
-      /* The types of the corresponding panel and modifier don't match. */
-      if (panel_type_from_modifier_type(region, md->type) != panel->type) {
-        modifiers_changed = true;
-        break;
-      }
-
-      md = md->next;
-      i++;
-    }
-    panel = panel->next;
-  }
+  bool panels_match = UI_panel_list_matches_data(region, modifiers, panel_type_from_modifier);
 
-  /* If we didn't make it to the last modifier, the panel list isn't complete. */
-  if (i != modifiers_len) {
-    modifiers_changed = true;
-  }
-
-  /* If the modifier list has changed at all, clear all of the list panels and rebuild them. */
-  if (modifiers_changed) {
-    UI_panels_free_recreate(region);
-    md = ob->modifiers.first;
-    for (i = 0; md; i++, md = md->next) {
+  if (!panels_match) {
+    UI_panels_free_list(C, region);
+    ModifierData *md = modifiers->first;
+    for (in

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list