Repository navigation
Conversation
modify_roi_in() turned OpenCL off for the module whenever the export upscaled, and process_cl() refused scale > 1, so an upscaling export copied the image back to the host for the last step. Nothing in dt_interpolation_resample_cl() is specific to downscaling: it uses the same resampling plan as the CPU code, and for an upscale that plan has fewer taps per pixel. Drop both guards. The CL_INVALID_WORK_GROUP_SIZE fallback in dt_iop_clip_and_zoom_roi_cl() still covers devices whose work group is too small for the vertical taps.
Without that having done and verified stability especiallly for (2) ... :-((( |
_default_process_tiling_cl_roi() multiplied the tile row by the row pitch in int before casting, so once a tile starts more than 2 GiB into the host buffer the offset wraps and the readback writes outside it. A 4x upscale of a 10 MP image to 15632x10408 (2.6 GB as float RGBA) crashed with OpenCL limited to 1 GB, on the first tile starting at row 8928. The CPU roi tiling already casts before multiplying.
Sync the upstream review fixes for darktable-org#22892, darktable-org#22900 and darktable-org#22901
|
What's clearly missing is a check for maximum image dimensions and maximum supported image size for the platform/device. |
You're right to ask, (2) wasn't safe. I've tested both now (Linux, PoCL). Darkroom with high quality processing on: fit, 100%, zoomed in to 110% and 121%, and panned at 121%. Finalscale upscales on CL0 in the full pipe, and the screenshots match OpenCL off within 1/255. Upscaled exports of a 10 MP raw at 2x, 3x and 4x (up to 15632x10408, 2.6 GB as float), with resourcelevel reference (2 GB of cl_mem, about what a 4 GB card gets at "default"), notebook (1 GB) and mini (200 MB):
The fix for that is #22912 (finalscale: place tiles of a scaled export where the image has them). It resamples each tile at its real offset, on CPU and OpenCL, and should go in before this PR. With both applied, tiled GPU upscales at 2 GB, 1 GB and 200 MB match the untiled export, on PoCL and on an M4 Pro with Apple OpenCL. |
dt_opencl_image_fits_device() is only asked about roi_in. With finalscale upscaling on OpenCL, an export wider or taller than the device's maximum image size (16384 on many devices) has an input that fits, so the module ran untiled, the output allocation failed and the pipe fell back to the CPU. Treat such an output as not fitting, so the module is tiled on the device instead.
| const gboolean out_fits = | ||
| roi_out->width <= darktable.opencl->dev[pipe->devid].max_image_width | ||
| && roi_out->height <= darktable.opencl->dev[pipe->devid].max_image_height; | ||
|
|
There was a problem hiding this comment.
I think we should also test for the dev max_mem_alloc here, no?
|
See comment in related PR too. There is one thing itching me. Many dt users still have graphics cards in the 4-8GB range, have sufficient ram and mostly have images > 30Mpixels. This could end up in OpenCL tiling as technically possible but still much slower as plain processing on CPU. Your benchmarking might be very misleading in such cases. |
A tiled upscale reads every output tile back to the host, which is no faster than the CPU path and slower over PCIe. Keep process_cl_ready FALSE for an upscale whose output exceeds the device's max image size or whose buffers don't fit the available cl_mem, so it runs on the CPU as before darktable-org#22901. Mirrors the fit check in finalscale-upscale-opencl (8c67c82).
|
Agreed, I measured it on an M4 Pro (Apple OpenCL, median of 3, whole export in ms, reference / notebook):
Untiled, the OpenCL upscale is faster (3x: finalscale 163 -> 113 ms). Once it has to tile, it's no faster than master's CPU upscale, and up to 1.4x slower with less memory. Every tile is read back to the host, and over PCIe that would only get worse. So finalscale now upscales on OpenCL only if the whole upscale fits the device untiled: output within the max image size, and in + out + interpolation within the available cl_mem. Otherwise process_cl_ready stays FALSE and it runs as on master. On PoCL with 2 GB, a 3x export of the 10 MP raw upscales on CL0, while with 1 GB, or at 17000 px wide, it stays on the CPU. All of these match the untiled CPU export (max 9/255). Something separate from this PR: after a large upscale, colorout tiles on OpenCL on master too, and costs 1.6-2.3x its CPU time (26 MP at 4x: 4.5 s vs 0.7 s). That's why no upscaled export in the table beats CPU only. Moved into #22912 as one commit, as you suggested there. Closing this one. |
Summary
When an export upscales,
modify_roi_in()turns OpenCL off for finalscale andprocess_cl()refuses scale > 1. The pipe then copies the image back to the host for the last step.dt_interpolation_resample_cl()has nothing specific to downscaling: it uses the same resampling plan as the CPU, and for an upscale that plan has fewer taps per pixel. This drops both guards. TheCL_INVALID_WORK_GROUP_SIZEfallback indt_iop_clip_and_zoom_roi_cl()still covers small work groups.Checklist
RELEASE_NOTES.mdentry was added (Performance Improvements).Test instructions
Export of mire1.cr2 with 0001-exposure's xmp,
--width 6000 --height 6000 --upscale true --hq true(6000x3994 out) and the integration runner's core options. The table shows the median of 3 runs on an M4 Pro, Apple OpenCL:Float output vs the CPU, same on PoCL and Apple: under 1e-5 everywhere except a few pixels around three demosaic outliers, which the upscale spreads. The outputs before the change have the same outliers. Tiling was forced with
--opencl-tilingon PoCL, with the same result.No integration test upscales, since they all export at 2048 px. Not tested: darkroom zoom above 100%, which goes through
dt_iop_clip_and_zoom_cl().AI assistance
Written with Claude Code. I checked it on PoCL and on my M4 Pro.