Darkroom: prevent useless recomputations of main preview when enterin…

…g the view

Darkrom `configure()` method is connected to `"configure-event"` event, which is called when initing the view AND upon window resizes events (through Gtk widget/window resize commands).

Problem is, at view init time, the final window size may not be correct just yet. It will be when we call `dt_ui_restore_panels()`, which will resize stuff properly.  That only happens when entering the current view.

Until we run `dt_dev_configure()`, main preview pipe gets output size -1×-1 px, which aborts the pipe recompute early. As soon as we init sizes with something "valid" with regard to the pipe, pipeline runs. Problem is it will not be valid with regard to the (final) window size and the output of the pipe will be thrown out, until we get the final size.

TD;DR: until we get the final window size, which happens only when entering the view, don't configure the main preview pipeline, which will disable useless recomputes.

src/develop/develop.c

@@ -228,16 +228,24 @@ void dt_dev_refresh_ui_images_real(dt_develop_t *dev)
   // which is handled everytime history is changed,
   // including when initing a new pipeline (from scratch or from user history).
   // Benefit is atomics are de-facto thread-safe.
-  if(dt_atomic_get_int(&dev->pipe->shutdown) && !dev->pipe->processing)
-  {
-    dt_atomic_set_int(&dev->pipe->shutdown, FALSE);
-    dt_dev_process_image(dev);
-  }
   if(dt_atomic_get_int(&dev->preview_pipe->shutdown) && !dev->preview_pipe->processing)
   {
     dt_atomic_set_int(&dev->preview_pipe->shutdown, FALSE);
     dt_dev_process_preview(dev);
   }
+  // When entering darkroom, the GUI will size itself and call the
+  // configure() method of the view, which calls dev_configure() below.
+  // Problem is the GUI can be glitchy and reconfigure the pipe twice with different sizes,
+  // Each one starting a recompute. The shutdown mechanism should take care of stopping
+  // an ongoing pipe which output we don't care about anymore.
+  // But just in case, always start with the preview pipe, hoping
+  // the GUI will have figured out what size it really wants when we start
+  // the main preview pipe.
+  if(dt_atomic_get_int(&dev->pipe->shutdown) && !dev->pipe->processing)
+  {
+    dt_atomic_set_int(&dev->pipe->shutdown, FALSE);
+    dt_dev_process_image(dev);
+  }
 }
 
 void dt_dev_pixelpipe_rebuild(dt_develop_t *dev)
@@ -390,6 +398,15 @@ void dt_dev_process_preview_job(dt_develop_t *dev)
 
 void dt_dev_process_image_job(dt_develop_t *dev)
 {
+  // -1×-1 px means the dimensions of the main preview in darkroom were not inited yet.
+  // 0×0 px is not feasible.
+  // Anything lower than 32 px might cause segfaults with blurs and local contrast.
+  // When the window size get inited, we will get a new order to recompute with a "zoom_changed" flag.
+  // Until then, don't bother computing garbage that will not be reused later.
+  if(dev->width < 32 || dev->height < 32) return;
+
+  dt_print(DT_DEBUG_DEV, "[pixelpipe] Started main preview recompute at %i×%i px\n", dev->width, dev->height);
+
   dt_pthread_mutex_lock(&dev->pipe->busy_mutex);
   dt_control_log_busy_enter();
   dt_control_toast_busy_enter();
@@ -593,27 +610,27 @@ int dt_dev_load_image(dt_develop_t *dev, const uint32_t imgid)
   return 0;
 }
 
-void dt_dev_configure(dt_develop_t *dev, int wd, int ht)
+void dt_dev_configure_real(dt_develop_t *dev, int wd, int ht)
 {
   // Called only from Darkroom to init and update drawing size
   // depending on sidebars and main window resizing.
   const int32_t tb = dev->border_size;
   wd -= 2*tb;
   ht -= 2*tb;
 
-  // Ensure we have non-zero image surface
-  wd = MAX(wd, 32);
-  ht = MAX(ht, 32);
-
-  if(dev->width != wd || dev->height != ht)
+  if(dev->width != wd || dev->height != ht || !dev->pipe->backbuf)
   {
+    // If dimensions didn't change or we don't have a valid output image to display
+
     dev->width = wd;
     dev->height = ht;
-    dt_dev_invalidate_zoom(dev);
+
+    dt_print(DT_DEBUG_DEV, "[pixelpipe] Darkroom requested a %i×%i px main preview\n", wd, ht);
 
     if(dev->image_storage.id > -1 && darktable.mipmap_cache)
     {
       // Only if it's not our initial configure call, aka if we already have an image
+      dt_dev_invalidate_zoom(dev);
       dt_control_queue_redraw_center();
       dt_dev_refresh_ui_images(dev);
     }

src/develop/develop.h

@@ -434,7 +434,8 @@ float dt_dev_get_zoom_scale(dt_develop_t *dev, dt_dev_zoom_t zoom, int closeup_f
 void dt_dev_get_pointer_zoom_pos(dt_develop_t *dev, const float px, const float py, float *zoom_x,
                                  float *zoom_y);
 
-void dt_dev_configure(dt_develop_t *dev, int wd, int ht);
+void dt_dev_configure_real(dt_develop_t *dev, int wd, int ht);
+#define dt_dev_configure(dev, wd, ht) DT_DEBUG_TRACE_WRAPPER(DT_DEBUG_DEV, dt_dev_configure_real, (dev), (wd), (ht))
 
 /*
  * exposure plugin hook, set the exposure and the black level

src/develop/pixelpipe_hb.c

@@ -2202,10 +2202,18 @@ static int dt_dev_pixelpipe_process_rec_and_backcopy(dt_dev_pixelpipe_t *pipe, d
   return ret;
 }
 
+#define KILL_SWITCH_PIPE                                                                                          \
+  if(dt_atomic_get_int(&pipe->shutdown))                                                                          \
+  {                                                                                                               \
+    pipe->processing = 0;                                                                                         \
+    return 1;                                                                                                     \
+  }
 
 int dt_dev_pixelpipe_process(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev, int x, int y, int width, int height,
                              float scale)
 {
+  KILL_SWITCH_PIPE
+
   pipe->processing = 1;
   pipe->opencl_enabled = dt_opencl_update_settings(); // update enabled flag and profile from preferences
   pipe->devid = (pipe->opencl_enabled) ? dt_opencl_lock_device(pipe->type)
@@ -2235,6 +2243,8 @@ int dt_dev_pixelpipe_process(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev, int x,
   GList *modules = g_list_last(pipe->iop);
   GList *pieces = g_list_last(pipe->nodes);
 
+  KILL_SWITCH_PIPE
+
 // re-entry point: in case of late opencl errors we start all over again with opencl-support disabled
 restart:;
   void *buf = NULL;
@@ -2243,11 +2253,15 @@ restart:;
   dt_iop_buffer_dsc_t _out_format = { 0 };
   dt_iop_buffer_dsc_t *out_format = &_out_format;
 
+  KILL_SWITCH_PIPE
+
   // Get the roi_out hash
   // Get the previous output size of the module, for cache invalidation.
   dt_dev_pixelpipe_get_roi_in(pipe, dev, roi);
   dt_pixelpipe_get_global_hash(pipe, dev);
 
+  KILL_SWITCH_PIPE
+
   // run pixelpipe recursively and get error status
   const int err =
     dt_dev_pixelpipe_process_rec_and_backcopy(pipe, dev, &buf, &cl_mem_out, &out_format, &roi, modules,

src/views/darkroom.c

@@ -1015,9 +1015,13 @@ static void _iso_12646_quickbutton_clicked(GtkWidget *w, gpointer user_data)
     d->border_size = DT_PIXEL_APPLY_DPI(dt_conf_get_int("plugins/darkroom/ui/border_size"));
   }
 
-  dt_dev_configure(d, d->width, d->height);
-
   dt_ui_restore_panels(darktable.gui->ui);
+
+  // The above _restore_panels() will resize the window
+  // which emits "configure-event" signal which execute here the configure() method
+  // which already calls dt_dev_configure(d, d->width, d->height)
+  // to reset pipeline output size for main preview.
+
 }
 
 /* overlay color */
@@ -3142,9 +3146,20 @@ static void change_slider_accel_precision(dt_action_t *action)
 void configure(dt_view_t *self, int wd, int ht)
 {
   dt_develop_t *dev = (dt_develop_t *)self->data;
-  dev->orig_width = wd;
-  dev->orig_height = ht;
-  dt_dev_configure(dev, wd, ht);
+
+  // Configure event is called when initing the view AND upon window resizes events (through Gtk widget/window resize commands).
+  // At init time, final window size may not be correct just yet.
+  // It will be when we call dt_ui_restore_panels(), which will resize stuff properly,
+  // but that will be only when entering the current view.
+  // Until we run dt_dev_configure(), main preview pipe gets output size -1×-1 px
+  // which aborts the pipe recompute early. As soon as we init
+  // sizes with something "valid" with regard to the pipe, pipeline runs.
+  // Problem is it will not be valid with regard to the window size and the output will be thrown out
+  // until we get the final size.
+  // TD;DR: until we get the final window size, which happens
+  // only when entering the view, don't configure the main preview pipeline, which will disable useless recomputes.
+  if(dt_view_manager_get_current_view(darktable.view_manager) == self)
+    dt_dev_configure(dev, wd, ht);
 }
 
 GSList *mouse_actions(const dt_view_t *self)