Skip to content

Opencl per device configs#21138

Merged
TurboGit merged 3 commits into
darktable-org:masterfrom
jenshannoschwalm:opencl_device_configs
May 26, 2026
Merged

Opencl per device configs#21138
TurboGit merged 3 commits into
darktable-org:masterfrom
jenshannoschwalm:opencl_device_configs

Conversation

@jenshannoschwalm
Copy link
Copy Markdown
Collaborator

  1. Make OpenCL device read&write configs static and don't expose outside
  2. Avoid use of locale while reading/writing OpenCL device configs
    Instead of reading/writing floats we do it using integers and fiddle a bit using snprintf() and sscanf(). This is safe as we know the floating point representation of the data in the conf string and avoids fiddling with locales.

@TurboGit @da-phil the use of locales in this part of code is certainly part of the problem we have when initializing dt OpenCL so high priority.

@TurboGit in general we might need to look in depth into conf strings related to floats. If you look at the config file some are represented with dots, some with comma. Not sure if we always get the correct value if there is a mismatch.

@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes labels May 26, 2026
@jenshannoschwalm jenshannoschwalm added this to the 5.6 milestone May 26, 2026
@jenshannoschwalm jenshannoschwalm force-pushed the opencl_device_configs branch 2 times, most recently from 74992a0 to 99b6c94 Compare May 26, 2026 13:07
@jenshannoschwalm
Copy link
Copy Markdown
Collaborator Author

@TurboGit i just added another stability fix :-)

Comment thread src/common/opencl.c
@jdchristensen
Copy link
Copy Markdown

I know this isn't claiming to fully fix #21035 , but I tested it anyways, and I still get a crash when starting with an empty config folder. It crashed on the 6th time I tried it.

21138-crash1.txt

Instead of reading/writing floats we do it using integers and fiddle a bit using snprintf()
and sscanf().

This is safe as we know the floating point representation of the data in the conf string and
avoids fiddling with locales.

rwgtb
1. dt_conf_save() must only by used when closing dt, otherwise we might have unstable results
   - we have dt_conf_get_string_const() for example and that is clearly not threadsafe.
   So it became a static function and is not exposed any more.
2. It was only used at one other place so far, we wrote while detecting a new OpenCL device,
   which happens if you start with a fresh resource file or change hardware/driver.
   I introduced that writing a while ago when detecting & checking OpenCL devices was not as stable
   as it is now. Let's get rid of it - it was a bad idea.
@jenshannoschwalm jenshannoschwalm force-pushed the opencl_device_configs branch from 25c8ec3 to e5afa7b Compare May 26, 2026 19:34
@jenshannoschwalm
Copy link
Copy Markdown
Collaborator Author

I know this isn't claiming to fully fix #21035 ,

In no way it's claiming to fix that issue :-) Let's continue the discussion there.

Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit TurboGit merged commit df408d3 into darktable-org:master May 26, 2026
5 checks passed
@jenshannoschwalm jenshannoschwalm deleted the opencl_device_configs branch May 26, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants