[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [32638] trunk/blender: Temporary fix for sequencer bugs #22925, #21429, #21783, #24165

[Janne Karhu]

Revision: 32638
          http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=32638
Author:   jhk
Date:     2010-10-21 19:00:38 +0200 (Thu, 21 Oct 2010)

Log Message:
-----------
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.

Modified Paths:
--------------
    trunk/blender/release/scripts/ui/space_sequencer.py
    trunk/blender/source/blender/blenkernel/intern/sequencer.c
    trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
    trunk/blender/source/blender/render/intern/source/pipeline.c

Modified: trunk/blender/release/scripts/ui/space_sequencer.py
===================================================================
--- trunk/blender/release/scripts/ui/space_sequencer.py	2010-10-21 15:02:39 UTC (rev 32637)
+++ trunk/blender/release/scripts/ui/space_sequencer.py	2010-10-21 17:00:38 UTC (rev 32638)
@@ -760,9 +760,10 @@
         render = context.scene.render
 
         col = layout.column()
+        col.active = False #Currently only opengl preview works!
         col.prop(render, "use_sequencer_gl_preview", text="Open GL Preview")
         col = layout.column()
-        col.active = render.use_sequencer_gl_preview
+        #col.active = render.use_sequencer_gl_preview
         col.prop(render, "sequencer_gl_preview", text="")
 
         '''

Modified: trunk/blender/source/blender/blenkernel/intern/sequencer.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/sequencer.c	2010-10-21 15:02:39 UTC (rev 32637)
+++ trunk/blender/source/blender/blenkernel/intern/sequencer.c	2010-10-21 17:00:38 UTC (rev 32638)
@@ -1801,7 +1801,8 @@
 	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 @@
 	   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,31 +1867,29 @@
 #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;
+
+		/* 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;
+		}
 		
-		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);
-		
 		RE_AcquireResultImage(re, &rres);
 		
 		if(rres.rectf) {

Modified: trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
===================================================================
--- trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c	2010-10-21 15:02:39 UTC (rev 32637)
+++ trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c	2010-10-21 17:00:38 UTC (rev 32638)
@@ -728,6 +728,10 @@
 	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) {

Modified: trunk/blender/source/blender/render/intern/source/pipeline.c
===================================================================
--- trunk/blender/source/blender/render/intern/source/pipeline.c	2010-10-21 15:02:39 UTC (rev 32637)
+++ trunk/blender/source/blender/render/intern/source/pipeline.c	2010-10-21 17:00:38 UTC (rev 32638)
@@ -2451,7 +2451,7 @@
 {
 	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 @@
 
 	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);