darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.5k stars 1.12k forks source link

Jz masks #17178

Open kofa73 opened 1 month ago

kofa73 commented 1 month ago

Describe the bug

In some modules (perhaps those not native to the scene-referred workflow?) the Jz picker picks outlandish values, which actually don't select anything in the image.

Steps to reproduce

  1. Get the files from https://tech.kovacs-telekes.org/files/2024-07-21-darktable-Jz-masks/
  2. Activate the exposure instance called no-op, masking only.
  3. Select parametric masking based on the Jz channel, and pick an area from the flower. You'll get values like this, and the mask will cover the flower: exposure-Jz-mask
  4. Now enable the contrast equalizer instance. Confirm that it's set to scene-referred working space.
  5. Try to repeat picking on the Jz channel. I got this: contrastEq-Jzg-picking
  6. The resulting mask does not actually cover anything in the image: contrastEq-Jzg-mask-display
  7. Manually copying the values from the no-op exposure instance selects the expected part of the image: contrastEq-Jzg-with-values-from-exposure

OpenCL on/off does not influence the behaviour.

Expected behavior

Jz mask should work for all modules

Logfile | Screenshot | Screencast

No response

Commit

No response

Where did you obtain darktable from?

self compiled

darktable version

4.9.0+72~g517889efe0

What OS are you using?

Linux

What is the version of your OS?

Ubuntu 24.04

Describe your system?

No response

Are you using OpenCL GPU in darktable?

None

If yes, what is the GPU card and driver?

No response

Please provide additional context if applicable. You can attach files too, but might need to rename to .txt or .zip

No response

jenshannoschwalm commented 1 month ago

Not sure if this was working in 4.6 ?

kofa73 commented 1 month ago

I don't know, either. I'll try to test. Edit: no, it wasn't. (The 'dirty' marker is due to LibRaw & rawspeed.)

image

jenshannoschwalm commented 1 month ago

So no 4.8 regression :-)

Will get into this issue after the pending PR's about colorspace conversions got merged.

jenshannoschwalm commented 1 month ago

Ok, we simply miss picker color space conversions for lab space modules. Are you interested to code? :-)

kofa73 commented 1 month ago

In theory, yes. I guess it's not too urgent. Is it (missing from) color_picker.c? Something to do with the message [colorpicker] unknown colorspace conversion from 1 to 5

void dt_color_picker_helper(...)
{
    ...
    else
    {
      // fallback, but this shouldn't happen
      dt_print(DT_DEBUG_ALWAYS,
               "[colorpicker] unknown colorspace conversion from %d to %d\n",
               image_cst, picker_cst);
      _color_picker_work_4ch(source, roi, box, pick, NULL, _color_picker_rgb_or_lab, 100);
    }
    ...
kofa73 commented 1 month ago

That would be a new else if, but what goes inside?

    else if(effective_cst == IOP_CS_JZCZHZ && picker_cst == IOP_CS_LAB)
    {
      _color_picker_work_4ch(source, roi, box, pick, NULL, _some_new_color_picker_conversion_function_from_lch_to_JzCzhz, 10);
    }

I guess that dt_Lab_to_JzCzhz could be puzzled together from dt_Lab_to_XYZ -> dt_XYZ_2_JzAzBz -> dt_JzAzBz_2_JzCzhz.

kofa73 commented 1 month ago

I've done something, but it does not work. :( https://github.com/darktable-org/darktable/pull/17209