Darktable: iop order isn't saved when image history from 2.6 is migrated
Describe the bug
Darktable 3.0.0rc0 is shorting in wrong order the input color profil and unbreak input profil iops when open an earlier edited image (2.6.2).
Update:
Not only GUI problem, the processing pipeline have wrong order too!
What I see in the color groups panel of dt 3.0.rc0:
.
.
.
.
unbreak input profil
color profil
But the right order is:
.
.
.
.
color profil
unbreak input profil
Platform:
- OS: [Ubuntu 19.10]
- Version [3.0.0rc vs. 2.6.2]
vsz1lu
All 29 comments
confirmed, but how relevant is this? or is it just a gui thing?
blitzgneisserin
on 12 Nov 2019
It's not just gui bug.
The order is changed in the processing pipeline too.
I think it's a very big problem when you want continuous earlier editings. Yes, user can reordering the iops manually, but it's not a real solution if you have many-many thousands raw shots what processed in earlier versions.
vsz1lu
on 12 Nov 2019
Yes, that's definitely wrong (the whole point of 'unbreak' is to preprocess for the input profile) There have been some fixes to ordering since rc0, but I don't know if any of them fixed this. Can anyone reproduce this on current master? If it does need further troubleshooting, some good things to provide would be a sidecar file which always produces this, as well as the console output related to this when running darktable -d ioporder. Paging @jenshannoschwalm @TurboGit
junkyardsparkle
on 13 Nov 2019
Ok, confirmed on current master. Imported image into 2.6.3, enabled profile_gamma iop, exported sidecar, then imported both into master from today. Console output:
*** checking for 77 known iops ***
gamma, 71.000000
watermark, 69.000000
borders, 68.000000
dither, 67.500000
rawoverexposed, 67.000000
overexposed, 66.000000
finalscale, 65.000000
clahe, 64.000000
colorout, 63.500000
velvia, 63.000000
splittoning, 62.000000
vignette, 61.000000
soften, 60.000000
channelmixer, 59.000000
colorcontrast, 57.000000
lut3d, 56.500000
grain, 56.000000
highpass, 55.000000
lowpass, 54.000000
sharpen, 53.000000
colorcorrection, 52.000000
relight, 51.000000
rgbcurve, 50.500000
rgblevels, 50.250000
levels, 50.000000
tonecurve, 49.000000
zonesystem, 48.000000
colisa, 47.000000
filmicrgb, 46.500000
filmic, 46.000000
monochrome, 45.000000
lowlight, 44.000000
colorzones, 43.000000
bilat, 42.000000
atrous, 41.000000
shadhi, 40.000000
globaltonemap, 39.000000
nlmeans, 38.000000
bloom, 37.000000
colormapping, 36.000000
colortransfer, 35.000000
colorize, 34.000000
colorbalance, 33.000000
vibrance, 32.000000
equalizer, 31.000000
defringe, 30.000000
colorchecker, 29.000000
colorreconstruct, 28.000000
hazeremoval, 26.000000
profile_gamma, 25.000000
bilateral, 24.000000
basecurve, 23.000000
graduatednd, 22.000000
toneequal, 21.500000
clipping, 21.000000
flip, 20.000000
scalepixels, 19.000000
rotatepixels, 18.000000
liquify, 17.000000
ashift, 16.000000
lens, 15.000000
retouch, 14.000000
spots, 13.000000
exposure, 12.000000
tonemap, 11.000000
denoiseprofile, 10.000000
mask_manager, 9.000000
basicadj, 8.750000
colorin, 8.500000
demosaic, 8.000000
rawdenoise, 7.000000
hotpixels, 6.000000
cacorrect, 5.000000
highlights, 4.000000
temperature, 3.000000
invert, 2.000000
rawprepare, 1.000000
***** On-the-fly history V[1]->V[2], imageid: 76876 ****************
0 flip multi 0 :: iop 20.00100000000 -> 20.00000000000
1 basecurve multi 0 :: iop 23.00100000000 -> 23.00000000000
2 profile_gamma multi 0 :: iop 25.00100000000 -> 25.00000000000
3 colorin multi 0 :: iop 27.00100000000 -> 8.50000000000
4 profile_gamma multi 0 :: iop 25.00100000000 -> 25.00000000000
dt_ioppr_set_default_iop_order
db: 1.00000000000 xmp 1.0000 rawprepare
db: 2.00000000000 xmp 2.0000 invert
db: 3.00000000000 xmp 3.0000 temperature
db: 4.00000000000 xmp 4.0000 highlights
db: 5.00000000000 xmp 5.0000 cacorrect
db: 6.00000000000 xmp 6.0000 hotpixels
db: 7.00000000000 xmp 7.0000 rawdenoise
db: 8.00000000000 xmp 8.0000 demosaic
db: 10.00000000000 xmp 10.0000 denoiseprofile
db: 24.00000000000 xmp 24.0000 bilateral
db: 18.00000000000 xmp 18.0000 rotatepixels
db: 19.00000000000 xmp 19.0000 scalepixels
db: 15.00000000000 xmp 15.0000 lens
db: 26.00000000000 xmp 26.0000 hazeremoval
db: 16.00000000000 xmp 16.0000 ashift
db: 20.00000000000 xmp 20.0000 flip
db: 21.00000000000 xmp 21.0000 clipping
db: 17.00000000000 xmp 17.0000 liquify
db: 13.00000000000 xmp 13.0000 spots
db: 14.00000000000 xmp 14.0000 retouch
db: 12.00000000000 xmp 12.0000 exposure
db: 9.00000000000 xmp 9.0000 mask_manager
db: 11.00000000000 xmp 11.0000 tonemap
db: 21.50000000000 xmp 21.5000 toneequal
db: 22.00000000000 xmp 22.0000 graduatednd
db: 25.00000000000 xmp 25.0000 profile_gamma
db: 31.00000000000 xmp 31.0000 equalizer
db: 8.50000000000 xmp 8.5000 colorin
db: 38.00000000000 xmp 38.0000 nlmeans
db: 29.00000000000 xmp 29.0000 colorchecker
db: 30.00000000000 xmp 30.0000 defringe
db: 41.00000000000 xmp 41.0000 atrous
db: 54.00000000000 xmp 54.0000 lowpass
db: 55.00000000000 xmp 55.0000 highpass
db: 53.00000000000 xmp 53.0000 sharpen
db: 56.50000000000 xmp 56.5000 lut3d
db: 35.00000000000 xmp 35.0000 colortransfer
db: 59.00000000000 xmp 59.0000 channelmixer
db: 8.75000000000 xmp 8.7500 basicadj
db: 33.00000000000 xmp 33.0000 colorbalance
db: 50.50000000000 xmp 50.5000 rgbcurve
db: 50.25000000000 xmp 50.2500 rgblevels
db: 23.00000000000 xmp 23.0000 basecurve
db: 46.00000000000 xmp 46.0000 filmic
db: 46.50000000000 xmp 46.5000 filmicrgb
db: 47.00000000000 xmp 47.0000 colisa
db: 49.00000000000 xmp 49.0000 tonecurve
db: 50.00000000000 xmp 50.0000 levels
db: 40.00000000000 xmp 40.0000 shadhi
db: 48.00000000000 xmp 48.0000 zonesystem
db: 39.00000000000 xmp 39.0000 globaltonemap
db: 51.00000000000 xmp 51.0000 relight
db: 42.00000000000 xmp 42.0000 bilat
db: 52.00000000000 xmp 52.0000 colorcorrection
db: 57.00000000000 xmp 57.0000 colorcontrast
db: 63.00000000000 xmp 63.0000 velvia
db: 32.00000000000 xmp 32.0000 vibrance
db: 43.00000000000 xmp 43.0000 colorzones
db: 36.00000000000 xmp 36.0000 colormapping
db: 37.00000000000 xmp 37.0000 bloom
db: 34.00000000000 xmp 34.0000 colorize
db: 44.00000000000 xmp 44.0000 lowlight
db: 45.00000000000 xmp 45.0000 monochrome
db: 56.00000000000 xmp 56.0000 grain
db: 60.00000000000 xmp 60.0000 soften
db: 62.00000000000 xmp 62.0000 splittoning
db: 61.00000000000 xmp 61.0000 vignette
db: 28.00000000000 xmp 28.0000 colorreconstruct
db: 63.50000000000 xmp 63.5000 colorout
db: 64.00000000000 xmp 64.0000 clahe
db: 65.00000000000 xmp 65.0000 finalscale
db: 66.00000000000 xmp 66.0000 overexposed
db: 67.00000000000 xmp 67.0000 rawoverexposed
db: 67.50000000000 xmp 67.5000 dither
db: 68.00000000000 xmp 68.0000 borders
db: 69.00000000000 xmp 69.0000 watermark
db: 71.00000000000 xmp 71.0000 gamma
dt_ioppr_set_default_iop_order
db: 1.00000000000 xmp 1.0000 rawprepare
db: 2.00000000000 xmp 2.0000 invert
db: 3.00000000000 xmp 3.0000 temperature
db: 4.00000000000 xmp 4.0000 highlights
db: 5.00000000000 xmp 5.0000 cacorrect
db: 6.00000000000 xmp 6.0000 hotpixels
db: 7.00000000000 xmp 7.0000 rawdenoise
db: 8.00000000000 xmp 8.0000 demosaic
db: 10.00000000000 xmp 10.0000 denoiseprofile
db: 24.00000000000 xmp 24.0000 bilateral
db: 18.00000000000 xmp 18.0000 rotatepixels
db: 19.00000000000 xmp 19.0000 scalepixels
db: 15.00000000000 xmp 15.0000 lens
db: 26.00000000000 xmp 26.0000 hazeremoval
db: 16.00000000000 xmp 16.0000 ashift
db: 20.00000000000 xmp 20.0000 flip
db: 21.00000000000 xmp 21.0000 clipping
db: 17.00000000000 xmp 17.0000 liquify
db: 13.00000000000 xmp 13.0000 spots
db: 14.00000000000 xmp 14.0000 retouch
db: 12.00000000000 xmp 12.0000 exposure
db: 9.00000000000 xmp 9.0000 mask_manager
db: 11.00000000000 xmp 11.0000 tonemap
db: 21.50000000000 xmp 21.5000 toneequal
db: 22.00000000000 xmp 22.0000 graduatednd
db: 25.00000000000 xmp 25.0000 profile_gamma
db: 31.00000000000 xmp 31.0000 equalizer
db: 8.50000000000 xmp 8.5000 colorin
db: 38.00000000000 xmp 38.0000 nlmeans
db: 29.00000000000 xmp 29.0000 colorchecker
db: 30.00000000000 xmp 30.0000 defringe
db: 41.00000000000 xmp 41.0000 atrous
db: 54.00000000000 xmp 54.0000 lowpass
db: 55.00000000000 xmp 55.0000 highpass
db: 53.00000000000 xmp 53.0000 sharpen
db: 56.50000000000 xmp 56.5000 lut3d
db: 35.00000000000 xmp 35.0000 colortransfer
db: 59.00000000000 xmp 59.0000 channelmixer
db: 8.75000000000 xmp 8.7500 basicadj
db: 33.00000000000 xmp 33.0000 colorbalance
db: 50.50000000000 xmp 50.5000 rgbcurve
db: 50.25000000000 xmp 50.2500 rgblevels
db: 23.00000000000 xmp 23.0000 basecurve
db: 46.00000000000 xmp 46.0000 filmic
db: 46.50000000000 xmp 46.5000 filmicrgb
db: 47.00000000000 xmp 47.0000 colisa
db: 49.00000000000 xmp 49.0000 tonecurve
db: 50.00000000000 xmp 50.0000 levels
db: 40.00000000000 xmp 40.0000 shadhi
db: 48.00000000000 xmp 48.0000 zonesystem
db: 39.00000000000 xmp 39.0000 globaltonemap
db: 51.00000000000 xmp 51.0000 relight
db: 42.00000000000 xmp 42.0000 bilat
db: 52.00000000000 xmp 52.0000 colorcorrection
db: 57.00000000000 xmp 57.0000 colorcontrast
db: 63.00000000000 xmp 63.0000 velvia
db: 32.00000000000 xmp 32.0000 vibrance
db: 43.00000000000 xmp 43.0000 colorzones
db: 36.00000000000 xmp 36.0000 colormapping
db: 37.00000000000 xmp 37.0000 bloom
db: 34.00000000000 xmp 34.0000 colorize
db: 44.00000000000 xmp 44.0000 lowlight
db: 45.00000000000 xmp 45.0000 monochrome
db: 56.00000000000 xmp 56.0000 grain
db: 60.00000000000 xmp 60.0000 soften
db: 62.00000000000 xmp 62.0000 splittoning
db: 61.00000000000 xmp 61.0000 vignette
db: 28.00000000000 xmp 28.0000 colorreconstruct
db: 63.50000000000 xmp 63.5000 colorout
db: 64.00000000000 xmp 64.0000 clahe
db: 65.00000000000 xmp 65.0000 finalscale
db: 66.00000000000 xmp 66.0000 overexposed
db: 67.00000000000 xmp 67.0000 rawoverexposed
db: 67.50000000000 xmp 67.5000 dither
db: 68.00000000000 xmp 68.0000 borders
db: 69.00000000000 xmp 69.0000 watermark
db: 71.00000000000 xmp 71.0000 gamma
^^^^^ modulegroups
rawprepare 1.00000
temperature 3.00000
highlights 4.00000
demosaic 8.00000
colorin 8.50000
flip 20.00000
basecurve 23.00000
profile_gamma 25.00000
colorout 63.50000
finalscale 65.00000, hidden
overexposed 66.00000, hidden
rawoverexposed 67.00000, hidden
gamma 71.00000, hidden
vvvvv
@vsz1lu It's actually that the 'input color profile' module is being placed much earlier in the pipe than it should be, so if you need to fix this manually for now, you should do it by moving that one to just after (above) the 'unbreak profile' module. Actually, there's more broken than just that...
junkyardsparkle
on 13 Nov 2019
I can also reproduce this without enabling "unbreak", so it doesn't seem related to that (other than that it makes the mess more visually apparent).
junkyardsparkle
on 13 Nov 2019
Thx! Tonight I will check other iops too.
vsz1lu
on 13 Nov 2019
@TurboGit before this gets hot, I am surprised about a) the 1-2v on the fly conversion, b) from memory the iop orders in xmp don't look like dt2.6. I won't have access to sources till tomorrow so I can't check myself.
jenshannoschwalm
on 13 Nov 2019
@TurboGit, this stuff is completely broken. _ioppr_migrate_iop_order function re-orders iops for some reason. dt_ioppr_get_iop_order_list used in there returns a list with order from the iop order version passed to it as an argument, because it uses _ioppr_legacy_iop_order_step function with last argument (dont_move) set to FALSE. The whole iop_order.c file is a total mess of functions with no apparent logic to them, I doubt anyone can tell how it's supposed to work.
So current master breaks 2.6 history stacks. This is an obvious blocker for a release.
parafin
on 13 Nov 2019
356259332285b83faee87fecdfac62d7229c581e - this commit is the culprit. It should have just added new iops without re-ordering modules. That's what dont_move flag in _ioppr_legacy_iop_order_step function is for.
parafin
on 13 Nov 2019
And I don't understand how this mess is supposed to work. So you update iop order v1 to v2 because v1 misses some modules, and you are saying that every new module has to be added to v2. So say new iop is added now. But pictures already migrated to v2 won't get migrated again, so they won't have this new iop in the list anyway.
Instead images should be always updated to the latest iop order, but without moving iops. And every iop addition requires new iop order version it seems. Probably. Please oh please someone write a coherent description of how this whole iop order business should work. Right now this code looks like random stabs in the dark.
parafin
on 13 Nov 2019
@vsz1lu : can you upload a .xmp from 2.6.3 to reproduce ?
TurboGit
on 13 Nov 2019
@TurboGit nope. sorry, the latest version what i used the 2.6.2+13.... from your repository.
i can send xmps and nefs from this after UTC 18:00. i'm in my workplace at now.
vsz1lu
on 13 Nov 2019
Ok, I have a 2.6.2 XMP. Will look at this.
TurboGit
on 13 Nov 2019
@parafin : it may not be clear, but not sure it is that's bad :)
The v1->v2 conversion is done for all old edits. The conversion is needed for old edits (with old iop-order) to get access to new modules. And so for old edits we keep the 2.6 iop-order (the original one). That's why all new modules must be added into v2.
All new edits are done using the v5 iop-order and so all modules have to be described here.
The v3, v4 are legacy and broken order and should never come into play. It was an attempt to fix the big problem we had in September.
That being said we seem to have an issue in v2 where profile_gamma does not have a proper order.
TurboGit
on 13 Nov 2019
Here's the two sidecars from my testing on 2.6.3:
junkyardsparkle
on 13 Nov 2019
I believe this is fixed here #3409. Can you test?
TurboGit
on 13 Nov 2019
I'll test that in a few minutes.
Does this perhaps warrant pulling the rc0 release downloadables? The case for somebody who isn't using profile_gamma is more subtle, but still badly broken...
junkyardsparkle
on 13 Nov 2019
No, the issue is with colorin, colorout and dither, not profile_gamma, see _ioppr_move_iop_* calls here: https://github.com/darktable-org/darktable/blob/master/src/common/iop_order.c#L90
My point about new modules is the following scenario:
- image imported and edited in darktable 2.6
- user updates darktable to 3.0 and re-opens that image
- history gets converted to iop order v2 which includes new 3.0 modules
- new iops are added to 3.2 release
- user updates to 3.2 and re-opens the same old 2.6 image, which was already converted to iop order v2 in step 3
- new 3.2 iops I guess now experience #3071 bug
So your code comment that all new modules have to go into v2 iop order doesn't make sense I think.
parafin
on 13 Nov 2019
OK, I think my scenario from previous comment works fine actually. I have another question now: why not just modify _ioppr_get_iop_order_v1 function to just include all modules? iop order v5 is defined as a simple list too anyway, so it's not like these _ioppr_insert_iop_* and _ioppr_move_iop_* serve any purpose now?
parafin
on 13 Nov 2019
Also I notice that there's no _rewrite_order in v1->v2 conversion, so _ioppr_insert_iop_* there create fractional iop priorities. Just defining evenly spaced v1 order that includes all modules (like it's done for v5) is IMHO much simpler solution and doesn't create this problem.
parafin
on 13 Nov 2019
@parafin : I have added some comments in iop-order in the hope that it makes things clearer. I haven't said it is crystal clear :)
TurboGit
on 13 Nov 2019
@parafin : we may probably add a rewrite, but this is not a fix for the issue at hand. This will rewrite iop-order evenly spaced but still the order will be wrong regarding colorin/profile_gamma.
TurboGit
on 13 Nov 2019
I just mean that this whole v1->v2 conversion is unneeded.
parafin
on 13 Nov 2019
why not just modify
_ioppr_get_iop_order_v1function to just include all modules?
That could have been a solution, I didn't choose it because:
- I want to keep v1 for legacy modules
- I want to have v2 with new modules since dt 3.0
I don't think there is a better solution in this area.
TurboGit
on 13 Nov 2019
I don't understand. legacy modules don't go anywhere, and v1 is always converted to v2, isn't it?
parafin
on 13 Nov 2019
Does this perhaps warrant pulling the rc0 release downloadables? The case for somebody who _isn't_ using
profile_gammais more subtle, but still badly broken...
I'll try to have RC1 today or tomorrow and the issue at hand can be fixed by moving the right module at the right place.
TurboGit
on 13 Nov 2019
I don't understand. legacy modules don't go anywhere, and v1 is always converted to v2, isn't it?
I mean in the implementation. It seems far better to understand the code as it is. v1 is legacy, v2 is legacy + new modules. v3, v4 are mess... and v5 the iop-order for dt 3.0.
TurboGit
on 13 Nov 2019
Thanks for the fast works guys!
If you want to play more, there are an raw, an xmp and an exported result with dt v1.1~rc2 from 2012.
https://drive.google.com/file/d/1o8jzJrgEx-0KSjkO34LA2zNKjhW2MoVE/view?usp=sharing
please note:
In this version there was on a bug in the 'unbreak input profil' iop: the gamma slide control the linearity and the linear slide control the gamma. As I remember fixed in around v2.0.0 (?), but it's a living compatibility problem in today too.
vsz1lu
on 13 Nov 2019
Related issues
AxelG-DE
路
5Comments
schwerdf
路
4Comments
dim162
路
3Comments
jade-nl
路
3Comments
mjut
路
5Comments
Most helpful comment
@TurboGit, this stuff is completely broken. _ioppr_migrate_iop_order function re-orders iops for some reason. dt_ioppr_get_iop_order_list used in there returns a list with order from the iop order version passed to it as an argument, because it uses _ioppr_legacy_iop_order_step function with last argument (dont_move) set to FALSE. The whole iop_order.c file is a total mess of functions with no apparent logic to them, I doubt anyone can tell how it's supposed to work.
So current master breaks 2.6 history stacks. This is an obvious blocker for a release.