Fix a bunch of temp full-screen glitches

Steps to reproduce fixed glitches were:
* Change any editor to be file browser from menu, Ctrl+O *from the file browser area*, Esc -> area reset to what it was before changing to file browser initially
* Ctrl+O from any area, F12, Esc -> returns to initial editor in full-screen (expected is file browser in full-screen)

Fixes T46229

Core of the fix is removing old area from spacedata list when going back to previous area (see ED_area_prevspace -> BKE_spacedata_remove). Also, when creating a new temp area we now don't exit old area anymore (needed so SpaceFile->op is kept, but it also makes sense in general)
Aaand finally removes some ugly hacks.

Tested quite a bit, so I think it's safe to apply (besides of remark below), just would like to get things double checked and confirmed. After all, this full-screen stuff finally starts to feel like it's working :P

Note, there's still a memory leak when quitting Blender with temp area open. Haven't found out how to solve yet, but it's not that important for review anyway.

Reviewers: campbellbarton, brecht

Reviewed By: brecht

Subscribers: plyczkowski, Blendify

Maniphest Tasks: T46229

Differential Revision: https://developer.blender.org/D1531

9 files changed, 46 insertions, 39 deletions

diff --git a/source/blender/editors/include/ED_screen.h b/source/blender/editors/include/ED_screen.h
index df3a582d378..85370de0013 100644
--- a/source/blender/editors/include/ED_screen.h
+++ b/source/blender/editors/include/ED_screen.h
@@ -90,7 +90,7 @@ void    ED_area_tag_refresh(ScrArea *sa);
 void    ED_area_do_refresh(struct bContext *C, ScrArea *sa);
 void    ED_area_azones_update(ScrArea *sa, const int mouse_xy[]);
 void    ED_area_headerprint(ScrArea *sa, const char *str);
-void    ED_area_newspace(struct bContext *C, ScrArea *sa, int type);
+void    ED_area_newspace(struct bContext *C, ScrArea *sa, int type, const bool skip_ar_exit);
 void    ED_area_prevspace(struct bContext *C, ScrArea *sa);
 void    ED_area_swapspace(struct bContext *C, ScrArea *sa1, ScrArea *sa2);
 int     ED_area_headersize(void);
@@ -112,7 +112,7 @@ void    ED_screen_animation_timer(struct bContext *C, int redraws, int refresh,
 void    ED_screen_animation_timer_update(struct bScreen *screen, int redraws, int refresh);
 void    ED_screen_restore_temp_type(struct bContext *C, ScrArea *sa);
 ScrArea *ED_screen_full_newspace(struct bContext *C, ScrArea *sa, int type);
-void    ED_screen_full_prevspace(struct bContext *C, ScrArea *sa, const bool was_prev_temp);
+void    ED_screen_full_prevspace(struct bContext *C, ScrArea *sa);
 void    ED_screen_full_restore(struct bContext *C, ScrArea *sa);
 struct ScrArea *ED_screen_state_toggle(struct bContext *C, struct wmWindow *win, struct ScrArea *sa, const short state);
 void    ED_screens_header_tools_menu_create(struct bContext *C, struct uiLayout *layout, void *arg);
diff --git a/source/blender/editors/render/render_view.c b/source/blender/editors/render/render_view.c
index 4e36303d829..65f0fec50bc 100644
--- a/source/blender/editors/render/render_view.c
+++ b/source/blender/editors/render/render_view.c
@@ -193,7 +193,7 @@ ScrArea *render_view_open(bContext *C, int mx, int my, ReportList *reports)
 			/* find largest open non-image area */
 			sa = biggest_non_image_area(C);
 			if (sa) {
-				ED_area_newspace(C, sa, SPACE_IMAGE);
+				ED_area_newspace(C, sa, SPACE_IMAGE, true);
 				sima = sa->spacedata.first;
 
 				/* makes ESC go back to prev space */
@@ -265,7 +265,7 @@ static int render_view_cancel_exec(bContext *C, wmOperator *UNUSED(op))
 
 		if (sima->flag & SI_FULLWINDOW) {
 			sima->flag &= ~SI_FULLWINDOW;
-			ED_screen_full_prevspace(C, sa, false);
+			ED_screen_full_prevspace(C, sa);
 		}
 		else {
 			ED_area_prevspace(C, sa);
@@ -330,16 +330,10 @@ static int render_view_show_invoke(bContext *C, wmOperator *op, const wmEvent *e
 
 					if (sima->flag & SI_FULLWINDOW) {
 						sima->flag &= ~SI_FULLWINDOW;
-						ED_screen_full_prevspace(C, sa, false);
+						ED_screen_full_prevspace(C, sa);
 					}
-					else if (sima->next) {
-						/* workaround for case of double prevspace, render window
-						 * with a file browser on top of it (same as in ED_area_prevspace) */
-						if (sima->next->spacetype == SPACE_FILE && sima->next->next)
-							ED_area_newspace(C, sa, sima->next->next->spacetype);
-						else
-							ED_area_newspace(C, sa, sima->next->spacetype);
-						ED_area_tag_redraw(sa);
+					else {
+						ED_area_prevspace(C, sa);
 					}
 				}
 			}
diff --git a/source/blender/editors/screen/area.c b/source/blender/editors/screen/area.c
index 3d116d6d6a0..37847b0e640 100644
--- a/source/blender/editors/screen/area.c
+++ b/source/blender/editors/screen/area.c
@@ -1611,15 +1611,31 @@ void ED_area_swapspace(bContext *C, ScrArea *sa1, ScrArea *sa2)
 	ED_area_tag_refresh(sa2);
 }
 
-void ED_area_newspace(bContext *C, ScrArea *sa, int type)
+/**
+ * \param skip_ar_exit  Skip calling area exit callback. Set for opening temp spaces.
+ */
+void ED_area_newspace(bContext *C, ScrArea *sa, int type, const bool skip_ar_exit)
 {
 	if (sa->spacetype != type) {
 		SpaceType *st;
 		SpaceLink *slold;
 		SpaceLink *sl;
+		/* store sa->type->exit callback */
+		void *sa_exit = sa->type ? sa->type->exit : NULL;
+
+		/* in some cases (opening temp space) we don't want to
+		 * call area exit callback, so we temporarily unset it */
+		if (skip_ar_exit && sa->type) {
+			sa->type->exit = NULL;
+		}
 
 		ED_area_exit(C, sa);
 
+		/* restore old area exit callback */
+		if (skip_ar_exit && sa->type) {
+			sa->type->exit = sa_exit;
+		}
+
 		st = BKE_spacetype_from_id(type);
 		slold = sa->spacedata.first;
 
@@ -1686,12 +1702,12 @@ void ED_area_prevspace(bContext *C, ScrArea *sa)
 	SpaceLink *sl = sa->spacedata.first;
 
 	if (sl && sl->next) {
-		/* workaround for case of double prevspace, render window
-		 * with a file browser on top of it */
-		if (sl->next->spacetype == SPACE_FILE && sl->next->next)
-			ED_area_newspace(C, sa, sl->next->next->spacetype);
-		else
-			ED_area_newspace(C, sa, sl->next->spacetype);
+		ED_area_newspace(C, sa, sl->next->spacetype, false);
+
+		/* keep old spacedata but move it to end, so calling
+		 * ED_area_prevspace once more won't open it again */
+		BLI_remlink(&sa->spacedata, sl);
+		BLI_addtail(&sa->spacedata, sl);
 	}
 	else {
 		/* no change */
diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c
index fce2ef3eb2d..cb07ce756a6 100644
--- a/source/blender/editors/screen/screen_edit.c
+++ b/source/blender/editors/screen/screen_edit.c
@@ -1800,25 +1800,21 @@ ScrArea *ED_screen_full_newspace(bContext *C, ScrArea *sa, int type)
 		newsa->flag &= ~AREA_FLAG_TEMP_TYPE;
 	}
 
-	ED_area_newspace(C, newsa, type);
-	
+	ED_area_newspace(C, newsa, type, (newsa->flag & AREA_FLAG_TEMP_TYPE));
+
 	return newsa;
 }
 
 /**
  * \a was_prev_temp for the case previous space was a temporary fullscreen as well
  */
-void ED_screen_full_prevspace(bContext *C, ScrArea *sa, const bool was_prev_temp)
+void ED_screen_full_prevspace(bContext *C, ScrArea *sa)
 {
 	BLI_assert(sa->full);
 
 	if (sa->flag & AREA_FLAG_STACKED_FULLSCREEN) {
 		/* stacked fullscreen -> only go back to previous screen and don't toggle out of fullscreen */
 		ED_area_prevspace(C, sa);
-		/* only clear if previous space wasn't a temp fullscreen as well */
-		if (!was_prev_temp) {
-			sa->flag &= ~AREA_FLAG_TEMP_TYPE;
-		}
 	}
 	else {
 		ED_screen_restore_temp_type(C, sa);
@@ -1853,7 +1849,7 @@ void ED_screen_full_restore(bContext *C, ScrArea *sa)
 	
 	if (sl->next) {
 		if (sa->flag & AREA_FLAG_TEMP_TYPE) {
-			ED_screen_full_prevspace(C, sa, false);
+			ED_screen_full_prevspace(C, sa);
 		}
 		else {
 			ED_screen_state_toggle(C, win, sa, state);
@@ -1963,7 +1959,7 @@ ScrArea *ED_screen_state_toggle(bContext *C, wmWindow *win, ScrArea *sa, const s
 		if (state == SCREENMAXIMIZED) {
 			/* returns the top small area */
 			newa = area_split(sc, (ScrArea *)sc->areabase.first, 'h', 0.99f, 1);
-			ED_area_newspace(C, newa, SPACE_INFO);
+			ED_area_newspace(C, newa, SPACE_INFO, false);
 
 			/* copy area */
 			newa = newa->prev;
diff --git a/source/blender/editors/screen/screen_ops.c b/source/blender/editors/screen/screen_ops.c
index 7806038b313..d485a163bd8 100644
--- a/source/blender/editors/screen/screen_ops.c
+++ b/source/blender/editors/screen/screen_ops.c
@@ -3838,7 +3838,7 @@ static int fullscreen_back_exec(bContext *C, wmOperator *op)
 		return OPERATOR_CANCELLED;
 	}
 
-	ED_screen_full_prevspace(C, sa, false);
+	ED_screen_full_prevspace(C, sa);
 
 	return OPERATOR_FINISHED;
 }
diff --git a/source/blender/makesrna/intern/rna_screen.c b/source/blender/makesrna/intern/rna_screen.c
index 7487d840636..43d5cda17ae 100644
--- a/source/blender/makesrna/intern/rna_screen.c
+++ b/source/blender/makesrna/intern/rna_screen.c
@@ -161,7 +161,7 @@ static void rna_Area_type_update(bContext *C, PointerRNA *ptr)
 			CTX_wm_area_set(C, sa);
 			CTX_wm_region_set(C, NULL);
 
-			ED_area_newspace(C, sa, sa->butspacetype);
+			ED_area_newspace(C, sa, sa->butspacetype, true);
 			ED_area_tag_redraw(sa);
 
 			/* It is possible that new layers becomes visible. */
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 4239ccc1b69..24d4144ec17 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -1792,9 +1792,12 @@ static int wm_handler_fileselect_do(bContext *C, ListBase *handlers, wmEventHand
 				if (sa->prev) {
 					sa = sa->prev;
 				}
-				ED_area_newspace(C, sa, SPACE_FILE);     /* 'sa' is modified in-place */
+				ED_area_newspace(C, sa, SPACE_FILE, true);     /* 'sa' is modified in-place */
 				/* we already had a fullscreen here -> mark new space as a stacked fullscreen */
-				sa->flag |= AREA_FLAG_STACKED_FULLSCREEN;
+				sa->flag |= (AREA_FLAG_STACKED_FULLSCREEN | AREA_FLAG_TEMP_TYPE);
+			}
+			else if (sa->spacetype == SPACE_FILE) {
+				sa = ED_screen_state_toggle(C, CTX_wm_window(C), sa, SCREENMAXIMIZED);
 			}
 			else {
 				sa = ED_screen_full_newspace(C, sa, SPACE_FILE);    /* sets context */
@@ -1823,11 +1826,9 @@ static int wm_handler_fileselect_do(bContext *C, ListBase *handlers, wmEventHand
 
 			if (val != EVT_FILESELECT_EXTERNAL_CANCEL) {
 				ScrArea *sa = CTX_wm_area(C);
-				const SpaceLink *sl = sa->spacedata.first;
-				const bool was_prev_temp = (sl->next && sl->next->spacetype == SPACE_IMAGE);
 
 				if (sa->full) {
-					ED_screen_full_prevspace(C, sa, was_prev_temp);
+					ED_screen_full_prevspace(C, sa);
 				}
 				/* user may have left fullscreen */
 				else {
diff --git a/source/blender/windowmanager/intern/wm_window.c b/source/blender/windowmanager/intern/wm_window.c
index 932f9994a4a..688be21cdd0 100644
--- a/source/blender/windowmanager/intern/wm_window.c
+++ b/source/blender/windowmanager/intern/wm_window.c
@@ -678,10 +678,10 @@ wmWindow *WM_window_open_temp(bContext *C, const rcti *rect_init, int type)
 	CTX_wm_area_set(C, sa);
 	
 	if (type == WM_WINDOW_RENDER) {
-		ED_area_newspace(C, sa, SPACE_IMAGE);
+		ED_area_newspace(C, sa, SPACE_IMAGE, false);
 	}
 	else {
-		ED_area_newspace(C, sa, SPACE_USERPREF);
+		ED_area_newspace(C, sa, SPACE_USERPREF, false);
 	}
 	
 	ED_screen_set(C, win->screen);
diff --git a/source/blenderplayer/bad_level_call_stubs/stubs.c b/source/blenderplayer/bad_level_call_stubs/stubs.c
index b4493717739..354b485074f 100644
--- a/source/blenderplayer/bad_level_call_stubs/stubs.c
+++ b/source/blenderplayer/bad_level_call_stubs/stubs.c
@@ -425,7 +425,7 @@ void ED_space_image_get_zoom(struct SpaceImage *sima, struct ARegion *ar, float
 const char *ED_info_stats_string(struct Scene *scene) RET_NULL
 void ED_area_tag_redraw(struct ScrArea *sa) RET_NONE
 void ED_area_tag_refresh(struct ScrArea *sa) RET_NONE
-void ED_area_newspace(struct bContext *C, struct ScrArea *sa, int type) RET_NONE
+void ED_area_newspace(struct bContext *C, struct ScrArea *sa, int type, const bool skip_ar_exit) RET_NONE
 void ED_region_tag_redraw(struct ARegion *ar) RET_NONE
 void WM_event_add_fileselect(struct bContext *C, struct wmOperator *op) RET_NONE
 void WM_cursor_wait(bool val) RET_NONE