Three ways to increase pixelpipe UI responsiveness

[jenshannoschwalm]

EDIT according to latest commits: In short, this is all about how fast the UI responds to user actions while already processing the pipes, collected work ...

1. 030c6e0 fixes two bugs when processing the pipe, a) there was a missing "stop pixelpipe check" in CPU code path thus in CPU code the response to changes in history was delayed , b) in some cases we didn't write back input cl_mem to the pixelpipe cache. Thus we were not completely sure about valid input data represented in cache. That was the reason for the two cache invalidations @masterpiga spotted. My code comments were slightly wrong and i was not completeöy sure why they seemed necessary ... Now all is good and those invalidations could go with this commit.
2. 4e6297c removes pipe processing flag, introduces DT_DEV_PIXELPIPE_PROCESSING as a shutdown "mode" and makes callers aware of this
3. 7c5ad38 handles faster shutdown response for history changes if we are in the middle of a long diffuse process.
4. 426419b introduces a new shutdown mode reducing UI responsiveness for drags & zooms quite a lot.
5. ca0e461 removes pipe obsolete flag as not required any more since pipe cache is now fully implemented.

[jenshannoschwalm]

Latest force-push further adds:

1. Integrate pipe->processing into the shutdown modes
2. removal of pipe->obsolete

[kofa73]

Rebased on top of current master, it does not look good. :-(

I opened a huge file (160 MPx), zoomed in/out like crazy as soon as I entered the darkroom, kept enabling/disabling modules. I don't see errors in the log (-d cache -d pipe -d tiling -d opencl -d cache -d common -d perf -d verbose)
pr-21274-160mpx.zip

I tried disabling OpenCL, turned modules on/off, panned, zoomed -- did not help, the editor preview remained black.

[jenshannoschwalm]

Arrg, yes i saw that too and fixed it on my local working brach but forgot to update and force-push.

[kofa73]

No problem, thanks for the quick response!

I gave the changes to the bots, they came back with these (probably includes the stuff you fixed on your branch):

• src/develop/pixelpipe_hb.c:2495 — OpenCL input colorspace conversion is written back to the host input cache line without updating that line's cst descriptor, leaving converted pixels labelled with the pre-conversion colorspace (wrong colours / double conversion on cache reuse).

  • Evidence: the OpenCL path transforms cl_mem_input in place to cst_to updating only the local input_cst_cl, not input_format->cst; in the tiling fallback the converted cl_mem_input is copied back into the host input cache buffer and the prior input_format->cst = input_cst_cl; assignment was deleted, while valid_input_on_gpu_only is set FALSE so the later invalidation is skipped; _writeback_clinput copies device data back into the host cache buffer on module pipe stop but has no access to the cache descriptor and never updates cache->dsc / input_format->cst; the early-shutdown writeback paths (_module_pipe_stop at ~2407 and ~2490) return before the normal successful-copy descriptor update at ~2697; during the CL blend step cl_mem_input is transformed to blend_cst before the shutdown check then written back, so the cache line can hold blend-colorspace data under the original descriptor; cache_get returns the cached data with its stale descriptor, so a later hit applies the conversion a second time on already-converted pixels.

• src/develop/pixelpipe_hb.c:2337 — The blend-cache fast path was changed to gate on fast_blend instead of bcaching, so a candidate whose bcache_data is NULL or whose blend hash no longer matches is treated as a cache hit — copying stale/NULL host data to the GPU and skipping real module processing.

  • Evidence: bcaching is computed separately from fast_blend and additionally verifies pipe->bcache_data != NULL plus a matching hash, yet fast_blend can be true while phash differs from pipe->bcache_hash or while bcache_data is NULL; the direct OpenCL branch now checks fast_blend (not bcaching) and copies pipe->bcache_data into *cl_mem_output via dt_opencl_copy_host_to_image; module->process_cl() and the blend-cache refresh sit in the else branch, so they are skipped whenever fast_blend is true — and the non-tiled cache-refresh block guarded by if(fast_blend && err==CL_SUCCESS) is itself inside that else, making it unreachable on the non-tiled path.

• src/develop/pixelpipe_hb.c:2251 — When the tiled OpenCL fallback's full-image input pre-copy fails, the path invalidates only the input cache line and returns TRUE without setting pipe->opencl_error or invalidating the already-reserved output cache line, so stale/uninitialised output can be cached and the pipe reported VALID with no CPU fallback.

  • Evidence: even when fits_on_device is false the possible_cl path first tries to copy the full input ROI into an OpenCL image, recording failure only in success_opencl; the tiling branch handles !success_opencl by invalidating only the input cache line and returning TRUE, without setting pipe->opencl_error and without CPU fallback; the module output cache line was already reserved and assigned the current hash (dt_dev_pixelpipe_cache_get writes cache->hash[cline]=hash before processing fills the buffer); CPU restart happens only for err && pipe->opencl_error, so otherwise the GUI job sets pipe->status = DT_DEV_PIXELPIPE_VALID and the stale output line is returned as a cache HIT on the next request for the same hash.

[jenshannoschwalm]

I will check again ... thanks EDIT: and will split the first commit from the rest.