Fix glitch updating manipulator after undo

Running undo would notify manipulators to refresh,
but this still allowed for events in the queue to be handled,
where manipulators could be drawn for selection before
their refresh callback runs.

This made Python manipulators raise exceptions
about referencing invalid data (or crash).

Now tag manipulator update on file load (including undo)
and ensure the refresh callback runs
before drawing manipulator selection.

Also split manipulator map refresh flag in two since selection doesn't
perform the same operations as regular drawing.

6 files changed, 61 insertions, 13 deletions

diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h
index 6ff344fea38..3cb78a427ab 100644
--- a/source/blender/blenkernel/BKE_screen.h
+++ b/source/blender/blenkernel/BKE_screen.h
@@ -294,6 +294,7 @@ void            BKE_area_region_free(struct SpaceType *st, struct ARegion *ar);
 void            BKE_screen_area_free(struct ScrArea *sa);
 /* Manipulator-maps of a region need to be freed with the region. Uses callback to avoid low-level call. */
 void BKE_region_callback_free_manipulatormap_set(void (*callback)(struct wmManipulatorMap *));
+void BKE_region_callback_refresh_tag_manipulatormap_set(void (*callback)(struct wmManipulatorMap *));
 
 struct ARegion *BKE_area_find_region_type(struct ScrArea *sa, int type);
 struct ARegion *BKE_area_find_region_active_win(struct ScrArea *sa);
@@ -309,6 +310,8 @@ unsigned int BKE_screen_view3d_layer_active(
 
 unsigned int BKE_screen_view3d_layer_all(const struct bScreen *sc) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1);
 
+void BKE_screen_manipulator_tag_refresh(struct bScreen *sc);
+
 void BKE_screen_view3d_sync(struct View3D *v3d, struct Scene *scene);
 void BKE_screen_view3d_scene_sync(struct bScreen *sc, struct Scene *scene);
 void BKE_screen_transform_orientation_remove(
diff --git a/source/blender/blenkernel/intern/blendfile.c b/source/blender/blenkernel/intern/blendfile.c
index 4de2433a89a..09760fcc4cb 100644
--- a/source/blender/blenkernel/intern/blendfile.c
+++ b/source/blender/blenkernel/intern/blendfile.c
@@ -218,6 +218,13 @@ static void setup_app_data(
 				BKE_screen_view3d_scene_sync(curscreen, curscene);
 			}
 		}
+
+		/* We need to tag this here because events may be handled immediately after.
+		 * only the current screen is important because we wont have to handle
+		 * events from multiple screens at once.*/
+		{
+			BKE_screen_manipulator_tag_refresh(curscreen);
+		}
 	}
 
 	/* free G.main Main database */
diff --git a/source/blender/blenkernel/intern/screen.c b/source/blender/blenkernel/intern/screen.c
index 237120b090d..a07abe166f0 100644
--- a/source/blender/blenkernel/intern/screen.c
+++ b/source/blender/blenkernel/intern/screen.c
@@ -292,6 +292,32 @@ void BKE_spacedata_id_unref(struct ScrArea *sa, struct SpaceLink *sl, struct ID
 	}
 }
 
+/**
+ * Avoid bad-level calls to #WM_manipulatormap_tag_refresh.
+ */
+static void (*region_refresh_tag_manipulatormap_callback)(struct wmManipulatorMap *) = NULL;
+
+void BKE_region_callback_refresh_tag_manipulatormap_set(void (*callback)(struct wmManipulatorMap *))
+{
+	region_refresh_tag_manipulatormap_callback = callback;
+}
+
+void BKE_screen_manipulator_tag_refresh(struct bScreen *sc)
+{
+	if (region_refresh_tag_manipulatormap_callback == NULL) {
+		return;
+	}
+
+	ScrArea *sa;
+	ARegion *ar;
+	for (sa = sc->areabase.first; sa; sa = sa->next) {
+		for (ar = sa->regionbase.first; ar; ar = ar->next) {
+			if (ar->manipulator_map != NULL) {
+				region_refresh_tag_manipulatormap_callback(ar->manipulator_map);
+			}
+		}
+	}
+}
 
 /**
  * Avoid bad-level calls to #WM_manipulatormap_delete.
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index b1e4b6a1d86..244f833e6b3 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -2197,6 +2197,10 @@ static int wm_handlers_do_intern(bContext *C, wmEvent *event, ListBase *handlers
 				wmManipulatorMap *mmap = handler->manipulator_map;
 				wmManipulator *mpr = wm_manipulatormap_highlight_get(mmap);
 
+				if (region->manipulator_map != handler->manipulator_map) {
+					WM_manipulatormap_tag_refresh(handler->manipulator_map);
+				}
+
 				wm_manipulatormap_handler_context(C, handler);
 				wm_region_mouse_co(C, event);
 
diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c
index a96ddb7368a..ec248c28f0f 100644
--- a/source/blender/windowmanager/intern/wm_init_exit.c
+++ b/source/blender/windowmanager/intern/wm_init_exit.c
@@ -173,6 +173,7 @@ void WM_init(bContext *C, int argc, const char **argv)
 	BKE_library_callback_free_window_manager_set(wm_close_and_free);   /* library.c */
 	BKE_library_callback_free_notifier_reference_set(WM_main_remove_notifier_reference);   /* library.c */
 	BKE_region_callback_free_manipulatormap_set(wm_manipulatormap_remove); /* screen.c */
+	BKE_region_callback_refresh_tag_manipulatormap_set(WM_manipulatormap_tag_refresh);
 	BKE_library_callback_remap_editor_id_reference_set(WM_main_remap_editor_id_reference);   /* library.c */
 	BKE_blender_callback_test_break_set(wm_window_testbreak); /* blender.c */
 	BKE_spacedata_callback_id_remap_set(ED_spacedata_id_remap); /* screen.c */
diff --git a/source/blender/windowmanager/manipulators/intern/wm_manipulator_map.c b/source/blender/windowmanager/manipulators/intern/wm_manipulator_map.c
index 1a52aa27d84..298a2b30c9e 100644
--- a/source/blender/windowmanager/manipulators/intern/wm_manipulator_map.c
+++ b/source/blender/windowmanager/manipulators/intern/wm_manipulator_map.c
@@ -75,8 +75,9 @@ static eWM_ManipulatorGroupTypeGlobalFlag wm_mmap_type_update_flag = 0;
  * Manipulator-map update tagging.
  */
 enum {
-	/* Tag manipulator-map for refresh. */
-	MANIPULATORMAP_REFRESH = (1 << 0),
+	/** #manipulatormap_prepare_drawing has run */
+	MANIPULATORMAP_IS_PREPARE_DRAW = (1 << 0),
+	MANIPULATORMAP_IS_REFRESH_CALLBACK = (1 << 1),
 };
 
 
@@ -166,7 +167,7 @@ wmManipulatorMap *WM_manipulatormap_new_from_type(
 
 	mmap = MEM_callocN(sizeof(wmManipulatorMap), "ManipulatorMap");
 	mmap->type = mmap_type;
-	mmap->update_flag = MANIPULATORMAP_REFRESH;
+	mmap->update_flag = MANIPULATORMAP_IS_PREPARE_DRAW | MANIPULATORMAP_IS_REFRESH_CALLBACK;
 
 	/* create all manipulator-groups for this manipulator-map. We may create an empty one
 	 * too in anticipation of manipulators from operators etc */
@@ -258,15 +259,12 @@ static GHash *WM_manipulatormap_manipulator_hash_new(
 void WM_manipulatormap_tag_refresh(wmManipulatorMap *mmap)
 {
 	if (mmap) {
-		mmap->update_flag |= MANIPULATORMAP_REFRESH;
+		mmap->update_flag |= (
+		        MANIPULATORMAP_IS_PREPARE_DRAW |
+		        MANIPULATORMAP_IS_REFRESH_CALLBACK);
 	}
 }
 
-static void manipulatormap_tag_updated(wmManipulatorMap *mmap)
-{
-	mmap->update_flag = 0;
-}
-
 static bool manipulator_prepare_drawing(
         wmManipulatorMap *mmap, wmManipulator *mpr,
         const bContext *C, ListBase *draw_manipulators)
@@ -278,7 +276,7 @@ static bool manipulator_prepare_drawing(
 	else {
 		if (do_draw & WM_MANIPULATOR_IS_VISIBLE_UPDATE) {
 			/* hover manipulators need updating, even if we don't draw them */
-			wm_manipulator_update(mpr, C, (mmap->update_flag & MANIPULATORMAP_REFRESH) != 0);
+			wm_manipulator_update(mpr, C, (mmap->update_flag & MANIPULATORMAP_IS_PREPARE_DRAW) != 0);
 		}
 		if (do_draw & WM_MANIPULATOR_IS_VISIBLE_DRAW) {
 			BLI_addhead(draw_manipulators, BLI_genericNodeN(mpr));
@@ -305,7 +303,7 @@ static void manipulatormap_prepare_drawing(
 	if (mpr_modal) {
 		if ((mpr_modal->parent_mgroup->type->flag & WM_MANIPULATORGROUPTYPE_DRAW_MODAL_ALL) == 0) {
 			if (manipulator_prepare_drawing(mmap, mpr_modal, C, draw_manipulators)) {
-				manipulatormap_tag_updated(mmap);
+				mmap->update_flag &= ~MANIPULATORMAP_IS_PREPARE_DRAW;
 			}
 			/* don't draw any other manipulators */
 			return;
@@ -324,8 +322,9 @@ static void manipulatormap_prepare_drawing(
 		wm_manipulatorgroup_ensure_initialized(mgroup, C);
 		/* update data if needed */
 		/* XXX weak: Manipulator-group may skip refreshing if it's invisible (map gets untagged nevertheless) */
-		if (mmap->update_flag & MANIPULATORMAP_REFRESH && mgroup->type->refresh) {
+		if ((mmap->update_flag & MANIPULATORMAP_IS_REFRESH_CALLBACK) && mgroup->type->refresh) {
 			mgroup->type->refresh(C, mgroup);
+			/* cleared below */
 		}
 		/* prepare drawing */
 		if (mgroup->type->draw_prepare) {
@@ -337,7 +336,9 @@ static void manipulatormap_prepare_drawing(
 		}
 	}
 
-	manipulatormap_tag_updated(mmap);
+	mmap->update_flag &=
+	        ~(MANIPULATORMAP_IS_REFRESH_CALLBACK |
+	          MANIPULATORMAP_IS_PREPARE_DRAW);
 }
 
 /**
@@ -550,6 +551,10 @@ wmManipulator *wm_manipulatormap_highlight_find(
 	for (wmManipulatorGroup *mgroup = mmap->groups.first; mgroup; mgroup = mgroup->next) {
 		if (wm_manipulatorgroup_is_visible(mgroup, C)) {
 			if (mgroup->type->flag & WM_MANIPULATORGROUPTYPE_3D) {
+				if ((mmap->update_flag & MANIPULATORMAP_IS_REFRESH_CALLBACK) && mgroup->type->refresh) {
+					mgroup->type->refresh(C, mgroup);
+					/* cleared below */
+				}
 				wm_manipulatorgroup_intersectable_manipulators_to_list(mgroup, &visible_3d_manipulators);
 			}
 			else if ((mpr = wm_manipulatorgroup_find_intersected_mainpulator(mgroup, C, event, r_part))) {
@@ -563,6 +568,8 @@ wmManipulator *wm_manipulatormap_highlight_find(
 		BLI_freelistN(&visible_3d_manipulators);
 	}
 
+	mmap->update_flag &= ~MANIPULATORMAP_IS_REFRESH_CALLBACK;
+
 	return mpr;
 }