Fixes bug in retouch module w/ blending on

[masterpiga]

The bug

The pixelpipe's fast blend cache (bcache) caches the focused module's process() output, so that tweaking blend parameters (opacity, blend mask…) can re-blend instantly without re-running process(). Its validity hash, _piece_process_hash, deliberately excludes the blend parameters — and with them, the drawn-mask group.

That's correct for ordinary modules, whose drawn mask is a blend mask applied after process(). But retouch is flagged IOP_FLAGS_NO_MASKS: its shapes are consumed inside process() — the spots drive clone/heal/blur/fill. So when you moved or reshaped a shape:

• The shape geometry changed → process() output should change.
• But the bcache hash (params only) did not change → the stale pre-edit buffer was copied straight back.
• Result: the UI says "working", but nothing changes

The path is active when blending is engaged, that's the gate (mask_mode != DISABLED) that turns the bcache on. The non-blending path was already fine: dt_iop_commit_params folds the spot geometry into the main module-output cache hash whenever the module is in focus.

The Fix [EDIT: revisited after Hanno's comment see below]

In _piece_process_hash, fold the form-group hash into the bcache validity hash for NO_MASKS modules — mirroring what dt_iop_commit_params already does for the main cache:

if((module->flags() & IOP_FLAGS_NO_MASKS) && piece->blendop_data)
{
  const dt_develop_blend_params_t *const bp = piece->blendop_data;
  dt_masks_form_t *grp = dt_masks_get_from_id(module->dev, bp->mask_id);
  if(grp)
    phash = dt_masks_group_hash(phash, grp);
}

Only retouch.c (and spots.c) use IOP_FLAGS_NO_MASKS. This keeps the bcache's fast-path for genuine blend-only tweaks (opacity etc. leave the forms unchanged, so the cache still hits), while shape moves/reshapes now correctly invalidate it. It covers both the CPU and OpenCL process paths, since both use this hash.

The Fix

Disable the fast-blend cache entirely for IOP_FLAGS_NO_MASKS modules by gating it off in _piece_fast_blend — the function that already collects all the cache-eligibility rules:

return dt_pipe_is_canvas(piece->pipe)
         && darktable.pipe_cache
         && dt_iop_has_focus(module)
         // IOP_FLAGS_NO_MASKS modules (retouch, spots) consume their drawn forms
         // inside process(), so the cached output depends on shape geometry that
         // the fast-blend hash ignores. Skip the fast path for them.
         && !(module->flags() & IOP_FLAGS_NO_MASKS)
         && _transform_for_blend(module, piece);

Only retouch.c and spots.c use IOP_FLAGS_NO_MASKS, so this is precisely scoped to the affected modules. Putting the decision here keeps all fast-cache eligibility rules in one place and means the bcache is never even allocated for these modules — rather than maintaining a validity hash whose only purpose is to force a miss. It covers both the CPU and OpenCL process paths, since both consult _piece_fast_blend.

The trade-off: blend-only tweaks (e.g. dragging opacity) on retouch/spots will re-run process() instead of just re-blending from cache. That's acceptable — these modules have no blend mask, so the fast path bought little, and correctness wins.

Without the fix

before.mp4

With the fix

after.mp4

Co-authored with Claude.

[masterpiga]

@TurboGit I set 5.6.1 as a milestone. This is not a fix for a 5.6-itnroduced bug, but it's still a fix nonetheless. It seems reasonable to me, but I am not sure if this is the right call.

[jenshannoschwalm]

1. The source comment "The blend cache deliberately ignores blending parameters so " is slightly misleading as those are only ignored for the testing module.
2. I think this would be better as the situation where we can use the fast cache are clearly visible

static inline gboolean _piece_fast_blend(const dt_dev_pixelpipe_iop_t *piece,
                                         const dt_iop_module_t *module)
{
  return dt_pipe_is_canvas(piece->pipe)
      && darktable.pipe_cache
      && module->dev
      && module->dev->gui_attached
      && module == module->dev->gui_module
      && !(module->flags() & IOP_FLAGS_NO_MASKS)
      && dt_dev_modulegroups_test_activated(darktable.develop)
      && _transform_for_blend(module, piece);
}

And yes, i agree this would be for 5.6.1

[masterpiga]

Thanks, @jenshannoschwalm! Updated accordingly :)

[jenshannoschwalm]

You might even concider to shorten this by using dt_iop_has_focus() for even more clarity ...

[masterpiga]

Done, thanks!

[TurboGit]

Thanks!

[TurboGit]

Need a release note, TIA.