vectorscope: add RYB option

[phweyland]

fixes #9847. See this FR for more details.

This is inspired by "Paint Inspired Color Mixing and Compositing for Visualization" - Gossett.
As the Gossett model is not reversible, we keep his cube hues and use them to transpose the rgb <-> ryb data by spline interpolation.
This model compensates the orange expansion by compressing from green to red unlike the model proposed by Junichi SUGITA & Tokiichiro TAKAHASHI, in "Computational RYB Color Model and its Applications", which compresses mainly the cyan colors. This latest is also reversible and is used in the G'MIC functions.

As we talk of colors harmony I'm embarrassed by the color wash of the presented graph. Example:

compared to:

In the first image, it's impossible to identify the blue... The second image is far more aligned on the real colors of the image.
Thoughts ?

[Mark-64]

Can't build on Windows

../../src/libs/histogram.c: In function '_lib_histogram_process_vectorscope._omp_fn.0':
../../src/libs/histogram.c:321:19: error: '*((void *)&chromaticity+8)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  321 |   const float h = dt_fast_hypotf(*x,*y);
      |                   ^~~~~~~~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.

[phweyland]

Can't build on Windows

Sorry, an oversight. Probably the same cause as for Travis error. Let's see if it is happier now.

[phweyland]

rebased

[Nilvus]

On your screenshots, I agree that first screenshot is quite washed. But the second is probably a little too oversaturated. Maybe something between them would be the best. That was discussed a lot for vectorscope as you probably now in PR #9209 by @dtorop. So maybe some improvement remaining here.

[phweyland]

On your screenshots, I agree that first screenshot is quite washed. But the second is probably a little too oversaturated. Maybe something between them would be the best. That was discussed a lot for vectorscope as you probably now in PR #9209 by @dtorop. So maybe some improvement remaining here.

I'm afraid that everyone will have his own preferences. Histograms and waveform do not suffer being dimmed. From my point of view vectorscopes talk colors and I would prefer them as real as possible.

[MStraeten]

the appearance is dependent to the selected theme: the background using a ...gray theme or even the ...dark theme reduces the contrast to the colors (it's quite contrasty in the ...darker theme)
But just darkening the background will result in an eye catching vercorscope because it's quite darker then approriate for the theme. In my option it's better to have the background color consistent to the theme - so nothing distracts from the image

[phweyland]

But just darkening the background will result in an eye catching vercorscope because it's quite darker then approriate for the theme

I can agree about this. EDIT I was not talking about the dark/grey background, but about colors.

But I disagree about washing the colors of the wheel (colored background of the vectorscope).
EDIT: at least for the RYB wheel I'm also wondering if it could be interesting to "see" the color chroma variation along the radius.

[TurboGit]

But I disagree about washing the colors of the wheel (colored background of the vectorscope).

I agree with @phweyland, it makes the vectorscope mostly unusable.

[Mark-64]

I am afraid the rebase doesn't consider the changes introduced with #9866 and #9906. It actually reverts some of those changes.
Position of samples is not calculated correctly in RYB.
@TurboGit How can we fix this before merging this one? Or shall we do it after ?

[TurboGit]

@TurboGit How can we fix this before merging this one? Or shall we do it after ?

Strange that the rebase broke something ! How did you handle this, if you go to this branch and do a "git rebase master" (making sure master is updated to current commit) what happens? Do you have conflicts? If not the rebase should be fine, no?

[TurboGit]

And anyway, there is no conflicts on this PR... So if I merge all should be fine. Was the washout colors fixed in this PR? (not tested yet, so asking...)

[phweyland]

Was the washout colors fixed in this PR? (not tested yet, so asking...)

No, I've let the current treatment of background vectorscope.
I've risen the topic to get opinion before touchng anything which was made just before.
I'm not sure we get a consensus on this. Maybe the solution would be to let the user saturate the vectorscope the way he likes.
Could be done in another PR. ?

I am afraid the rebase doesn't consider the changes introduced with #9866 and #9906.

The rebase was just to resolve some conflicts. I don't know if these commits are included.

[Mark-64]

Well I merged locally this PR with master and the changes in #9866 and #9906 are there, but this is not enough.
We have to add some code to find coordinates of primary sample an live samples in RYB, but it should not be difficult

[phweyland]

We have to add some code to find coordinates of primary sample an live samples in RYB

Two possibilities. Either you need this PR merged first to apply your changes or you guide me to fix the issue in that PR. What do you prefer ?

[Mark-64]

We have to apply the same transformation from RGB to RYB to primary sample and live samples.

If I understood correctly the transformation, this part of _lib_histogram_process_vectorscope()

  // find position of the primary sample
  for(int k = 0; k < 3; k++)
  {
    switch(statistic)
    {
      case DT_LIB_COLORPICKER_STATISTIC_MEAN:
        RGB[k] = sample->picked_color_rgb_mean[k];
        break;

      case DT_LIB_COLORPICKER_STATISTIC_MIN:
        RGB[k] = sample->picked_color_rgb_min[k];
        break;

      case DT_LIB_COLORPICKER_STATISTIC_MAX:
        RGB[k] = sample->picked_color_rgb_max[k];
        break;
      default:
        fprintf(stderr, "[histogram] unsupported color picker statistics %i\n", statistic);
        break;
    }
  }

  dt_ioppr_rgb_matrix_to_xyz(RGB, XYZ_D50, vs_prof->matrix_in_transposed, vs_prof->lut_in,
                             vs_prof->unbounded_coeffs_in, vs_prof->lutsize, vs_prof->nonlinearlut);
  if(vs_type == DT_LIB_HISTOGRAM_VECTORSCOPE_CIELUV)
  {
    dt_aligned_pixel_t xyY_D50;
    dt_XYZ_to_xyY(XYZ_D50, xyY_D50);
    dt_xyY_to_Luv(xyY_D50, chromaticity);
  }
  else
  {
    dt_aligned_pixel_t XYZ_D65;
    dt_XYZ_D50_2_XYZ_D65(XYZ_D50, XYZ_D65);
    dt_XYZ_2_JzAzBz(XYZ_D65, chromaticity);
  }
  if(vs_scale == DT_LIB_HISTOGRAM_SCALE_LOGARITHMIC)
    log_scale(&chromaticity[1], &chromaticity[2], max_radius);

  d->vectorscope_pt[0] = chromaticity[1];
  d->vectorscope_pt[1] = chromaticity[2];

  // if live simple visualized, find their position
  if(d->vectorscope_samples && darktable.lib->proxy.colorpicker.display_samples)
  {
    g_slist_free_full((GSList *)d->vectorscope_samples, free);
    d->vectorscope_samples = NULL;
    d->selected_sample = -1;
  }
  GSList *samples = darktable.lib->proxy.colorpicker.live_samples;
  if(samples)
  {
    const dt_colorpicker_sample_t *selected = darktable.lib->proxy.colorpicker.selected_sample;

    int pos = 0;
    for(; samples; samples = g_slist_next(samples))
    {
      sample = samples->data;
      if(sample == selected) d->selected_sample = pos;
      pos++;

      //find coordinates
      for(int k = 0; k < 3; k++)
      {
        switch(statistic)
        {
          case DT_LIB_COLORPICKER_STATISTIC_MEAN:
            RGB[k] = sample->picked_color_rgb_mean[k];
            break;

          case DT_LIB_COLORPICKER_STATISTIC_MIN:
            RGB[k] = sample->picked_color_rgb_min[k];
            break;

          case DT_LIB_COLORPICKER_STATISTIC_MAX:
            RGB[k] = sample->picked_color_rgb_max[k];
            break;
          default:
            fprintf(stderr, "[histogram] unsupported color picker statistics %i\n", statistic);
            break;
        }
      }

      dt_ioppr_rgb_matrix_to_xyz(RGB, XYZ_D50, vs_prof->matrix_in_transposed, vs_prof->lut_in,
                                 vs_prof->unbounded_coeffs_in, vs_prof->lutsize, vs_prof->nonlinearlut);
      if(vs_type == DT_LIB_HISTOGRAM_VECTORSCOPE_CIELUV)
      {
        dt_aligned_pixel_t xyY_D50;
        dt_XYZ_to_xyY(XYZ_D50, xyY_D50);
        dt_xyY_to_Luv(xyY_D50, chromaticity);
      }
      else
      {
        dt_aligned_pixel_t XYZ_D65;
        dt_XYZ_D50_2_XYZ_D65(XYZ_D50, XYZ_D65);
        dt_XYZ_2_JzAzBz(XYZ_D65, chromaticity);
      }
      if(vs_scale == DT_LIB_HISTOGRAM_SCALE_LOGARITHMIC)
        log_scale(&chromaticity[1], &chromaticity[2], max_radius);

      float *sample_xy = (float *)calloc(2, sizeof(float));

      sample_xy[0] = chromaticity[1];
      sample_xy[1] = chromaticity[2];

      d->vectorscope_samples = g_slist_append(d->vectorscope_samples, sample_xy);
    }
  }

becomes

  // find position of the primary sample
  sample = darktable.lib->proxy.colorpicker.primary_sample;
  for(int k = 0; k < 3; k++)
  {
    switch(statistic)
    {
      case DT_LIB_COLORPICKER_STATISTIC_MEAN:
        RGB[k] = sample->picked_color_rgb_mean[k];
        break;

      case DT_LIB_COLORPICKER_STATISTIC_MIN:
        RGB[k] = sample->picked_color_rgb_min[k];
        break;

      case DT_LIB_COLORPICKER_STATISTIC_MAX:
        RGB[k] = sample->picked_color_rgb_max[k];
        break;
      default:
        fprintf(stderr, "[histogram] unsupported color picker statistics %i\n", statistic);
        break;
    }
  }

  if(vs_type != DT_LIB_HISTOGRAM_VECTORSCOPE_RYB)
  {

    dt_ioppr_rgb_matrix_to_xyz(RGB, XYZ_D50, vs_prof->matrix_in_transposed, vs_prof->lut_in,
                               vs_prof->unbounded_coeffs_in, vs_prof->lutsize, vs_prof->nonlinearlut);
    if(vs_type == DT_LIB_HISTOGRAM_VECTORSCOPE_CIELUV)
    {
      dt_aligned_pixel_t xyY_D50;
      dt_XYZ_to_xyY(XYZ_D50, xyY_D50);
      dt_xyY_to_Luv(xyY_D50, chromaticity);
    }
    else
    {
      dt_aligned_pixel_t XYZ_D65;
      dt_XYZ_D50_2_XYZ_D65(XYZ_D50, XYZ_D65);
      dt_XYZ_2_JzAzBz(XYZ_D65, chromaticity);
    }
  }
  else
  {
    dt_aligned_pixel_t RYB, rgb, HCV;
    for_each_channel(ch, aligned(rgb, RGB:16))
      rgb[ch] = RGB[ch] <= 0.04045f ? RGB[ch] / 12.92f : powf((RGB[ch] + 0.055f) / (1.0f + 0.055f), 2.4f);
    _rgb2ryb(rgb, RYB, d);
    dt_RGB_2_HCV(RYB, HCV);
    const float alpha = 2.0 * M_PI * HCV[0];
    chromaticity[1] = cosf(alpha) * HCV[1] * 0.01;
    chromaticity[2] = sinf(alpha) * HCV[1] * 0.01;
  }

  if(vs_scale == DT_LIB_HISTOGRAM_SCALE_LOGARITHMIC)
    log_scale(&chromaticity[1], &chromaticity[2], max_radius);

  d->vectorscope_pt[0] = chromaticity[1];
  d->vectorscope_pt[1] = chromaticity[2];

  // if live simple visualized, find their position
  if(d->vectorscope_samples && darktable.lib->proxy.colorpicker.display_samples)
  {
    g_slist_free_full((GSList *)d->vectorscope_samples, free);
    d->vectorscope_samples = NULL;
    d->selected_sample = -1;
  }
  GSList *samples = darktable.lib->proxy.colorpicker.live_samples;
  if(samples)
  {
    const dt_colorpicker_sample_t *selected = darktable.lib->proxy.colorpicker.selected_sample;

    int pos = 0;
    for(; samples; samples = g_slist_next(samples))
    {
      sample = samples->data;
      if(sample == selected) d->selected_sample = pos;
      pos++;

      //find coordinates
      for(int k = 0; k < 3; k++)
      {
        switch(statistic)
        {
          case DT_LIB_COLORPICKER_STATISTIC_MEAN:
            RGB[k] = sample->picked_color_rgb_mean[k];
            break;

          case DT_LIB_COLORPICKER_STATISTIC_MIN:
            RGB[k] = sample->picked_color_rgb_min[k];
            break;

          case DT_LIB_COLORPICKER_STATISTIC_MAX:
            RGB[k] = sample->picked_color_rgb_max[k];
            break;
          default:
            fprintf(stderr, "[histogram] unsupported color picker statistics %i\n", statistic);
            break;
        }
      }

      if(vs_type != DT_LIB_HISTOGRAM_VECTORSCOPE_RYB)
      {

        dt_ioppr_rgb_matrix_to_xyz(RGB, XYZ_D50, vs_prof->matrix_in_transposed, vs_prof->lut_in,
                                   vs_prof->unbounded_coeffs_in, vs_prof->lutsize, vs_prof->nonlinearlut);
        if(vs_type == DT_LIB_HISTOGRAM_VECTORSCOPE_CIELUV)
        {
          dt_aligned_pixel_t xyY_D50;
          dt_XYZ_to_xyY(XYZ_D50, xyY_D50);
          dt_xyY_to_Luv(xyY_D50, chromaticity);
        }
        else
        {
          dt_aligned_pixel_t XYZ_D65;
          dt_XYZ_D50_2_XYZ_D65(XYZ_D50, XYZ_D65);
          dt_XYZ_2_JzAzBz(XYZ_D65, chromaticity);
        }
      }
      else
      {
        dt_aligned_pixel_t RYB, rgb, HCV;
        for_each_channel(ch, aligned(rgb, RGB:16))
          rgb[ch] = RGB[ch] <= 0.04045f ? RGB[ch] / 12.92f : powf((RGB[ch] + 0.055f) / (1.0f + 0.055f), 2.4f);
        _rgb2ryb(rgb, RYB, d);
        dt_RGB_2_HCV(RYB, HCV);
        const float alpha = 2.0 * M_PI * HCV[0];
        chromaticity[1] = cosf(alpha) * HCV[1] * 0.01;
        chromaticity[2] = sinf(alpha) * HCV[1] * 0.01;
      }

      if(vs_scale == DT_LIB_HISTOGRAM_SCALE_LOGARITHMIC)
        log_scale(&chromaticity[1], &chromaticity[2], max_radius);

      float *sample_xy = (float *)calloc(2, sizeof(float));

      sample_xy[0] = chromaticity[1];
      sample_xy[1] = chromaticity[2];

      d->vectorscope_samples = g_slist_append(d->vectorscope_samples, sample_xy);
    }
  }

I tested this code on my local merge (this PR + master) and it works, however, as we repeat three times the same code for the conversion RGB => (JzCzhz / Luv / RYB), we can perhaps compact the code with a macro or a function

[phweyland]

@Mark-64 Great. Thanks for preparing the fix.
I've just rebased, I've integrated your code and created a routine as you suggested.
However I see the main sample on the scope, but not the live ones, whatever the vectorscope chosen.
I'll try to debug that.

EDIT: darktable.lib->proxy.colorpicker.live_samples seems to stay empty
EDIT2: I've pushed the code as is (lives_samples empty while I've added several samples. They are visible on the images, but the vectorscope code seems not to see them).

[Mark-64]

I tested this code on my local merge with master and it works.
If you rebased, you should have taken all the changes in master as well, so I don't understand.
I can have a look, but only in few hours

[phweyland]

so I don't understand.

Sorry, I'm back and I see I have tested the wrong executable. Yes everything works fine. Sorry again for inconvenience!

[Mark-64]

Yes :-)

[phweyland]

rebased to resolve conflicts

[phweyland]

About colors, different parameters influence the colorfulness of the graph.

• the fading of the graph colors. There is a parameter which could be used to get more or less vivid colors.
• the blending with the theme's background. The clearer the theme is the more the background colors are washed. The only action I can see is to play with the darkness of the vectorscope background.
• the graph color saturation is also proportional to the density of points for a given chroma. So the color of the graph is not directly representative of the corresponding color in the image by design, even if we give the user some way to tweak the saturation.

So I'm afraid the user cannot expect to rely on the graph's colors to get an immediate feeling of the harmony.
He has to learn how to read the vectorscope with these 3 dimensions:

• hue: the angle (that is not the graph color which defines the color), EDIT which may be enough to apply basic geometric rules of harmony.
• chroma: the distance from center
• density of image's points with that color: the colorfulness. In any case this last one is not very readable with the grey themes.
• the lightness/brightness being not represented

Any thoughts ?
Any idea to try to improve the color feeling, at least for the RYB vectorscope ?
Pinging also @dtorop

[dtorop]

However in the omp loop (histogram.c line 600) I don't know what to do about the parameter d->rgb2ryb_ypp.

I think you just put d in the dt_omp_firstprivate() clause (should already be there) and then that pointer is passed to each thread. If that is the question?

About colors, different parameters influence the colorfulness of the graph.

This all seems a good rundown. I'd add one more, which is the Cairo compositing operation with which the graph is drawn. So long as it is CAIRO_OPERATOR_ADD, the graph will desaturate as the background lightens up.

Here's with current code (not this PR), CAIRO_OPERATOR_ADD and the grey theme:

With CAIRO_OPERATOR_OVER:

With CAIRO_OPERATOR_HARD_LIGHT:

The graph will still appear more vivid with a dark theme, due to more color contrast with the background color. Here's with CAIRO_OPERATOR_OVER and the "darker" theme:

Perhaps shifting around the drawing mode (and tuning some other parameters) could produce something nice?

Here's a diff using _OVER:

modified   src/libs/histogram.c
@@ -1004,8 +1004,7 @@ static void _lib_histogram_draw_vectorscope(dt_lib_histogram_t *d, cairo_t *cr,
   const gboolean display_live_samples = d->vectorscope_samples &&
       darktable.lib->proxy.colorpicker.display_samples;
 
-  if(display_primary_sample || display_live_samples)
-    cairo_push_group(cr);
+  cairo_push_group(cr);
   cairo_set_source(cr, bkgd_pat);
   cairo_mask(cr, graph_pat);
   cairo_set_operator(cr, CAIRO_OPERATOR_HARD_LIGHT);
@@ -1017,13 +1016,9 @@ static void _lib_histogram_draw_vectorscope(dt_lib_histogram_t *d, cairo_t *cr,
   cairo_pattern_destroy(graph_pat);
   cairo_surface_destroy(graph_surface);
 
-  if(display_primary_sample || display_live_samples)
-  {
-    cairo_pop_group_to_source(cr);
-    cairo_paint_with_alpha(cr, 0.5);
-  }
-
+  cairo_pop_group_to_source(cr);
   cairo_set_operator(cr, CAIRO_OPERATOR_OVER);
+  cairo_paint_with_alpha(cr, (display_primary_sample || display_live_samples) ? 0.5 : 1.0);
 
   // overlay central circle
   set_color(cr, darktable.bauhaus->graph_grid);

[phweyland]

Thanks @dtorop, I'll take the time to digest this.

[phweyland]

I think you just put d in the dt_omp_firstprivate() clause (should already be there) and then that pointer is passed to each thread. If that is the question?

Yes it was the question, thanks.
As d->rgb2ryb_ypp is also a pointer, and d not used otherwise, I've replaced d by a local copy of rgb2ryb_ypp.

[dtorop]

Also! I can't guarantee that #9925 hasn't messed up this PR... If so, apologies, but it shouldn't be too difficult to fix.

[phweyland]

I can't guarantee that #9925 hasn't messed up this PR... If so, apologies, but it shouldn't be too difficult to fix.

No problem, I've rebased and haven't found any issue!

[phweyland]

@TurboGit I think the PR is ok for review.
I'll try to improve a little bit the vectorscope colors but in another PR (which may take advantage of #9640 - local preferences).

[TurboGit]

TIA.

[phweyland]

All done. Thanks !

[johnny-bit]

CI got "problematiced" in GCC 11 nofeatures + nosse with :

/home/runner/work/darktable/darktable/src/src/libs/histogram.c: In function ‘dt_lib_histogram_process’:
/home/runner/work/darktable/darktable/src/src/libs/histogram.c:391:46: error: ‘chromaticity[1]’ is used uninitialized [-Werror=uninitialized]
  391 |       dt_aligned_pixel_t rgb_scope, XYZ_D50, chromaticity;
      |                                              ^~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.

[phweyland]

Travelling, I'll check this asap.

[TurboGit]

Travelling, I'll check this asap.

The fix looks trivial, we need to initialize chromaticity as indeed chromaticity[0] is not used afterward and in the RYB case we only set the index 1 and 2. Will merge and add necessary init for GCC 11.