darktable icon indicating copy to clipboard operation
darktable copied to clipboard

Artifacts when importing / opening old edit in current master

Open Macchiato17 opened this issue 2 years ago • 35 comments

Describe the bug

see also: https://discuss.pixls.us/t/artifacts-when-importing-opening-old-edit-in-current-master/40097

After opening / importing a darktable 3.2.1 edit, the picture is totally screwed up already in lighttable (see Screenshot 1 below). Same for darkroom.

Things I noted:

  • filmic settings are version 4 (2020), contrast in highlights/shadows are both set to “hard”. Changing the contrast settings doesn't matter
  • white luminance setting in filmic is e.g. 0,1742% instead of 100% (Screenshot 2)
  • when I go one step before filmic down in the history, the picture returns to normal.
  • my workflow default is scene referred (filmic)
  • turning openCL off or on has no impact

Finally: leaving everything as is after importing the picture and then changing the contrast (being 1.0 after importing) to the same value of 1.0, then everything “realigns” and the picture looks fine. May this be some kind of mismatch between persisted values in the XMP and the value after reading the XMP (see attached file) or writing to the database?

Steps to reproduce

see description if this can just be debugged with the RAW file available, please drop a note and I will add it to this ticket. => added together with the related XMP

Expected behavior

No response

Logfile | Screenshot | Screencast

Screenshot 1

Screenshot 2

RAW file

XMP file

Commit

No response

Where did you install darktable from?

self compiled

darktable version

darktable-4.5.0+958~ga37b7fb789-win64

What OS are you using?

Windows

What is the version of your OS?

Windows 11

Describe your system?

Laptop 8GB RAM NVIDIA GeForce GTX 1050 Max-Q (4GB) Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz

Are you using OpenCL GPU in darktable?

Yes

If yes, what is the GPU card and driver?

NVIDIA GeForce GTX 1050 Max-Q (4GB), driver 31.0.15.3699 (04.08.2023)

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

No response

Macchiato17 avatar Oct 25 '23 20:10 Macchiato17

May this be some kind of mismatch between persisted values in the XMP and the value after reading the XMP (see attached file) or writing to the database?

No idea but applying your side car to a variety of my images leads to no weird issues ...setting seem intact... maybe copy those images to a new location with the sidecars... then remove them from the db and reimport...any issue... it might have just been a database hiccup at some point along the way....

todd-prior avatar Oct 25 '23 22:10 todd-prior

That's quite strange. I already removed that film roll and re-imported it (even quitting dt inbetween). I typically just hold pictures in the dt database for as long as I work on them. I also tried your suggestion regarding copying the RAW + XMP to a different location, same result. Even if I put the XMP as a sidecar to any other picture, I see kind of similar issues. May I ask you to try it once again with the RAW that I just attached to this ticket? Just to sort out, if the issue may be related to the RAW (Nikon D7500 NEF file). Thanks so much for your support.

Macchiato17 avatar Oct 26 '23 19:10 Macchiato17

Seems to open fine... What if you set DT to look for new xmp...maybe that will refresh the database or you could open DT with the --configdir option and give it a path for a set of fresh config files...if it opens fine your database might be the issue

image

todd-prior avatar Oct 26 '23 23:10 todd-prior

I also tried your image and it looks beautiful. believe you should not have added denoise as I see no difference with it removed.

Tumbleweed openSUSE 20231023 darktable 4.5.0+993~g7b001ad6b

nice image

ptilopteri avatar Oct 26 '23 23:10 ptilopteri

While importing from V4 the spline correction is not good. That's "deep inside filmicrgb dev history", i am currently not sure how we can handle that correctly.

jenshannoschwalm avatar Oct 29 '23 09:10 jenshannoschwalm

Seems to open fine... What if you set DT to look for new xmp...maybe that will refresh the database or you could open DT with the --configdir option and give it a path for a set of fresh config files...if it opens fine your database might be the issue

Sorry for not replying earlier to your suggestions. But now I have some more time to come back to this.

Thanks for pointing me there @todd-prior . I just tried with --configdir and a non-existing path. Well, everything was initialized, but unfortunately the import shows exactly the same issue. Meeeee...

So I just kept on playing and had a closer look at your screenshot (thanks for sharing,). I noticed that after re-applying just the contrast value of "1", the filmic "look only" spline looked quite different on my system from what your screenshot shows. This was also reflected in the overall look when comparing your screenshot to my dt darkroom view. So I adjusted my filmic settings to match my spline with yours. And - hm - there are indeed some interesting value changes:

filmic_adjustments

Please ignore the highlight reconstruction, which I simply turned off without having any influence. But the rest is perhaps interesting

  • target white luminance was 18,45 before (sounds to me like this being the middle grey value instead of the target 100%)
  • latitude and shadow/highlight balance seem, as if they just were mixed-up
  • contrast was a nan before changing to "1"

That all still sounds to me like a persistence / reading issue. At the same time it seems as if under Linux there is no issue at all with this picture (am I right that you are also running dt on Linux as @ptilopteri does?) So I really have no clue what might be a Windows specific problem here. But another user in the pixls-thread mentioned, that he also experienced the same issue with one of the "Windows Insider" test-builds one / two weeks ago. His picture had also filmic v4 applied.

Macchiato17 avatar Oct 30 '23 20:10 Macchiato17

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

github-actions[bot] avatar Dec 30 '23 00:12 github-actions[bot]

I tried again with current master [0c2ad29b] to please the github-actions bot 😁 and as I expected, the issue is still there.

@jenshannoschwalm, I was not sure, if you already had the opportunity to read my last comment on this in which I have tried to summarize my findings after successfully restoring the filmic RGB settings manually after the faulty import. As a bottom line, I had the impression, that there was "just" some kind of "mixing-up effects" during import of the affected pictures / reading the affected XMP on Windows. Or is your comment "deep inside filmicrgb dev history" to be understood as to also point to the same effect?

Best regards and wishes for the advancing New Year 😃

Macchiato17 avatar Dec 30 '23 11:12 Macchiato17

Or is your comment "deep inside filmicrgb dev history" to be understood as to also point to the same effect?

I don't know the filmicrgb code well enough to understand how parameters would have to be adapted in some module version change.

jenshannoschwalm avatar Jan 03 '24 09:01 jenshannoschwalm

I was still trying to gather some more supporting information on this effect to help narrowing things down a little bit more...

So I tried latest Ansel nightly [4d185b4] today and was indeed able to import the picture on my system without any of the above mentioned artifacts resulting after import 😃 After that I closed Ansel and imported the thus "migrated" XMP in darktable [bce83d51] and voilà no artifacts anymore.

Hopefully, these findings may help you...

Macchiato17 avatar Jan 04 '24 23:01 Macchiato17

That would point to Filmic's legacy_params function as the culprit.

ralfbrown avatar Jan 07 '24 16:01 ralfbrown

That would point to Filmic's legacy_params function as the culprit.

Think so too but couldn't spot the problem...

jenshannoschwalm avatar Jan 07 '24 18:01 jenshannoschwalm

Tried importing this image from the first time and got no artifact. The Filmic UI looks fine:

image

And the image clean:

image

TurboGit avatar Jan 08 '24 09:01 TurboGit

Thanks a lot for testing also on your side @TurboGit. Did you use Linux or Windows for the test? I'm wondering, since it seems, as if Linux works fine, whereas on Windows me and another user (from a pixls.us discussion) can see this effect.

Macchiato17 avatar Jan 08 '24 12:01 Macchiato17

@Macchiato17 : I'm using Linux. Hard to believe that this could be a Windows only issue... but everything is possible.

TurboGit avatar Jan 08 '24 12:01 TurboGit

I added the "reproduced" label some time ago and as @TurboGit couldn't reproduce i just checked again on this mornings master and can still confirm the issue by

  1. downloading original raw and xmp
  2. OpenCL correctly working but that does not matter at all
  3. Checked for a) no available filmic presets and b) no style applied
  4. Checked with all "apply workflow" settings

Visual output is Bildschirmfoto vom 2024-01-10 08-10-36

and the logs from -d params -d pipe also suggest a problem there as shown

     7.7145 commit params                 [full]           temperature            piece hash=58cd2398489aeef9, 
     7.7145 commit params                 [full]           highlights             piece hash=4b3de2e5a3cd1640, 
     7.7145 [iop_validate_params] `filmicrgb' failed for type "float", field: latitude
     7.7145 [iop_validate_params] `filmicrgb' failed for type "dt_iop_filmicrgb_params_t"
     7.7145 commit params                 [full]           filmicrgb              piece hash=9ad7f367250b12c8, 
     7.7145 commit params                 [full]           exposure               piece hash=e5452aad4c898038, 
     7.7145 commit params                 [full]           flip                   piece hash=0, 
     7.7145 [iop_validate_params] `lens' failed for type "float", field: scale_md_v1
     7.7145 [iop_validate_params] `lens' failed for type "dt_iop_lens_params_t"
     7.7152 commit params                 [full]           lens                   piece hash=356ed5356b38a292, 
     7.7159 commit params                 [full]           denoiseprofile         piece hash=a0ee6b805127c3e3, 

jenshannoschwalm avatar Jan 10 '24 07:01 jenshannoschwalm

@jenshannoschwalm : Seeing the error could this be a coma vs dot in float?

TurboGit avatar Jan 10 '24 08:01 TurboGit

I have tested with English locale instead of French and I have:

    42.1980 commit params                 [preview]        temperature            piece hash=58cd2398489aeef9, 
    42.1980 commit params                 [preview]        highlights             piece hash=4b3de2e5a3cd1640, 
    42.1980 commit params                 [preview]        filmicrgb              piece hash=a792a8d82e1a35b3, 
    42.1980 commit params                 [preview]        exposure               piece hash=e5452aad4c898038, 
    42.1980 commit params                 [preview]        flip                   piece hash=0, 
    42.1980 [iop_validate_params] `lens' failed for type "float", field: scale_md_v1
    42.1980 [iop_validate_params] `lens' failed for type "dt_iop_lens_params_t"
    42.1988 commit params                 [preview]        lens                   piece hash=356ed5356b38a292, 
    42.1999 commit params                 [preview]        denoiseprofile         piece hash=a0ee6b805127c3e3, 
    42.1999 commit params                 [preview]        filmicrgb              piece hash=0, 
    42.2000 commit params                 [preview]        exposure               piece hash=e5452aad4c898038, 
    42.2000 commit params                 [preview]        filmicrgb              piece hash=a792a8d82e1a35b3, 
    42.2000 synch all modules done        [preview]                               defaults 0.0222s, history 0.0024s

So not problem with FilmicRGB, something really fishy here.

TurboGit avatar Jan 10 '24 08:01 TurboGit

So not problem with FilmicRGB, something really fishy here.

Yes, i checked this too. After compressing history it goes away, we basically check parameters for every history step i think.

jenshannoschwalm avatar Jan 10 '24 08:01 jenshannoschwalm

@jenshannoschwalm : Seeing the error could this be a coma vs dot in float?

Well i can confirm the issue can not be reproduced if importing the original xmp with "german" being the dt language.

VERY stangely - after switching back to english i can not confirm :-(

jenshannoschwalm avatar Jan 10 '24 09:01 jenshannoschwalm

Well i can confirm the issue can not be reproduced if importing the original xmp with "german" being the dt language.

maybe something related to #16044 where the aperture filter didn't work because of locale issues (comma vs dot in floats)?

zisoft avatar Jan 10 '24 10:01 zisoft

For lens see #16079

TurboGit avatar Jan 10 '24 11:01 TurboGit

@zisoft : I don't think it is related to #16044 as parameters are just seen as binary blob overlaid on top of the C struct.

TurboGit avatar Jan 10 '24 12:01 TurboGit

@Macchiato17 : Can you still reproduce with 4.6.0 (or using branch darktable-4.6.x) or master ?

TurboGit avatar Jan 10 '24 12:01 TurboGit

First of all, thank you all for your great commitment. I really appreciate it.

So I built dt on master [420bc453] and was unfortunately still able to see this issue.

Following your discussion, I did a darktable.exe -d params -d pipe and attached the relevant logfile output here. The log starts when the photo is imported in the lighttable and ends when the image is displayed in the darkroom.

There are several occurrences of an error related to filmicrgb concerning field latitude:

316,6005 [iop_validate_params] `filmicrgb' failed for type "float", field: latitude
316,6006 [iop_validate_params] `filmicrgb' failed for type "dt_iop_filmicrgb_params_t"

I'm on a German Windows 11, dt GUI is set to English. But even when changing dt GUI to German, this issue still resist. If there is anything else I could try out for you, please don't hesitate...

Macchiato17 avatar Jan 10 '24 22:01 Macchiato17

Since you are building from source, can you add this patch and report what is displayed for the wrong value?

diff --git a/src/develop/imageop.c b/src/develop/imageop.c
index dad3f86913..310ba4b572 100644
--- a/src/develop/imageop.c
+++ b/src/develop/imageop.c
@@ -1972,6 +1972,16 @@ gboolean _iop_validate_params(dt_introspection_field_t *field,
       || dt_isinf(*(float*)p)
       || (*(float*)p == field->Float.Default)
       || (*(float*)p >= field->Float.Min && *(float*)p <= field->Float.Max);
+     if(!all_ok)
+     {
+       printf("@@@@@ FLOAT : %d %d %d %d %d (%f)\n",
+              dt_isnan(*(float*)p),
+              dt_isinf(*(float*)p),
+              *(float*)p == field->Float.Default,
+              *(float*)p >= field->Float.Min,
+              *(float*)p <= field->Float.Max,
+              *(float*)p);
+     }
     break;
   case DT_INTROSPECTION_TYPE_INT:
     all_ok = (*(int*)p >= field->Int.Min && *(int*)p <= field->Int.Max);

TIA.

TurboGit avatar Jan 11 '24 06:01 TurboGit

So we get this output now:

@@@@@ FLOAT : 0 0 0 0 1 (0,000000)
110,0760 [iop_validate_params] `filmicrgb' failed for type "float", field: latitude
110,0760 [iop_validate_params] `filmicrgb' failed for type "dt_iop_filmicrgb_params_t"

Hope that helps 🙂 have a nice day👋

Macchiato17 avatar Jan 11 '24 07:01 Macchiato17

Somehow it helps, the value 0.0000 is not a correct one as latitude (and this since first implementation of FilmicRGB) should be between 0.01 and 99.0. So somehow the params is wrong, yet the float value is 'sane' (not inf or nan) and so looks like a real value not some corrupted one. But I have no idea on how 0.0 could have been set here, and so maybe some corruption after all... As you see, it helps just a little bit :)

Side question, if you set latitude to 0.01 manually does it fixes the FilmicRGB curve and image look?

What I really do not understand is why I don't see this on my side!

TurboGit avatar Jan 11 '24 08:01 TurboGit

I tried on a second computer with CLang instead of GCC and using a Release build instead of a Debug one... And still cannot reproduce :(

TurboGit avatar Jan 11 '24 09:01 TurboGit

Another try, can you test with this change:

diff --git a/src/iop/filmicrgb.c b/src/iop/filmicrgb.c
index 6043d2cdec..4828952935 100644
--- a/src/iop/filmicrgb.c
+++ b/src/iop/filmicrgb.c
@@ -421,7 +421,7 @@ static inline void convert_to_spline_v3(dt_iop_filmicrgb_params_t* n)
   contrast *= 8.0f / (n->white_point_source - n->black_point_source);
   contrast *= hardness * powf(grey_display, hardness-1.0f);
   // latitude is the % of the segment [b+safety*(w-b),w-safety*(w-b)] which is covered, where b is black_display and w white_display
-  const float latitude = CLAMP((shoulder_display - toe_display) / ((white_display - black_display) - 2.0f * scaled_safety_margin), 0.0f, 0.99f);
+  const float latitude = CLAMP((shoulder_display - toe_display) / ((white_display - black_display) - 2.0f * scaled_safety_margin), 0.0001f, 0.99f);
   // find balance
   float toe_display_ref = latitude * (black_display + scaled_safety_margin) + (1.0f - latitude) * grey_display;
   float shoulder_display_ref = latitude * (white_display - scaled_safety_margin) + (1.0f - latitude) * grey_display;

It is just changing CLAMP lower value to 0.0001f instead of 0.0f.

TurboGit avatar Jan 11 '24 11:01 TurboGit