Fix #35979, #35937, #35739: undo crashes and missing updates with blender

internal viewport rendering. Lots of tweaks here, mainly:

* Stop 3D viewport render and free database before undo.
* Accumulate update flags rather than replace them each time it rerenders, to
  avoid previous updates getting lost.
* Don't check against Render struct view parameters for changes, those are set
  in the job thread which might not run before the next update call.

7 files changed, 189 insertions, 92 deletions

diff --git a/source/blender/editors/include/ED_render.h b/source/blender/editors/include/ED_render.h
index c1ddd9a6294..e8e7643164f 100644
--- a/source/blender/editors/include/ED_render.h
+++ b/source/blender/editors/include/ED_render.h
@@ -52,6 +52,8 @@ void ED_render_engine_changed(struct Main *bmain);
 void ED_render_engine_area_exit(struct ScrArea *sa);
 void ED_render_scene_update(struct Main *bmain, struct Scene *scene, int updated);
 
+void ED_viewport_render_kill_jobs(const struct bContext *C);
+
 /* render_preview.c */
 
 /* stores rendered preview  - is also used for icons */
diff --git a/source/blender/editors/render/render_intern.h b/source/blender/editors/render/render_intern.h
index edd61bfc5c3..eb09606e57e 100644
--- a/source/blender/editors/render/render_intern.h
+++ b/source/blender/editors/render/render_intern.h
@@ -83,7 +83,7 @@ void TEXTURE_OT_envmap_clear_all(struct wmOperatorType *ot);
 
 /* render_internal.c */
 void RENDER_OT_render(struct wmOperatorType *ot);
-void render_view3d(struct RenderEngine *engine, const struct bContext *C);
+void render_view3d_update(struct RenderEngine *engine, const struct bContext *C);
 void render_view3d_draw(struct RenderEngine *engine, const struct bContext *C);
 
 /* render_opengl.c uses this */
diff --git a/source/blender/editors/render/render_internal.c b/source/blender/editors/render/render_internal.c
index 03ccb2496a1..15aa67feefa 100644
--- a/source/blender/editors/render/render_internal.c
+++ b/source/blender/editors/render/render_internal.c
@@ -728,6 +728,7 @@ void RENDER_OT_render(wmOperatorType *ot)
 #define PR_UPDATE_VIEW				1
 #define PR_UPDATE_RENDERSIZE		2
 #define PR_UPDATE_MATERIAL			4
+#define PR_UPDATE_DATABASE			8
 
 typedef struct RenderPreview {
 	/* from wmJob */
@@ -744,8 +745,6 @@ typedef struct RenderPreview {
 	RenderEngine *engine;
 	
 	float viewmat[4][4];
-	
-	int keep_data;
 } RenderPreview;
 
 static int render_view3d_disprect(Scene *scene, ARegion *ar, View3D *v3d, RegionView3D *rv3d, rcti *disprect)
@@ -846,7 +845,13 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 	float clipsta, clipend, pixsize;
 	bool orth, restore = 0;
 	char name[32];
-		
+	int update_flag;
+
+	update_flag = rp->engine->job_update_flag;
+	rp->engine->job_update_flag = 0;
+
+	//printf("ma %d res %d view %d db %d\n", update_flag & PR_UPDATE_MATERIAL, update_flag & PR_UPDATE_RENDERSIZE, update_flag & PR_UPDATE_VIEW, update_flag & PR_UPDATE_DATABASE);
+
 	G.is_break = FALSE;
 	
 	if (false == render_view3d_get_rects(rp->ar, rp->v3d, rp->rv3d, &viewplane, rp->engine, &clipsta, &clipend, &pixsize, &orth))
@@ -861,13 +866,6 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 	sprintf(name, "View3dPreview %p", (void *)rp->ar);
 	re = rp->engine->re = RE_GetRender(name);
 	
-	if (rp->engine->re == NULL) {
-		
-		re = rp->engine->re = RE_NewRender(name);
-		
-		rp->keep_data = 0;
-	}
-	
 	/* set this always, rp is different for each job */
 	RE_test_break_cb(re, rp, render_view3d_break);
 	RE_display_draw_cb(re, rp, render_view3d_draw_update);
@@ -875,7 +873,7 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 	
 	rstats = RE_GetStats(re);
 
-	if (rp->keep_data == 0 || rstats->convertdone == 0 || (rp->keep_data & PR_UPDATE_RENDERSIZE)) {
+	if ((update_flag & (PR_UPDATE_RENDERSIZE|PR_UPDATE_DATABASE)) || rstats->convertdone == 0) {
 		/* no osa, blur, seq, layers, etc for preview render */
 		rdata = rp->scene->r;
 		rdata.mode &= ~(R_OSA | R_MBLUR | R_BORDER | R_PANORAMA);
@@ -901,11 +899,7 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 
 	RE_SetPixelSize(re, pixsize);
 	
-	/* database free can crash on a empty Render... */
-	if (rp->keep_data == 0 && rstats->convertdone)
-		RE_Database_Free(re);
-
-	if (rstats->convertdone == 0) {
+	if ((update_flag & PR_UPDATE_DATABASE) || rstats->convertdone == 0) {
 		unsigned int lay = rp->scene->lay;
 
 		/* allow localview render for objects with lights in normal layers */
@@ -917,12 +911,17 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 
 		/* copying blender data while main thread is locked, to avoid crashes */
 		WM_job_main_thread_lock_acquire(rp->job);
+		RE_Database_Free(re);
 		RE_Database_FromScene(re, rp->bmain, rp->scene, lay, 0);		// 0= dont use camera view
 		WM_job_main_thread_lock_release(rp->job);
 
 		/* do preprocessing like building raytree, shadows, volumes, SSS */
 		RE_Database_Preprocess(re);
 
+		/* conversion not completed, need to do it again */
+		if (!rstats->convertdone)
+			rp->engine->job_update_flag |= PR_UPDATE_DATABASE;
+
 		// printf("dbase update\n");
 	}
 	else {
@@ -940,8 +939,6 @@ static void render_view3d_startjob(void *customdata, short *stop, short *do_upda
 		/* always rotate back */
 		if (restore)
 			RE_DataBase_IncrementalView(re, rp->viewmat, 1);
-
-		rp->engine->flag &= ~RE_ENGINE_DO_UPDATE;
 	}
 }
 
@@ -952,16 +949,88 @@ static void render_view3d_free(void *customdata)
 	MEM_freeN(rp);
 }
 
-static void render_view3d_do(RenderEngine *engine, const bContext *C, int keep_data)
+static bool render_view3d_flag_changed(RenderEngine *engine, const bContext *C)
+{
+	RegionView3D *rv3d = CTX_wm_region_view3d(C);
+	View3D *v3d = CTX_wm_view3d(C);
+	ARegion *ar = CTX_wm_region(C);
+	Scene *scene = CTX_data_scene(C);
+	Render *re;
+	rctf viewplane;
+	rcti disprect;
+	float clipsta, clipend;
+	bool orth;
+	int job_update_flag = 0;
+	char name[32];
+	
+	/* ensure render engine exists */
+	re = engine->re;
+
+	if (!re) {
+		sprintf(name, "View3dPreview %p", (void *)ar);
+		re = engine->re = RE_GetRender(name);
+		if (!re)
+			re = engine->re = RE_NewRender(name);
+
+		engine->update_flag |= RE_ENGINE_UPDATE_DATABASE;
+	}
+
+	/* check update_flag */
+	if (engine->update_flag & RE_ENGINE_UPDATE_MA)
+		job_update_flag |= PR_UPDATE_MATERIAL;
+	
+	if (engine->update_flag & RE_ENGINE_UPDATE_OTHER)
+		job_update_flag |= PR_UPDATE_MATERIAL;
+	
+	if (engine->update_flag & RE_ENGINE_UPDATE_DATABASE)
+		job_update_flag |= PR_UPDATE_DATABASE;
+	
+	engine->update_flag = 0;
+	
+	/* check if viewport changed */
+	if (engine->last_winx != ar->winx || engine->last_winy != ar->winy) {
+		engine->last_winx = ar->winx;
+		engine->last_winy = ar->winy;
+		job_update_flag |= PR_UPDATE_RENDERSIZE;
+	}
+
+	if (compare_m4m4(engine->last_viewmat, rv3d->viewmat, 0.00001f) == 0) {
+		copy_m4_m4(engine->last_viewmat, rv3d->viewmat);
+		job_update_flag |= PR_UPDATE_VIEW;
+	}
+	
+	render_view3d_get_rects(ar, v3d, rv3d, &viewplane, engine, &clipsta, &clipend, NULL, &orth);
+	
+	if (BLI_rctf_compare(&viewplane, &engine->last_viewplane, 0.00001f) == 0) {
+		engine->last_viewplane = viewplane;
+		job_update_flag |= PR_UPDATE_VIEW;
+	}
+	
+	render_view3d_disprect(scene, ar, v3d, rv3d, &disprect);
+	if (BLI_rcti_compare(&disprect, &engine->last_disprect) == 0) {
+		engine->last_disprect = disprect;
+		job_update_flag |= PR_UPDATE_RENDERSIZE;
+	}
+
+	/* any changes? go ahead and rerender */
+	if (job_update_flag) {
+		engine->job_update_flag |= job_update_flag;
+		return true;
+	}
+
+	return false;
+}
+
+static void render_view3d_do(RenderEngine *engine, const bContext *C)
 {
 	wmJob *wm_job;
 	RenderPreview *rp;
 	Scene *scene = CTX_data_scene(C);
 	
-	if (CTX_wm_window(C) == NULL) {
-		engine->flag |= RE_ENGINE_DO_UPDATE;
+	if (CTX_wm_window(C) == NULL)
+		return;
+	if (!render_view3d_flag_changed(engine, C))
 		return;
-	}
 
 	wm_job = WM_jobs_get(CTX_wm_manager(C), CTX_wm_window(C), CTX_wm_region(C), "Render Preview",
 	                     WM_JOB_EXCL_RENDER, WM_JOB_TYPE_RENDER_PREVIEW);
@@ -976,7 +1045,6 @@ static void render_view3d_do(RenderEngine *engine, const bContext *C, int keep_d
 	rp->v3d = rp->sa->spacedata.first;
 	rp->rv3d = CTX_wm_region_view3d(C);
 	rp->bmain = CTX_data_main(C);
-	rp->keep_data = keep_data;
 	copy_m4_m4(rp->viewmat, rp->rv3d->viewmat);
 	
 	/* dont alloc in threads */
@@ -991,80 +1059,33 @@ static void render_view3d_do(RenderEngine *engine, const bContext *C, int keep_d
 	WM_jobs_start(CTX_wm_manager(C), wm_job);
 	
 	engine->flag &= ~RE_ENGINE_DO_UPDATE;
-
 }
 
 /* callback for render engine , on changes */
-void render_view3d(RenderEngine *engine, const bContext *C)
+void render_view3d_update(RenderEngine *engine, const bContext *C)
 {	
-	render_view3d_do(engine, C, 0);
-}
-
-static int render_view3d_changed(RenderEngine *engine, const bContext *C)
-{
-	ARegion *ar = CTX_wm_region(C);
-	Render *re;
-	int update = 0;
-	char name[32];
-	
-	sprintf(name, "View3dPreview %p", (void *)ar);
-	re = RE_GetRender(name);
+	/* this shouldn't be needed and causes too many database rebuilds, but we
+	 * aren't actually tracking updates for all relevent datablocks so this is
+	 * a catch-all for updates */
+	engine->update_flag |= RE_ENGINE_UPDATE_DATABASE;
 
-	if (re) {
-		RegionView3D *rv3d = CTX_wm_region_view3d(C);
-		View3D *v3d = CTX_wm_view3d(C);
-		Scene *scene = CTX_data_scene(C);
-		rctf viewplane, viewplane1;
-		rcti disprect, disprect1;
-		float mat[4][4];
-		float clipsta, clipend;
-		bool orth;
-
-		if (engine->update_flag & RE_ENGINE_UPDATE_MA)
-			update |= PR_UPDATE_MATERIAL;
-		
-		if (engine->update_flag & RE_ENGINE_UPDATE_OTHER)
-			update |= PR_UPDATE_MATERIAL;
-		
-		engine->update_flag = 0;
-		
-		if (engine->resolution_x != ar->winx || engine->resolution_y != ar->winy)
-			update |= PR_UPDATE_RENDERSIZE;
-
-		RE_GetView(re, mat);
-		if (compare_m4m4(mat, rv3d->viewmat, 0.00001f) == 0) {
-			update |= PR_UPDATE_VIEW;
-		}
-		
-		render_view3d_get_rects(ar, v3d, rv3d, &viewplane, engine, &clipsta, &clipend, NULL, &orth);
-		RE_GetViewPlane(re, &viewplane1, &disprect1);
-		
-		if (BLI_rctf_compare(&viewplane, &viewplane1, 0.00001f) == 0)
-			update |= PR_UPDATE_VIEW;
-		
-		render_view3d_disprect(scene, ar, v3d, rv3d, &disprect);
-		if (BLI_rcti_compare(&disprect, &disprect1) == 0)
-			update |= PR_UPDATE_RENDERSIZE;
-		
-		if (update)
-			engine->flag |= RE_ENGINE_DO_UPDATE;
-		//if (update)
-		//	printf("changed ma %d res %d view %d\n", update & PR_UPDATE_MATERIAL, update & PR_UPDATE_RENDERSIZE, update & PR_UPDATE_VIEW);
-	}
-	
-	return update;
+	render_view3d_do(engine, C);
 }
 
 void render_view3d_draw(RenderEngine *engine, const bContext *C)
 {
 	Render *re = engine->re;
 	RenderResult rres;
-	int keep_data = render_view3d_changed(engine, C);
+	char name[32];
 	
-	if (engine->flag & RE_ENGINE_DO_UPDATE)
-		render_view3d_do(engine, C, keep_data);
-
-	if (re == NULL) return;
+	render_view3d_do(engine, C);
+	
+	if (re == NULL) {
+		sprintf(name, "View3dPreview %p", (void *)CTX_wm_region(C));
+		re = RE_GetRender(name);
+	
+		if (re == NULL) return;
+	}
 	
 	RE_AcquireResultImage(re, &rres);
 	
@@ -1114,3 +1135,52 @@ void render_view3d_draw(RenderEngine *engine, const bContext *C)
 
 	RE_ReleaseResultImage(re);
 }
+
+void ED_viewport_render_kill_jobs(const bContext *C)
+{
+	wmWindowManager *wm = CTX_wm_manager(C);
+	Main *bmain = CTX_data_main(C);
+	bScreen *sc;
+	ScrArea *sa;
+	ARegion *ar;
+
+	if (!wm)
+		return;
+
+	/* kill all actively running jobs */
+	WM_jobs_kill(wm, NULL, render_view3d_startjob);
+
+	/* loop over 3D view render engines */
+	for (sc = bmain->screen.first; sc; sc = sc->id.next) {
+		for (sa = sc->areabase.first; sa; sa = sa->next) {
+			if (sa->spacetype != SPACE_VIEW3D)
+				continue;
+			
+			for (ar = sa->regionbase.first; ar; ar = ar->next) {
+				RegionView3D *rv3d;
+				
+				if (ar->regiontype != RGN_TYPE_WINDOW)
+					continue;
+				
+				rv3d = ar->regiondata;
+
+				if (rv3d->render_engine) {
+					/* free render database now before we change data, because
+					 * RE_Database_Free will also loop over blender data */
+					char name[32];
+					Render *re;
+
+					sprintf(name, "View3dPreview %p", (void *)ar);
+					re = RE_GetRender(name);
+
+					if (re)
+						RE_Database_Free(re);
+
+					/* tag render engine to update entire database */
+					rv3d->render_engine->update_flag |= RE_ENGINE_UPDATE_DATABASE;
+				}
+			}
+		}
+	}
+}
+
diff --git a/source/blender/editors/render/render_update.c b/source/blender/editors/render/render_update.c
index 436aef943e4..5f74bf6576a 100644
--- a/source/blender/editors/render/render_update.c
+++ b/source/blender/editors/render/render_update.c
@@ -539,7 +539,7 @@ void ED_render_internal_init(void)
 {
 	RenderEngineType *ret = RE_engines_find("BLENDER_RENDER");
 	
-	ret->view_update = render_view3d;
+	ret->view_update = render_view3d_update;
 	ret->view_draw = render_view3d_draw;
 	
 }
diff --git a/source/blender/editors/util/undo.c b/source/blender/editors/util/undo.c
index 7f2e5b4b81c..52f87c19dc8 100644
--- a/source/blender/editors/util/undo.c
+++ b/source/blender/editors/util/undo.c
@@ -59,6 +59,7 @@
 #include "ED_mball.h"
 #include "ED_mesh.h"
 #include "ED_object.h"
+#include "ED_render.h"
 #include "ED_screen.h"
 #include "ED_sculpt.h"
 #include "ED_util.h"
@@ -140,9 +141,12 @@ static int ed_undo_step(bContext *C, int step, const char *undoname)
 		SpaceImage *sima = (SpaceImage *)sa->spacedata.first;
 		
 		if ((obact && (obact->mode & OB_MODE_TEXTURE_PAINT)) || (sima->mode == SI_MODE_PAINT)) {
-			if (!ED_undo_paint_step(C, UNDO_PAINT_IMAGE, step, undoname) && undoname)
-				if (U.uiflag & USER_GLOBALUNDO)
+			if (!ED_undo_paint_step(C, UNDO_PAINT_IMAGE, step, undoname) && undoname) {
+				if (U.uiflag & USER_GLOBALUNDO) {
+					ED_viewport_render_kill_jobs(C);
 					BKE_undo_name(C, undoname);
+				}
+			}
 			
 			WM_event_add_notifier(C, NC_WINDOW, NULL);
 			return OPERATOR_FINISHED;
@@ -192,6 +196,8 @@ static int ed_undo_step(bContext *C, int step, const char *undoname)
 			/* for example, texface stores image pointers */
 			undo_editmode_clear();
 			
+			ED_viewport_render_kill_jobs(C);
+
 			if (undoname)
 				BKE_undo_name(C, undoname);
 			else
@@ -363,6 +369,8 @@ int ED_undo_operator_repeat(bContext *C, struct wmOperator *op)
 		{
 			int retval;
 
+			ED_viewport_render_kill_jobs(C);
+
 			if (G.debug & G_DEBUG)
 				printf("redo_cb: operator redo %s\n", op->type->name);
 			ED_undo_pop_op(C, op);
@@ -529,6 +537,7 @@ static int undo_history_exec(bContext *C, wmOperator *op)
 			WM_event_add_notifier(C, NC_GEOM | ND_DATA, NULL);
 		}
 		else {
+			ED_viewport_render_kill_jobs(C);
 			BKE_undo_number(C, item);
 			WM_event_add_notifier(C, NC_SCENE | ND_LAYER_CONTENT, CTX_data_scene(C));
 		}
diff --git a/source/blender/render/extern/include/RE_engine.h b/source/blender/render/extern/include/RE_engine.h
index b6c3668512b..786c67f5305 100644
--- a/source/blender/render/extern/include/RE_engine.h
+++ b/source/blender/render/extern/include/RE_engine.h
@@ -70,6 +70,7 @@ struct Scene;
 /* RenderEngine.update_flag, used by internal now */
 #define RE_ENGINE_UPDATE_MA			1
 #define RE_ENGINE_UPDATE_OTHER		2
+#define RE_ENGINE_UPDATE_DATABASE	4
 
 extern ListBase R_engines;
 
@@ -97,7 +98,7 @@ typedef struct RenderEngine {
 	RenderEngineType *type;
 	void *py_instance;
 
-	int flag, update_flag;
+	int flag;
 	struct Object *camera_override;
 
 	int tile_x;
@@ -110,6 +111,15 @@ typedef struct RenderEngine {
 	int resolution_x, resolution_y;
 
 	struct ReportList *reports;
+
+	/* for blender internal only */
+	int update_flag;
+	int job_update_flag;
+
+	rctf last_viewplane;
+	rcti last_disprect;
+	float last_viewmat[4][4];
+	int last_winx, last_winy;
 } RenderEngine;
 
 RenderEngine *RE_engine_create(RenderEngineType *type);
diff --git a/source/blender/render/intern/source/convertblender.c b/source/blender/render/intern/source/convertblender.c
index bd2d292f633..ce71ab188a9 100644
--- a/source/blender/render/intern/source/convertblender.c
+++ b/source/blender/render/intern/source/convertblender.c
@@ -4863,7 +4863,11 @@ static void init_render_object(Render *re, Object *ob, Object *par, DupliObject
 void RE_Database_Free(Render *re)
 {
 	LampRen *lar;
-	
+
+	/* will crash if we try to free empty database */
+	if (!re->i.convertdone)
+		return;
+
 	/* statistics for debugging render memory usage */
 	if ((G.debug & G_DEBUG) && (G.is_rendering)) {
 		if ((re->r.scemode & (R_BUTS_PREVIEW|R_VIEWPORT_PREVIEW))==0) {
@@ -5399,8 +5403,10 @@ void RE_Database_Preprocess(Render *re)
 				volume_precache(re);
 	}
 	
-	if (re->test_break(re->tbh))
+	if (re->test_break(re->tbh)) {
+		re->i.convertdone = TRUE;
 		RE_Database_Free(re);
+	}
 	else
 		re->i.convertdone = TRUE;