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.7k stars 1.14k forks source link

3 regressions tonight #14398

Closed TurboGit closed 1 year ago

TurboGit commented 1 year ago

Regressions introduced between: 25ead59b2b..07cdefdb6a

Errors     3
   - 0119-blending-rgb-scene-1
   - 0120-blending-rgb-scene-2
   - 0121-blending-rgb-scene-3

To be analyzed.

TurboGit commented 1 year ago

The diffs:

0119: diff

0120: diff

0121: diff

TurboGit commented 1 year ago

The culprit is:

commit a5ee7e75f7d50e749fb88109d21bce1cf0b4abf1
Author: Ralf Brown <ralfbrown@users.noreply.github.com>
Date:   Sun Apr 30 16:26:57 2023 -0400

    colorbalancergb: improve vectorization and other optimizations

    Passes tests 0083, 0093, and 0103.

    Using JzAzBz saturation formula:
    Thr     Master  PR
    1       5563.95 3706.48 -33.3%
    2       2788.23 1860.80 -33.2%
    4       1396.71  948.37 -32.0%
    8        701.75  468.62 -33.2%
    16       352.17  235.45 -33.1%
    32       179.90  120.27 -33.1%
    64       120.45   75.87 -37.0%

    Using UCS saturation formula:
    Thr     Master  PR
    1       5052.46 4172.56 -17.4%
    2       2535.63 2092.42 -17.4%
    4       1270.48 1049.65 -17.3%
    8        642.33  532.26 -17.1%
    16       322.20  266.97 -17.1%
    32       166.92  135.18 -19.0%
    64       111.12   87.49 -21.2%

@ralfbrown : Can you have a look at those diffs which seems to be quite high and so probably not expected? TIA.

ralfbrown commented 1 year ago

Best to revert that commit for now, as I may not have time to investigate until Thursday.

TurboGit commented 1 year ago

Ok, will revert, for the record the diff are:

Test 0119-blending-rgb-scene-1
      Image mire1.cr2
      Expected CPU vs. current CPU report :
      ----------------------------------
      Max dE                   : 18.27183
      Avg dE                   : 0.04444
      Std dE                   : 0.14635
      ----------------------------------
      Pixels below avg + 0 std : 86.60 %
      Pixels below avg + 1 std : 90.73 %
      Pixels below avg + 3 std : 97.68 %
      Pixels below avg + 6 std : 99.84 %
      Pixels below avg + 9 std : 99.97 %
      ----------------------------------
      Pixels above tolerance   : 0.01 %

  FAILS: image visually changed
         see diff.png for visual difference
         (388673 pixels changed)

Test 0120-blending-rgb-scene-2
      Image mire1.cr2
      Expected CPU vs. current CPU report :
      ----------------------------------
      Max dE                   : 27.30602
      Avg dE                   : 0.15709
      Std dE                   : 0.65947
      ----------------------------------
      Pixels below avg + 0 std : 77.86 %
      Pixels below avg + 1 std : 96.58 %
      Pixels below avg + 3 std : 98.67 %
      Pixels below avg + 6 std : 99.37 %
      Pixels below avg + 9 std : 99.70 %
      ----------------------------------
      Pixels above tolerance   : 1.24 %

  FAILS: image visually changed
         see diff.png for visual difference
         (644190 pixels changed)

Test 0121-blending-rgb-scene-3
      Image mire1.cr2
      Expected CPU vs. current CPU report :
      ----------------------------------
      Max dE                   : 4.91282
      Avg dE                   : 0.07517
      Std dE                   : 0.23013
      ----------------------------------
      Pixels below avg + 0 std : 86.00 %
      Pixels below avg + 1 std : 89.71 %
      Pixels below avg + 3 std : 97.68 %
      Pixels below avg + 6 std : 99.54 %
      Pixels below avg + 9 std : 99.90 %
      ----------------------------------
      Pixels above tolerance   : 0.07 %

  FAILS: image visually changed
         see diff.png for visual difference
         (392462 pixels changed)
TurboGit commented 1 year ago

I think that the issue is dt_vector_pow1() vs powf().

TurboGit commented 1 year ago

I have reverted only the changes in dt_XYZ_2_JzAzBz and dt_JzAzBz_2_XYZ which are using the new dt_vector_pow1().

TurboGit commented 1 year ago

I confirm that the 3 regressions are now fixed after reverting part of changes as described above.