From f4e28d47622b4f4e95dcc308ac658b991ea489c4 Mon Sep 17 00:00:00 2001
From: Richard Antalik <richardantalik@gmail.com>
Date: Thu, 23 Nov 2023 04:29:09 +0100
Subject: [PATCH] Fix #114982: Crash when rendering scene strips

Since b1526dd2c6, viewport renderer sets `G.is_rendering`, but VSE scene
rendering function used this to decide whether to do offscreen opengl
render or use render API. This logic was quite weak.

Use `SeqRenderData::for_render` instead of `G.is_rendering`, since it
explicitly defines whether strip rendering is invoked by F12 render job.

Since offscreen gl rendering rely on `BLI_thread_is_main()` being
true, use this to initialize `do_seq_gl` variable for clarity.

The use of render job path was further conditioned on `G.is_rendering`,
with exception of running headless, but this condition was incorrect.
This condition was reformulated as precondition, which if true, returns
`nullptr` instead of crashing.

Pull Request: https://projects.blender.org/blender/blender/pulls/115079
---
 source/blender/sequencer/intern/render.cc | 59 ++++++++++++-----------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/source/blender/sequencer/intern/render.cc b/source/blender/sequencer/intern/render.cc
index 9fbfc3f282d..fb4fca8713f 100644
--- a/source/blender/sequencer/intern/render.cc
+++ b/source/blender/sequencer/intern/render.cc
@@ -1430,14 +1430,14 @@ static ImBuf *seq_render_scene_strip(const SeqRenderData *context,
    */
 
   const bool is_rendering = G.is_rendering;
-  const bool is_background = G.background;
-  const bool do_seq_gl = is_rendering ? false : (context->scene->r.seq_prev_type) != OB_RENDER;
+  bool do_seq_gl = !context->for_render && (context->scene->r.seq_prev_type) != OB_RENDER &&
+                   BLI_thread_is_main();
+
   bool have_comp = false;
   bool use_gpencil = true;
   /* do we need to re-evaluate the frame after rendering? */
   bool is_frame_update = false;
   Scene *scene;
-  int is_thread_main = BLI_thread_is_main();
 
   /* don't refer to seq->scene above this point!, it can be nullptr */
   if (seq->scene == nullptr) {
@@ -1498,7 +1498,7 @@ static ImBuf *seq_render_scene_strip(const SeqRenderData *context,
   is_frame_update = (orig_data.timeline_frame != scene->r.cfra) ||
                     (orig_data.subframe != scene->r.subframe);
 
-  if ((sequencer_view3d_fn && do_seq_gl && camera) && is_thread_main) {
+  if ((sequencer_view3d_fn && do_seq_gl && camera)) {
     char err_out[256] = "unknown";
     int width, height;
     BKE_render_resolution(&scene->r, false, &width, &height);
@@ -1544,37 +1544,40 @@ static ImBuf *seq_render_scene_strip(const SeqRenderData *context,
     const int totviews = BKE_scene_multiview_num_views_get(&scene->r);
     ImBuf **ibufs_arr;
 
-    ibufs_arr = static_cast<ImBuf **>(
-        MEM_callocN(sizeof(ImBuf *) * totviews, "Sequence Image Views Imbufs"));
-
-    /* XXX: this if can be removed when sequence preview rendering uses the job system
+    /*
+     * XXX: this if can be removed when sequence preview rendering uses the job system
      *
-     * disable rendered preview for sequencer while rendering -- it's very much possible
-     * that preview render will went into conflict with final render
+     * Disable rendered preview for sequencer while rendering - invoked render job will
+     * conflict with already running render
      *
      * When rendering from command line renderer is called from main thread, in this
      * case it's always safe to render scene here
      */
-    if (!is_thread_main || is_rendering == false || is_background || context->for_render) {
-      if (re == nullptr) {
-        re = RE_NewSceneRender(scene);
-      }
-
-      const float subframe = frame - floorf(frame);
-
-      RE_RenderFrame(re,
-                     context->bmain,
-                     scene,
-                     have_comp ? nullptr : view_layer,
-                     camera,
-                     floorf(frame),
-                     subframe,
-                     false);
-
-      /* restore previous state after it was toggled on & off by RE_RenderFrame */
-      G.is_rendering = is_rendering;
+    if (!context->for_render && (is_rendering && !G.background)) {
+      goto finally;
     }
 
+    ibufs_arr = static_cast<ImBuf **>(
+        MEM_callocN(sizeof(ImBuf *) * totviews, "Sequence Image Views Imbufs"));
+
+    if (re == nullptr) {
+      re = RE_NewSceneRender(scene);
+    }
+
+    const float subframe = frame - floorf(frame);
+
+    RE_RenderFrame(re,
+                   context->bmain,
+                   scene,
+                   have_comp ? nullptr : view_layer,
+                   camera,
+                   floorf(frame),
+                   subframe,
+                   false);
+
+    /* restore previous state after it was toggled on & off by RE_RenderFrame */
+    G.is_rendering = is_rendering;
+
     for (int view_id = 0; view_id < totviews; view_id++) {
       SeqRenderData localcontext = *context;
       RenderResult rres;