Darktable: altered / not altered status and history / module_order inconcistencies

[phweyland]

The altered / not altered status is available

• as an icon on thumbnails in lighttable
• as history entry in collect module

What I see:

• importing raw image without xmp and auto-apply options disabled sets the image as altered in history and on thumbnails.
• opening an image from history/not altered in darkroom and leaving darkroom without alteration sets the image as altered in history
• with history/altered selected, a lot of images haven't the icon altered
• EDIT: change white balance of a not altered image (thumbnail) and back to lighttable, the thumbnail is still not altered (but with different color :))

Related to that, images found in history and module_order are not kept consistent.

What I see (in database):

• import a nef file without xmp, adds an image is history but not in module_order done
• remove an image, removes it from history but not from module_order done

The altered / not altered status should be consistent across the UI and the user actions.
The tables history and module_order should stay consistent and, in some way, in agreement with the altered / not altered status.

[phweyland]

From my standpoint this may be qualified as enhancement but that represents several bugs...

The routine dt_image_altered() is too simple to be accurate, but before trying to improve it, I think we can try to define properly what should be altered / not altered status.

Let's start. Please comment and complete as needed.

• just imported without auto-apply option nor xmp, the image should be set as not altered. The current list of modules and their state represent the not altered status.
• opening an image in darkroom without making any change should let it not altered
• any change made in darkroom, even change on an already existing module (white balance for example), should set the image as altered
• any move back to initial state of modules should set the image back to not altered (by ctrl-Z or by any other mean)
• at import time, if there is an xmp file, the altered / not altered status in xmp should be preserved.

open items

• if auto-apply options are set, do we want to have the status altered or not altered ?
• for the internal mechanisms of dt, has the status altered / not altered the same meaning ?

[TurboGit]

The current altered status is based on the fact that an image does not have a module that would change its rendering. As you find out for a RAW the white-balance is considered as a non-edit, this comes from dt_image_altered (let me put the code here):

           "SELECT 1"
           " FROM main.history, main.images"
           " WHERE id=?1 AND imgid=id AND num<history_end AND enabled=1"
           "       AND operation NOT IN ('flip', 'dither', 'highlights', 'rawprepare',"
           "                             'colorin', 'colorout', 'gamma', 'demosaic', 'temperature'%s%s)",
           basecurve_auto_apply ? ", 'basecurve'" : "",
           sharpen_auto_apply ? ", 'sharpen'" : "");

That being said, this has always been something confusing for many user.

What I would propose is to have the "altered" status to be ON only after actually having done something on the image, this means:

• a copy/paste of some history
• just entering the darkroom (as this sets the module order anyway)

If we really want to be picky and maybe more user friendly we can:

• check the current history of image when entering darkroom
• check the history when leaving darkroom and set module_order ONLY if something has changed. This can be done by computing a hash for the current history list or maybe just checking that the undo list is empty. To be checked.

And the "altered" status is reset when:

• deleting the history

If we agree on that we are probably very close to already have this status at hand. If I'm not mistaken this is exactly the version in module order.

• version = 0 && iop_list == NULL or no entry in module_order (image non altered)
• all other cases (version > 0 or version == 0 && iop_list != NULL) means image altered

How does that sound ?

[TurboGit]

And of course we keep dt_image_altered() as is as it is used to update or not the mipmap cache.

[phweyland]

What I would propose is to have the "altered" status to be ON only after actually having done something on the image, this means:

* a copy/paste of some history

* just entering the darkroom (as this sets the module order anyway)

Ok for the first one, not for the second one. That's not understandable for a user.
EDIT. For the first one, if the copied history move back the image to its initial state the image should be back not altered.

dt_image_altered

There is another bug in that routine. If you change the configuration of auto-apply after having imported you don't get the same status.

[TurboGit]

Ok for the first one, not for the second one. That's not understandable for a user.

Ok, so for the second one we want the next part I described in being picky :) And I agree it is more work but certainly far better for end-users.

[phweyland]

Ok for the first one, not for the second one. That's not understandable for a user.

Ok, so for the second one we want the next part I described in being picky :) And I agree it is more work but certainly far better for end-users.

Agreed.

[phweyland]

computing a hash for the current history list

That's probably an excellent way. Keeping the initial hash of history (in module_order) we could say at any time if the image is altered or not. If I'm not mistaken it answers all the requirements I can think about. It even can be passed through xmp file.
But that may imply a change on the current module_order behavior. We need an entry there at import time (and not only at edit time).

[TurboGit]

That's probably an excellent way. Keeping the initial hash of history (in module_order) we could say at any time if the image is altered or not.

This cannot be done for module_order which keep only the order of modules. We need a hash in the in-memory list of current history (dev->history) and the hash should use the operation name, the module params and blend_params.

[phweyland]

That's probably an excellent way. Keeping the initial hash of history (in module_order) we could say at any time if the image is altered or not.

This cannot be done for module_order which keep only the order of modules. We need a hash in the in-memory list of current history (dev->history) and the hash should use the operation name, the module params and blend_params.

If you stick on the table name ok. But in fact for every image you keep the type/version of the ordering of modules. Adding a field (hash) representing the full set of modules in history table (operation + op_params) for a given image gives exactly (I think) what we need and doesn't change the nature of the table (one entry per image).

EDIT: to be sure to be clear, in module_order we would keep only the initial hash. The calculation you describe would just be compared to that initial hash.

EDIT2: if you don't like the idea to put the hash in module_order, it could be set in images table itself.

[TurboGit]

No the hash should be part of the dev structure in dt. Set when opening an image and compared when leaving it. There is no need to keep the hash anywhere I think.

EDIT: My idea was to use the hash only to detect a change while in darkroom. But maybe you have another use in mind?

[phweyland]

No the hash should be part of the dev structure in dt. Set when opening an image and compared when leaving it. There is no need to keep the hash anywhere I think.

I would not agree on this. That way you check only what does happen in darkroom session. A lot of things may happen outside (copy of history or whatever).
Keeping that value at creation time in a table will survive all the life of the image.
The routine dt_image_altered() would just have to calculate current hash, compare it to the reference to provide an accurate and stable status.
Then if you use xmp for a not altered image, it makes possible to import it again (with xmp) but keeping the not altered status (which is true).

[TurboGit]

Ok, I see your point which I find valid.

But you certainly cannot calculate the hash in dt_image_altered() which is used in lighttable to display the modified icon. Computing the has for every image in the lt view (so without a dev structure) means that you'll have to compute it by taking the history in the DB for each image! This would be very very slow.

A better approach is probably to have dt_image_altered() return a diff from two hash stored somewhere. The original one (as discussed) and the current one updated each time we do something on a picture. But again this is far from trivial as (for example) copying history is done with the dev-history loaded when pasting on current image but is done directly on the db when pasting from lighttable. And again computing hash from the DB doesn't sound a nice idea performance wise.

[phweyland]

A better approach is probably to have dt_image_altered() return a diff from two hash stored somewhere. The original one (as discussed) and the current one updated each time we do something on a picture.

Agreed.

computing hash from the DB doesn't sound a nice idea performance wise.

If we do as you describe it's only done on some specific user's actions... Do you have an idea of the size of data we need to hash ? The time that would take ?

But again this is far from trivial

I can guess you are right. :)

And of course we keep dt_image_altered() as is as it is used to update or not the mipmap cache.

The context of mipmap cache sounds different to me. If for the user, that's the comparison between the initial import and the current state, for the cache it is the comparison between the last cache update and the current state. Shouldn't we need different routines for user and cache ?

[TurboGit]

If we do as you describe it's only done on some specific user's actions... Do you have an idea of the size of data we need to hash ? The time that would take ?

The size is hard to guess this is:

N * (OPLEN + OP_PARAMS + BLEND_PARAMS)

with

• N the number of enabled item in history
• OPLEN is max 20 (length of operation name)
• OP_PARAMS is a blob on my DB min size is 1 , max is 16600, average is 450
• BLEND_PARAMS is a blob on my DB min size is 300, max is 348 average is 325.

[phweyland]

I'll make a try on this. Any direction to start with is welcome.

[TurboGit]

Wait this won't work ! Taking the hash of op_params (or blendop_param is not stable as when a module is update (going through legacy_params) the hash will change but the image won't be this does not mean that the image has been altered!

[phweyland]

Taking the hash of op_params (or blendop_param is not stable as when a module is update (going through legacy_params) the hash will change but the image won't be this does not mean that the image has been altered!

If I understand properly, this is only an issue if the image is not altered. So in that case it's enough to replace the original hash (or the last cache one) by the new one.
For altered images, we just update the new hash.

[TurboGit]

Yes, you understand correctly but this must be done at the point of updating legacy params (and same for blendop_params BTW). Of course we want to do that in a generic procedure "somewhere" as we don't want to impose such house keeping in each iop module.

[phweyland]

Of course we want to do that in a generic procedure "somewhere" as we don't want to impose such house keeping in each iop module.

Cela va sans dire ...

But don't worry I don't know yet where to start. :)
Just looking at dev->history stuff.

[TurboGit]

Any direction to start with is welcome.

What I would do is have a routine to compute the hash in history.h

int64_t dt_history_compute_hash(GList *history);
(not that a simple hash CRC like may be faster than MD5 or SHA-1).

And then one can get the history item from any image id. I think we already have one routine to do that. And then we can do some timing to see where we go. Is that ok for a start?

[phweyland]

Is that ok for a start?

Fine ! Thanks

[TurboGit]

Thanks to you ! You are taking an important project which is really needed.

[johnny-bit]

Wow! That was interesting convo...

Just to keep it "current", how do you decided to have "altered" state ultimatelly from user's standpoint?

Eg: Image is considered not altered when:

• just imported without auto-apply option nor xmp
• opening an image in darkroom without making any change (if there are no auto-apply presets)
• at import time, if there is an xmp file, the altered / not altered status in xmp should be preserved.

Image is considered altered when:

• any change made in darkroom, even change on an enabled-by-default module (like white balance)
• any auto-apply preset was applied (at import/export time that would result in image being actually somehow processed)
• has history pasted that was in altered state (copying histroy from not altered into not-altered shouldn't change anything? unless there are differences in module settings...)

Image has "altered" status cleaned when:

• it's history has been reset to default.

For completness sake:

• altered icon is present for all images considered altered
• every image in "altered" history collection is guaranteed to have altered icon
• every image in "non altered" history collection is guaranteed not to have altered icon
• every image in "none" module order collection is guaranteed not to have altered icon
• module version change (even the most earliest one, like eg demosaic), does not change (non-)altered state of image.

Have I got this right?

[phweyland]

Have I got this right?

I don't know if we have already a full consensus but I agree with your description, at least as an objective to be reached.

Other constraint mentioned by Pascal which can be added to your list: new module version should not make loose the not altered status.

[GrahamByrnes]

A possibly ignorant question: the summary by Johnny-bit would seem to depend on the definition of a default image... since every image from a camera will have been through a demosaic, raw-black point etc.

At the same time, we want invariance under re-specification of modules.

So what happens if the default setting of an obligatory module changes? Or if I choose to do my demosaic with VNG4 rather than Amaze, or if some code comes to set a default ISO at which there is a change from Amaze to VNG4? Or someone invents "MoreAmazing"?

I can see there being a difference between "unaltered beyond the minimum at the time of processing" and "as would be produced by current default processing." It could be argued that the defaults will only ever move in the direction of "better"... but maybe I don't trust that assumption...

[phweyland]

if I choose to do my demosaic with VNG4 rather than Amaze, or if some code comes to set a default ISO at which there is a change from Amaze to VNG4? Or someone invents "MoreAmazing"?

In any of these cases you have altered the initial import, haven't you ?

I agree there is a grey zone (basic modules, auto-apply, your examples, ...) to define the reference.
I would start with the simplest solution (if we get there). Then, depending on user feelings we may add some configuration points to say: consider auto-apply as altered or not, ...
EDIT. or having a function to update / redefine the reference.

[parafin]

If user has auto-applied presets, then just opening the image will mark it as altered. I see a contradiction here with defined goal that "opening an image in darkroom without making any change" shouldn't mean altered. Why make this feature totally useless for users with such setup?

[phweyland]

If user has auto-applied presets, then just opening the image will mark it as altered.

Why do you say that ?

If we decide to consider auto-applied presets as an alteration, the image will be shown altered before you open darkroom.
If we decide the opposite, or we have configuration point for that, the image will be shown as not altered, as long as you don't make change.

[GrahamByrnes]

EDIT. or having a function to update / redefine the reference.

I think that's the issue. Suppose I run an image through today's default, and I love the result. Tomorrow, the default in a mandatory module changes. Maybe I will not love the updated version.
Do I get to choose if I want it updated or not? Do I get warned that exporting the image now will not get me what I loved earlier?

It's possibly trivial...

[parafin]

If user has auto-applied presets, then just opening the image will mark it as altered.

Why do you say that ?

If we decide to consider auto-applied presets as an alteration, the image will be shown altered before you open darkroom.

OK, doesn't change the point that if user has some auto-applied preset that, say, changes demosaic algorithm for all images, then altered flag becomes totally useless for him.

[TurboGit]

I think that the idea was that the initial hash would be computed AFTER the auto-applied modules are created. There is a single point where this happens and only for new edits.

[TurboGit]

And as Philippe says, nothing is graved in stone and whatever makes more sense will be implemented.

[phweyland]

Suppose I run an image through today's default, and I love the result. Tomorrow, the default in a mandatory module changes. Maybe I will not love the updated version.
Do I get to choose if I want it updated or not? Do I get warned that exporting the image now will not get me what I loved earlier?

For me that's another issue. If somebody changes a module behavior, he should update the legacy_params() routine to make sure the previous user settings are not lost. At least that what I expect... (we have seen some failures, right ?)

For our concern here, we have to make sure that legacy_param() doesn't affect the reference point (assuming it does the right job not to loose your loved image version).

[TurboGit]

Let's also add that today the altered status is quite broken:

• fresh image, go to darkroom, activate exposition (let all default, no action at all) => image altered
• fresh image, go to darkroom, rotate 90° image, back to lighttable => image non altered

So whatever we do there is most probably going to be better :)

[todd-prior]

@TurboGit @phweyland Just my 2 cents but could you work on a model a bit like RT with their profiles....there if you want a clean image you apply the neutral profile (this could be selected as a default) then it is clean nothing done. Otherwise it shows as custom. So for DT you could have neutral which might be all the mandatory modules nothing else with no changes. Then custom if you modify this in any way and I suppose you could allow for named versions of custom and say a third called default which would encompass any auto applied presets and modules and would be defined by the user so this would be their "default" profile on opening..so then images show an indicator of processing status vs altered and unaltered ...none (neutral), custom (something altered) and default to indicate only user selected defaults had been applied.....You could even replace the +/- indicator on the image with say letters N, C, D and hovering could still show the modules if you wanted to as it does now. This is all over the place but maybe also working on some version of this approach might help with sorting out styles....Maybe its a stupid idea but just started thinking out loud.....

[phweyland]

neutral profile

or neutral / default style ?
Probably Pascal knows if a style is a good representation of history + module order.

By extension your suggestion made me think about snapshot. I would love to have the snapshots less volatile and to keep them along with the image data. Just keeping history + module order...

[todd-prior]

@phweyland @TurboGit I was going by memory that there was some debating on how to get styles sorted out but I was trying to in my head think of a way to simplify the task that you guys are trying to solve as it is complicated with so many inputs in the equation and so many combinations. Thats what made me think of tracking things in terms of a profile rather than altered and not altered. So clean would be only the base modules and nothing activated in preferences. Then to work with auto presets you could incorporate that into a profile called default so all settings auto applied would be defined...any deviation from default or clean would be custom. Using this broad approach or some more well thought out one would potentially capture most eventualities I think but likely its more complicated...As to your snapshot comments are you aware of Picture WIndow photo editor. Jonathan Sachs of Lotus 1-2-3 fame is the authour. He has a graphical tree that works as a history as well. It might not have made sense before and its a ways off for Dt but now that you can rearrange modules in darktable you should check it out. Its like a folder tree but with each processing step has a linked thumbnail image so you can see the result as you add modules...you can copy and rearrange then and or use any resulting image in the tree as a starting point so lets say you add exposure and contrast then you can fork at that point and process the image with two or more pipelines in parallel...its really cool and you can put it on a second monitor....This program is now free and runs only on WIndows currently but it has a lot of cool features.....This tree can be saved as a script and used to batch process...its very flexible. Clicking on any image in the tree updates the preview so you can walk through it like having a snapshot for every step and you can easily do a before and after at each step or deactivate a step to see how that affect the final image and any image in the tree can serve as an input for a blending mode with another so it doesn't support layers but the images in the tree can be blended eg say a high pass filter...its quite clever....

[phweyland]

@todd-prior trying to elaborate on your ideas.

In a first time I would consider the above branches as snapshots. Each snapshot is then a set of (id, history, module order).

As you suggest we could have standard ones (neutral, default). There signature would gives us a quick and consistent mean to qualify the images (on thumbnails) and to sort them out (collection) on lighttable.

At the darkroom level the user could easily switch from a snapshot to another and make comparison between different versions as of today. The difference would be the persistence of these snapshots (saved in db and xmp).

Implementing that we could register the parent of each snapshot to be able in the future to represent them in a tree fashion as the above screen shot describes.

[todd-prior]

Philippe, thanks for your thoughts ….I was just throwing something out there….unfortunately I am not any good with code and so not helpful but I have 30 years of working in a research lab and using software often cryptic to drive instruments and run analysis so sometimes I can see optimization possibilities….maybe when I retire I will find time to learn some coding…..thanks again for your feedback…..good luck if you press on with the snapshots etc….

From: Philippe [e-mail]
Sent: February 19, 2020 10:04 AM
To: darktable-org/darktable [e-mail]
Cc: Prior,Todd [e-mail]; Mention [e-mail]
Subject: Re: [darktable-org/darktable] altered / not altered status and history / module_order inconcistencies (#4313)

@todd-priorhttps://github.com/todd-prior trying to elaborate on your ideas.

In a first time I would consider the above branches as snapshots. Each snapshot is then a set of (id, history, module order).

As you suggest we could have standard ones (neutral, default). There signature would gives us a quick and consistent mean to qualify the images (on thumbnails) and to sort them out (collection) on lighttable.

At the darkroom level the user could easily switch from a snapshot to another and make comparison between different versions as of today. The difference would be the persistence of these snapshots (saved in db and xmp).

Implementing that we could register the parent of each snapshot to be able in the future to represent them in a tree fashion as the above screen shot describes.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/darktable-org/darktable/issues/4313?email_source=notifications&email_token=AKS2ESYB7F6VGLSJXU6CSA3RDVC4TA5CNFSM4KWPOO42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMIFIOY#issuecomment-588272699, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKS2ES2TY2WSNQ27XOGM443RDVC4TANCNFSM4KWPOO4Q.

[github-actions[bot]]

This issue did not get any activity in the past 30 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.