Temporary fix for sequencer bugs #22925, #21429, #21783, #24165

* This fix is really only a bandage, as the underlying issue is that sequencer preview render doesn't yet use the job system.
* The sequencer preview can start a full render of the scene, and this can collide with other preview/actual renders in many cases.
* Drawing the sequencer preview is now disabled when an other render is in progress, but the sequence preview rendering could have already been started before the other render, so this doesn't really fix anything.
* For now only OpenGL rendering can be used for the sequencer preview reliably until it's reimplemented using the job system.
* Using the job system in the future can handle the clashes between different renders properly and will give users a nice progress bar to indicate something is happening while the preview is recalculated.

3 files changed, 46 insertions, 23 deletions

diff --git a/source/blender/blenkernel/intern/sequencer.c b/source/blender/blenkernel/intern/sequencer.c
index 5950a4630b7..53930cee68b 100644
--- a/source/blender/blenkernel/intern/sequencer.c
+++ b/source/blender/blenkernel/intern/sequencer.c
@@ -1801,7 +1801,8 @@ static ImBuf * seq_render_scene_strip_impl(
 	Object *oldcamera;
 	ListBase oldmarkers;
 	
-	/* Hack! This function can be called from do_render_seq(), in that case
+	/* Old info:
+	   Hack! This function can be called from do_render_seq(), in that case
 	   the seq->scene can already have a Render initialized with same name,
 	   so we have to use a default name. (compositor uses scene name to
 	   find render).
@@ -1813,9 +1814,27 @@ static ImBuf * seq_render_scene_strip_impl(
 	   and since G.rendering is uhm, gone... (Peter)
 	*/
 
+	/* New info:
+	   Using the same name for the renders works just fine as the do_render_seq()
+	   render is not used while the scene strips are rendered.
+	   
+	   However rendering from UI (through sequencer_preview_area_draw) can crash in
+	   very many cases since other renders (material preview, an actual render etc.)
+	   can be started while this sequence preview render is running. The only proper
+	   solution is to make the sequencer preview render a proper job, which can be
+	   stopped when needed. This would also give a nice progress bar for the preview
+	   space so that users know there's something happening.
+
+	   As a result the active scene now only uses OpenGL rendering for the sequencer
+	   preview. This is far from nice, but is the only way to prevent crashes at this
+	   time. 
+
+	   -jahka
+	*/
+
 	int rendering = G.rendering;
 	int doseq;
-	int doseq_gl= G.rendering ? /*(scene->r.seq_flag & R_SEQ_GL_REND)*/ 0 : (scene->r.seq_flag & R_SEQ_GL_PREV);
+	int doseq_gl= G.rendering ? /*(scene->r.seq_flag & R_SEQ_GL_REND)*/ 0 : /*(scene->r.seq_flag & R_SEQ_GL_PREV)*/ 1;
 	int have_seq= FALSE;
 	Scene *sce= seq->scene; /* dont refer to seq->scene above this point!, it can be NULL */
 	int sce_valid= FALSE;
@@ -1848,30 +1867,28 @@ static ImBuf * seq_render_scene_strip_impl(
 #endif
 	
 	if(sequencer_view3d_cb && BLI_thread_is_main() && doseq_gl && (seq->scene == scene || have_seq==0) && seq->scene->camera) {
+		/* for old scened this can be uninitialized, should probably be added to do_versions at some point if the functionality stays */
+		if(scene->r.seq_prev_type==0)
+			scene->r.seq_prev_type = 3 /* ==OB_SOLID */; 
+
 		/* opengl offscreen render */
 		scene_update_for_newframe(bmain, seq->scene, seq->scene->lay);
-		ibuf= sequencer_view3d_cb(seq->scene, seqrectx, seqrecty, IB_rect,
-					  scene->r.seq_prev_type);
+		ibuf= sequencer_view3d_cb(seq->scene, seqrectx, seqrecty, IB_rect, scene->r.seq_prev_type);
 	}
 	else {
-		Render *re;
+		Render *re = RE_GetRender(sce->id.name);
 		RenderResult rres;
-		
-		if(rendering)
-			re= RE_NewRender(" do_build_seq_ibuf");
-		/* If the top level scene that does the sequencer rendering is included 
-		 * as a strip the default render name for the strip will conflict with
-		 * the original render, so override the name in this case.
-		 * See bugs #22236 and #24160 for examples.
-		 * XXX: Somebody with deeper insight to the rendering pipeline should
-		 *      probably check if this is the best way to handle this. -jahka
-		 */
-		else if(seq->scene == scene)
-			re= RE_NewRender("scene_conflict_render");
-		else
-			re= RE_NewRender(sce->id.name);
-		
-		RE_BlenderFrame(re, bmain, sce, NULL, sce->lay, frame);
+
+		/* XXX: this if can be removed when sequence preview rendering uses the job system */
+		if(rendering || scene != sce) {
+			if(re==NULL)
+				re= RE_NewRender(sce->id.name);
+			
+			RE_BlenderFrame(re, bmain, sce, NULL, sce->lay, frame);
+
+			/* restore previous state after it was toggled on & off by RE_BlenderFrame */
+			G.rendering = rendering;
+		}
 		
 		RE_AcquireResultImage(re, &rres);
 		
diff --git a/source/blender/editors/space_sequencer/sequencer_draw.c b/source/blender/editors/space_sequencer/sequencer_draw.c
index c4ae658ce37..bbd18e7c1ee 100644
--- a/source/blender/editors/space_sequencer/sequencer_draw.c
+++ b/source/blender/editors/space_sequencer/sequencer_draw.c
@@ -728,6 +728,10 @@ void draw_image_seq(const bContext* C, Scene *scene, ARegion *ar, SpaceSeq *sseq
 	UI_view2d_totRect_set(v2d, viewrectx + 0.5f, viewrecty + 0.5f);
 	UI_view2d_curRect_validate(v2d);
 
+	/* only initialize the preview if a render is in progress */
+	if(G.rendering)
+		return;
+
 	if (special_seq_update)
 		ibuf= give_ibuf_seq_direct(bmain, scene, rectx, recty, cfra + frame_ofs, proxy_size, special_seq_update);
 	else if (!U.prefetchframes) // XXX || (G.f & G_PLAYANIM) == 0) {
diff --git a/source/blender/render/intern/source/pipeline.c b/source/blender/render/intern/source/pipeline.c
index 7230e0b3909..1feb43c04dd 100644
--- a/source/blender/render/intern/source/pipeline.c
+++ b/source/blender/render/intern/source/pipeline.c
@@ -2451,7 +2451,7 @@ static void do_render_seq(Render * re)
 {
 	static int recurs_depth = 0;
 	struct ImBuf *ibuf;
-	RenderResult *rr = re->result;
+	RenderResult *rr; /* don't assign re->result here as it might change during give_ibuf_seq */
 	int cfra = re->r.cfra;
 
 	re->i.cfra= cfra;
@@ -2463,9 +2463,11 @@ static void do_render_seq(Render * re)
 
 	recurs_depth++;
 
-	ibuf= give_ibuf_seq(re->main, re->scene, rr->rectx, rr->recty, cfra, 0, 100.0);
+	ibuf= give_ibuf_seq(re->main, re->scene, re->result->rectx, re->result->recty, cfra, 0, 100.0);
 
 	recurs_depth--;
+
+	rr = re->result;
 	
 	BLI_rw_mutex_lock(&re->resultmutex, THREAD_LOCK_WRITE);