Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.76k stars 313 forks source link

Snapshot support/multiple PP3 per image support #2624

Open Beep6581 opened 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2641

As discussed in IRC and often requested in the Forum:
A feature request for being able to easily store and use snapshots and/or multiple
PP3 files for a single image.

The suggested behavior is for using the Snapshots panel to facilitate this.
Each PP3 file would encapsulate the parameters in a tag which contains the name of
the snapshot, something like [name=High key] or <High key>.
The setting of whether the processing profile should be stored next to the image or
in the cache would be respected, as should the preference of which PP3 to use - cache
or alongside.
The Snapshots panel should be populated with the name of the snapshots when you open
an image with such a multi-PP3 (or PP4? or MP3? :P) applied.
The user should be able to copy and paste either the whole multi-PP3 or just a sub-PP3
from the File Browser, Filmstrip and from the processing profile panel in the Image
Editor.
Tweaks made to the image when no snapshot is saved would go into the 'default' section.
There might be a need to make a new tag which tells RT which sub-PP3 to use: if there
are a few sub-PP3s, e.g. default, one named BW and one named Color, and you close RT
while you were working on Color, then either Color should replace the default section,
or a tag in the head of the PP3 could tell RT to use the one named Color.

Reported by entertheyoni on 2015-01-15 15:08:15

Beep6581 commented 9 years ago
May be worth encapsulating PP3 format in XML structure?

<pSets>

    <pSet type="current">
     <pp3 data=""/>
    </pSet

    <pSet type="snapShot" name="SnapShot 1">
     <pp3 data=""/>
    </pSet

    <pSet type="snapShot" name="SnapShot 2">
     <pp3 data=""/>
    </pSet

</pSets>

Reported by michaelezra000 on 2015-01-16 19:29:25

Beep6581 commented 9 years ago
Just for the ease of programming? May be. 

I suggest to make just a Header, not open/close tag, like "SnapshotMain", "Snapshot001"
etc. The sign of end of progile is EOF or next Snapshot. What is easier than this and
are there really any downsides of this straight solution?

Priority=low? Well, this is certainly underrated, it is a very useful feature. It is
only partially compensated with save/load dialog.

Reported by pinhuer on 2015-02-10 19:40:46

Beep6581 commented 9 years ago
could you parse such pp3 file?

Reported by michaelezra000 on 2015-02-10 21:27:13

Beep6581 commented 9 years ago
Easily. I hope that this pseudo is not too dirty.

.....
while(token=getToken())
{
    if(token~="^Snapshot")
    {
          snapnum++;
          parse();
      }
      else
      {
            snapnum=1;
            parse();
       }
       ....
}
.....

parse()
{
    ...
    if(token~="^Snapshot")
         end;
    ......
}

Reported by pinhuer on 2015-02-11 06:09:50

Beep6581 commented 9 years ago
However, I might be a bit naive without knowing the actual pp3 reading flow.

Reported by pinhuer on 2015-02-13 22:00:43

Beep6581 commented 9 years ago
I now know that Glib::keyFile is used for parsing the pp3 file.

I think that it may be splitted somewhere to preserve keyFile usage.

Reported by pinhuer on 2015-02-14 11:57:58

tacman commented 6 years ago

In the future, the snapshots will be saved to the PP3 sidecar file. For now, the history and snapshots are lost when you load a new photo in the Image Editor or close RawTherapee.

http://rawpedia.rawtherapee.com/The_Image_Editor_Tab#Processing_Profile_Selector

Intuitively, the behavior I'm expecting with Snapshot is similar to "Export" -- a prompt for a filename, basic parameters, and an option to save the sidecar file.

It seems odd that snapshots wouldn't be preserved -- at a minimum it'd be great if "Snapshot" saved a pp3 file, along with a reference to the original file, so the raw image could be reloaded with the profile set.

However, it doesn't feel like a global profile, so it seems odd to go to the Profiles menu and navigate around to the pp3 file that is associated with an export (which is not coupled with the snapshot).

When I re-open a raw file, I'd like to have access to all my snapshots, it's rare that I'd want to lose them just closing RawTherapee. Of course, sometimes I'd like to right-click and delete them.

Finally, it seems like snapshot should prompt for a name (so it could save the sidecar file), rather than defaulting to Snapshot N, and then renaming it (which would would require the sidecar file to be renamed if snapshot coulds be reloaded that way).

raceviper13 commented 6 years ago

I agree this should probably be in one PP3 file. Each additional file really affects how the custom profile builder works.

There would be a "working PP3 section" that would be the current PP3 unchanged.

Then, at the end of the PP3, there would be each additional PP3 (beyond the current working) in additional sections and the section names within that PP3 would be concatenated to the key name.

For instance looking at only the 1st two sections of PP3, here is how it may look for the working and additional embedded PP3s:

"main working PP3" [Version] AppVersion=5.3-436-g6824f613 Version=329

[General] Rank=0 ColorLabel=0 InTrash=false

"1st additional PP3" [PP3Instance BW] Version-AppVersion=5.3-436-g6824f613 Version-Version=329 General-Rank=0 General-ColorLabel=0 General-InTrash=false

"2nd additional PP3" [PP3Instance HighContrast] Version-AppVersion=5.3-436-g6824f613 Version-Version=329 General-Rank=0 General-ColorLabel=0 General-InTrash=false

Maybe we need another section to handle naming enumerating all of the available PP3 files included beyond the current working and the current main working PP3 user defined name.

I don't see this being too complicated. But then again, I am not a programmer for RT. What is the current view on how to handle this? Has anyone decided anything yet?

Floessie commented 6 years ago

Then, at the end of the PP3, there would be each additional PP3 (beyond the current working) in additional sections and the section names within that PP3 would be concatenated to the key name.

Maybe we need another section to handle naming enumerating all of the available PP3 files included beyond the current working and the current main working PP3 user defined name.

Rule of thumb: Don't squeeze hierarchical data into a flat structure. We could implement a backward compatible format with some quirks when deviating from the GTK INI parser and doing our own, or we could switch to a JSON (-like) format (PP4?) with only a GTK INI fallback parser but no PP3 writer.

I don't see this being too complicated. But then again, I am not a programmer for RT. What is the current view on how to handle this? Has anyone decided anything yet?

Not that I know of. As always: The devil is in the details.

tacman commented 6 years ago

Brainstorming: Is it possible to combine snapshots with "Save Current Image", and just keep snapshots flat (as pp3 files) rather than hierarchical?

Save Current Image already gives the option of saving the pp3 file. Perhaps "Snapshot" could be the same as saving the pp3 file without the associated jpeg?

There would need to be a link back to the original image, of course (to tie all the snapshots together). Since RT doesn't have a database of images and relationships, everything would need to be done via the filename and/or the meta-data in the pp3 file, which of course where the "devil in the details" comes into play.

Still, if the assumption is that the source file is in the same directory as the pp3 files, scanning the pp3 files on startup is pretty fast.

So "Snapshot" would the basically an image-specific profile.

Again, just brainstorming, I might be completely misunderstanding some key concept.

On Wed, Feb 21, 2018 at 4:29 AM, Floessie notifications@github.com wrote:

Then, at the end of the PP3, there would be each additional PP3 (beyond the current working) in additional sections and the section names within that PP3 would be concatenated to the key name.

Maybe we need another section to handle naming enumerating all of the available PP3 files included beyond the current working and the current main working PP3 user defined name.

Rule of thumb: Don't squeeze hierarchical data into a flat structure. We could implement a backward compatible format with some quirks when deviating from the GTK INI parser and doing our own, or we could switch to a JSON (-like) format (PP4?) with only a GTK INI fallback parser but no PP3 writer.

I don't see this being too complicated. But then again, I am not a programmer for RT. What is the current view on how to handle this? Has anyone decided anything yet?

Not that I know of. As always: The devil is in the details.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/2624#issuecomment-367264285, or mute the thread https://github.com/notifications/unsubscribe-auth/AAl0QXePf6cS6G63eocX1rOXnhR_m-tkks5tW-IHgaJpZM4SHmkz .

Beep6581 commented 6 years ago

@tacman you can save just the PP3, which means you can save snapshots to individual pp3 files: screenshot_20180221_145123

There's no need to complicate things, all snapshots are to be stored in the same PP3. The difficulty is not in figuring out how to implement it in a key-value pair PP3 file or a JSON file, it just that no one has done it yet.

tacman commented 6 years ago

Yes, but snapshots feel like image-specific profiles, and downloading them to the global profiles directory seems like it will clutter up that directory.

Plus, there's no link (other than the filename) from the profile back to the original picture, which seems fundamental to the idea of a snapshot.

On Wed, Feb 21, 2018 at 8:55 AM, Beep6581 notifications@github.com wrote:

@tacman https://github.com/tacman you can save just the PP3, which means you can save snapshots to individual pp3 files: [image: screenshot_20180221_145123] https://user-images.githubusercontent.com/5844619/36483597-e25424c4-1716-11e8-85ef-287fe86432b2.png

There's no need to complicate things, all snapshots are to be stored in the same PP3. There's no difficulty in figuring out how to implement it in a key-value pair INI file or a JSON file, it just that no one has done it yet.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/2624#issuecomment-367333050, or mute the thread https://github.com/notifications/unsubscribe-auth/AAl0QVZCH09A1sItrD9TSDRiLe84Cnsxks5tXCBdgaJpZM4SHmkz .

Hombre57 commented 6 years ago

@tacman

Yes, but snapshots feel like image-specific profiles, and downloading them to the global profiles directory seems like it will clutter up that directory.

When in the save dialog, you'll find a shortcut to your image's folder on the left.

Other than that, I agree that we're only lacking manpower.

@floessie Can JSON be human readable and editable ? Can JSON integrate binary blob as well if we want to integrate bitmap data for local editing in the future (not sure we want it, I'm just asking) ?

Floessie commented 6 years ago

@Hombre57 JSON is per se human readable (in contrast to XML :grin:). Wikipedia has some nice examples whereas the first link gives the formal definition. The only way to transparently add binary data is to escape those outside the one-byte representation (I read there are 94 one-byte characters) with \u00FF (where 00FF is the example value). This should be handled transparently by the JSON libraries. Another common solution is a base64 encoding.

thirtythreeforty commented 5 years ago

I would like to sponsor this feature for v5.6. I have wished for this several times, especially once I tried Capture One's implementation! I also think it should replace Snapshots, which is not very useful for me.

Is the consensus that this needs a new file format? I think it would be less work to use multiple PP3s although admittedly, wrangling them becomes more of a challenge. From a user's perspective, a single sidecar would be a nice experience.

I am not concerned about JSON's ability to store small binary data, although a Protobuf-based format could be better if lots of that is planned.

Beep6581 commented 5 years ago

All snapshots should be stored in one file. The point of this request is to be able to easily store and access various takes on processing the same image, e.g. one in color, one black-and-white, one for print, etc. I agree that JSON would be great.

thirtythreeforty commented 5 years ago

Okay, I will start thinking about a new format. Are there any other features that are waiting on a new file format?

Hombre57 commented 5 years ago

Other than being able to embed binary data, I'd say no. At some point we'll have to do compositing between the embedded pp3 entries, but giving a name or ID will suffice.

We also wanted to replace the pp3 file format by adopting the xmp one. Going JSON is not a transitional step, so I guess that we drop the xmp route ?

thirtythreeforty commented 5 years ago

Does XMP automatically give compatibility between RT and other programs? I know #3728 asks about supporting XMP rating tags. But RT's features don't map well into Lightroom, and vice versa.

Otherwise, as far as the user is concerned, the sidecar is just a binary blob. Savvy users may try to write their own, in which case I'd say they'd enjoy JSON far more than XML.

Beep6581 commented 5 years ago

It's an often-requested feature that RT be able to read color/number rankings from other software, e.g. from digiKam. Having said that, just the other day someone who uses darktable mentioned that they lost their image settings when digiKam edited the file's XMP. So while compatibility is nice, it's not necessarily a bad thing if we don't use the same format as other software, as doing so just for the sake of compatibility could come round to bite us in the buttocks.

agriggio commented 5 years ago

reading ratings from XMP and storing parameters in XMP files are two different things. It would be nice if RT supported the former, it would be horrible if it switched to the latter (IMHO, of course)

Hombre57 commented 5 years ago

Then it seem we have a consensus not to use XPM for the params. Adding multi-image version support to the actual format is possible as well, but I don't care and don't mind if you chose extending the actual format or switch to JSON.

thirtythreeforty commented 5 years ago

@Hombre57 how would it look to add multi-variant support to the current PP3 format? It's a flat file structure... Could be tricky to get hierarchy in it

Hombre57 commented 5 years ago

@thirtythreeforty With the section name ! Instead of using says [Vignetting], you could use [Vignetting#FooVersion]. A new [Snapshots] section would list all the versions (i.e. the snapshot name) and the actual default version, to be displayed in the file browser and that would be overwritten when pasting a profile. So ProcParams::save and ProcParams::load would look for this section first, then load all the snapshots in a loop.

Decision would have to be taken about the [General] section, should there be only one for all snapshots or one per snapshot ?

The codified section name with # separators would enable more than one level too.

thirtythreeforty commented 5 years ago

I see, makes sense. That would probably be a lot less work than switching to a new format. Does anybody see a reason this strategy would be a potential footgun?

More work would need to be done on the UI; we can discuss ideas for that or I can start hacking on the file parser.

On Sun, Nov 4, 2018, 10:45 AM Jean-Christophe <notifications@github.com wrote:

@thirtythreeforty https://github.com/thirtythreeforty With the section name ! Instead of using says [Vignetting], you could use [Vignetting#FooVersion]. A new [Snapshots] section would list all the versions (i.e. the snapshot name) and the actual default version, to be displayed in the file browser and that would be overwritten when pasting a profile. So ProcParams::save and ProcParams::load would look for this section first, then load all the snapshots in a loop.

Decision would have to be taken about the [General] section, should there be only one for all snapshots or one per snapshot ?

The codified section name with # separators would enable more than one level too.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/2624#issuecomment-435685306, or mute the thread https://github.com/notifications/unsubscribe-auth/ADxP5aTw8GPep2StFWHIQ9K2otthvxKLks5urxmigaJpZM4SHmkz .

Hombre57 commented 5 years ago

See also @Floessie's comment here ?

Thanatomanic commented 5 years ago

It's reasonable to think about the long run here. This is a rather major feature, so we'd better make sure we work towards something that can be sustainable for several future RT versions. I tend to go with Floessie's comment here that our current format is actually not really suitable for hierarchical data, and we can better invest time and effort into developing something different and more flexible.

(also, think about storing a timeline for the editing history)

thirtythreeforty commented 5 years ago

Yeah I think a new format would be much more versatile and a lot cleaner in the long run. Just a matter of how much work it turns into, and the timeline for 5.6 i suppose. I am fine with going either route, there are benefits to both.

Floessie commented 5 years ago

@thirtythreeforty

Just a matter of how much work it turns into, and the timeline for 5.6 i suppose.

No worries about the timeline for 5.6, as a new sidecar file format would probably require a new major number.

I see, makes sense. That would probably be a lot less work than switching to a new format. Does anybody see a reason this strategy would be a potential footgun?

Been there, done that. :wink: It's definitely feasible. In a project at work I've started from a legacy INI format and extended it like @Hombre57 proposed above, except I chose / as a seperator and # as the comment starter. Internally there is a Configuration class with key-value mappings taking booleans, integers, floats, strings, and Configurations. Later I changed the format to a downward compatible "structured INI" format with braces like so:

config_key {
    key = "value" # path is 'config_key/key'

   [legacy/config]
   key = true # path is 'config_key/legacy/config/key'
}

Coming from INI and key-value has one downside, though: There are no lists (or arrays in JSON speak). They should be part of the Configuration (or whatever it will be called) class from the beginning, as there are rare cases where order matters and I had to hack up something like 0/..., 1/....

The internal representation (Configuration in my case) should be format agnostic. There should be an abstract reader/writer interface (outside of Configuration, taking one as a parameter to read()/write()). E.g. I have readers/writers for legacy INI, structured INI, and a highly compressed binary format. Here we would have a legacy PP3 reader and a reader and a writer for the new format.

So the thing to start with should be the internal representation of the settings tree.

Just some thoughts from the back row.

HTH, Flössie

raceviper13 commented 5 years ago

Additionally, please consider:

  1. Updating the built-in profile generator to use the new sidecar. a. how will that affect the built-in profile builder handling?
  2. user created profile-generators (this is something I have, so I would have to re-write it if the INI structure if abandoned. a. how will that affect handling of user created profile-generators?

If it is necessary to abandon the INI format for a concrete reason, I can support that. I just hope my work to update what my builder does is not extensive.

JamesForBugReport commented 5 years ago

It would be nice for the thumbnail view to represent the different versions within the sidecar, or at least indicate that there are multiple versions and be able to select which one is used for the thumbnail. Probably also worth considering the processing copy/pasta context menus etc.

thirtythreeforty commented 5 years ago

@raceviper13 I would imagine RT would be able to import valid PP3s pretty much indefinitely so that you don't lose work created with previous versions. Of course, the output might look slightly different between versions as algorithms improve, but that has always been the case.

@JamesForBugReport I agree. I would suggest that the variants UI treat none of them as a "special" main version and let you choose the thumbnail'd version. Especially because computing a thumbnail is a relatively expensive operation, especially for XTrans images.

Considering copy/paste is important. Do you have suggestions for how that would look?

thirtythreeforty commented 5 years ago

Another option is to display one thumbnail for each variant; perhaps with a thin line around all the variants that are associated with a single backing raw image. That also solves the copy/paste workflow.

JamesForBugReport commented 5 years ago

@thirtythreeforty I like your proposal, it is much cleaner than what I had in mind, which was to have an additional layer to the submenu to indicate which profile the action should apply to.

fortiernor commented 5 years ago

Since @Hombre57 gave me an opening I'd like to suggest a few things. I am not trying to highjack this thread, just make a proposal and see if it's worth pursuing; I've never contributed to an open-source project before, so your patience is appreciated. I would like to see a persistent history (that incorporates edits made in previous sessions to a given image), so that a user can know at a glance what processing, and can go back into history to examine the effect of the different steps.

So I could open an issue about setting up tests, and then another one when/if I figure out a decent approach to ProcParams. I suppose I would offer patches. Does that make any sense?

Beep6581 commented 5 years ago

I would like to see a persistent history (that incorporates edits made in previous sessions to a given image), so that a user can know at a glance what processing

The processing history is messy, it's a heap of trial-and-error by eyeball, and so offers a very poor overview of what processing is actually being performed. As such, I think a persistent history is a bad idea. However, the end state, as currently recorded in a PP3 file, gives a good overview of what is actually being done. Snapshots save these "end states", and this issue is about being able to save these end states in a single PP3 file.

florentgluck commented 5 years ago

How about keeping the existing pp3 file format untouched, but having one pp3 file per snapshot?

Let's say the user is editing the photo image.raw. This first editing will produce an image.raw.pp3 file. Then, the user makes a few changes and decides to take a snapshot: image.raw.1.pp3 is created. For every new/additional snapshot, a new image.raw.n.pp3 is created (where n is >= 1 and is the next "available" integer, i.e. the first non already existing filename).

With such a scheme, there is no hierarchy, just a flat list of pp3 files associated to the original image. The relationship between the original (raw) file and a snapshot is just the filename: image.raw.pp3 or image.raw.n.pp3 where n is a numerical value >= 1. If a snapshot (or the original edit) is deleted, then the next snapshot will use the next "available" file name. For instance, let's say there are 3 edits (original image and 2 snapshots): image.raw.pp3, image.raw.1.pp3, image.raw.2.pp3. If the user deletes the first snapshot, we then get: image.raw.pp3, image.raw.2.pp3. The next snapshot that will be taken will use image.raw.1.pp3 as its file name.

In other words, nothing distinguishes a snapshot from the original edit. They are all handled the same and can be seen as a same thing: the state of an image edit at a point in time. The original edit can be seen as the first default snapshot.

The editor would have to be modified to remove the snapshot window. When a user takes a snapshot, a new thumbnail would appear and now the current image being edited becomes the new snapshot. Edits from this point on are saved in this new (current) snapshot. Then the user is free to edit another snapshot (or the original edit) and it will be this other snapshot file that will then be modified (and being written to).

The file browser module would have to be modified to simply display any pp3 file associated to an editable photo file: either the original file (if it exists, e.g. image.raw.pp3) or any snapshot files (e.g. image.raw.1.pp3, image.raw.42.pp3, etc.). In terms of display order, thumbnails of the various snapshots could be displayed in alpha-numerical order for instance.

This proposal also means a user could delete any pp3 associated to an image (whether the original photo edit or any snapshots) and RawTherape will keep working just fine.

I'm not a RawTherapee developer (yet), but I'd assume these changes should be fairly easy to implement (compared to proposing a new file format) while maintaining compatibility with the existing pp3 format.

What do you guys think?

tacman commented 5 years ago

Gut feeling is that I'd prefer the first one be called raw.0.pp3, instead of raw.pp3.

Something to think about: it might be better to have raw.0011.pp3, etc., so that filenames in alpha order are also timeline order.

Or use a timestamp instead of a sequential number.

Tac

On Tue, Aug 13, 2019 at 12:26 PM Florent Gluck notifications@github.com wrote:

How about keeping the existing pp3 file format untouched, but having one pp3 file per snapshot?

Let's say the user is editing the photo image.raw. This first editing will produce an image.raw.pp3 file. Then, the user makes a few changes and decides to take a snapshot: image.raw.1.pp3 is created. For every new/additional snapshot, a new image.raw.n.pp3 is created (where n is >= 1 and is the next "available" integer, i.e. the first non already existing filename).

With such a scheme, there is no hierarchy, just a flat list of pp3 files associated to the original image. The relationship between the original (raw) file and a snapshot is just the filename: image.raw.pp3 or image.raw.n.pp3 where n is a numerical value >= 1. If a snapshot (or the original edit) is deleted, then the next snapshot will use the next "available" file name. For instance, let's say there are 3 edits (original image and 2 snapshots): image.raw.pp3, image.raw.1.pp3, image.raw.2.pp3. If the user deletes the first snapshot, we then get: image.raw.pp3, image.raw.2.pp3. The next snapshot that will be taken will use image.raw.1.pp3 as its file name.

In other words, nothing distinguishes a snapshot from the original edit. They are all handled the same and can be seen as a same thing: the state of an image edit at a point in time. The original edit can be seen as the first default snapshot.

The editor would have to be modified to remove the snapshot window. When a user takes a snapshot, a new thumbnail would appear and now the current image being edited becomes the new snapshot. Edits from this point on are saved in this new (current) snapshot. Then the user is free to edit another snapshot (or the original edit) and it will be this other snapshot file that will then be modified (and being written to).

The file browser module would have to be modified to simply display any pp3 file associated to an editable photo file: either the original file (if it exists, e.g. image.raw.pp3) or any snapshot files (e.g. image.raw.1.pp3, image.raw.42.pp3, etc.). In terms of display order, thumbnails of the various snapshots could be displayed in alpha-numerical order for instance.

This proposal also means a user could delete any pp3 associated to an image (whether the original photo edit or any snapshots) and RawTherape will keep working just fine.

I'm not a RawTherapee developer (yet), but I'd assume these changes should be fairly easy to implement (compared to proposing a new file format) while maintaining compatibility with the existing pp3 format.

What do you guys think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/2624?email_source=notifications&email_token=AAEXIQKGU3ZPWLAMASKCTOTQELODLA5CNFSM4EQ6NEZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4GGTMA#issuecomment-520907184, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEXIQM2FGP2DDHUR4LFVILQELODLANCNFSM4EQ6NEZQ .

Hombre57 commented 5 years ago

@tacman What @florentgluck proposed is already something implemented in RT, i.e.

where n is >= 1 and is the next "available" integer, i.e. the first non already existing filename

For the idea itself, it's probably a better option than a new file format, you're right, and should be the way to go (at least I vote for it). However the implementation might not be this one.

The two possible models are :

  1. snapshots : save the current profile on disk and do not touch it again (i.e. can be loaded and deleted, but can't be modified afterward). Switching from one solution to the other would be done in a single History, as it's the case today. The general profile would be the currently displayed editing, be it a snapshot. Note that it should be possible to overwrite an existing snapshot, the Snapshot list would require minimal modifications to make this possible.
  2. fork : save the current profile on disk and let the user modify it afterward, and switch between forks at will. Each fork would have it's own History (at least would be a logical demand from the users).

Solution 1 is simpler to implement (that's the way it works now, with a single History).

Hombre57 commented 5 years ago

PS : The snapshot's name should be a parameter in the [general] section of the pp3 file.

Beep6581 commented 5 years ago

I would prefer a single-file solution, which works exactly as things work now except that snapshots are stored in the (single) sidecar file. Why would I want 42 sidecar files littering my drive, what would be the benefit? It's not like one day I will want to delete all *.raw.36.pp3. Besides, some photo managers (digiKam) can handle a sidecar file on file move/copy/delete operations, but I doubt they would handle multiple sidecar files. 1547339222718

Thanatomanic commented 5 years ago

I'm with @Beep6581 on this one. Snapshots in separate files makes very little sense to me. And since some people even want fewer pp3's (e.g. #2857), making more does not seem to be a good way forward.

Also, could we take a page out of Darktable's playbook here? They seem to have a working snapshots system and a persistant history in their software. Win-win?

Hombre57 commented 5 years ago

@Beep6581 @Thanatomanic Nevermind, let's go for one file only. The saving time for 3-4 params in a single file should be negligible compared to only 1 params. Will it still be the case if RT handles bitmaps for local editing (just projecting in a far horizon...) ?

florentgluck commented 5 years ago

From the comments above, it wasn't clear to me that the consensus proposal was a 2 file solution (one for the current pp3 file and one for the file storing the snapshots).

If we also want to tackle the support for a history, it would make sense to store a history of changes per snapshots. I think before deciding on how snapshots and history should be stored on disk, we should first define what type of behavior we want in terms of snapshot and history.

Darktable seems to store an image history to an xmp file. I assume the notion of snapshot is stored somewhere in its database. As far as Darktable's user experience point of view goes, I dislike the way both snapshots and history are handled. It just seems counter-intuitive to me.

I think the way snapshots and history should be handled is as follow. The user works on image.raw and does some edits. These changes are stored in the history for image.raw. At some point, the user creates a snapshot. The history of the snapshot is initially empty (upon snapshot creation). However, as soon as the user works on the snapshot, its history gets populated, but nothing is added to the history of the image the snapshot was created from.

This is the way snapshot and history work on both Lightroom and CaptureOne. I think it makes a lot of sense.

Does everybody agree with this snapshot/history behavior or do you guys envision something else?

Thanatomanic commented 5 years ago

@florentgluck

I dislike the way both snapshots and history are handled. It just seems counter-intuitive to me

Could you elaborate on that? In particular on the history part.

As for your description of snapshots, it resembles some sort of branching, right? Edit A becomes starting point for edit B, which could become a new starting point, etc. I think that is an elegant way to do it.

My gut feeling tells me that having the complete edit history in the snapshot, instead of starting 'clean', has benefits, but I cannot think of a very good argument.

Hombre57 commented 5 years ago

@florentgluck

From the comments above, it wasn't clear to me that the consensus proposal was a 2 file solution (one for the current pp3 file and one for the file storing the snapshots).

That's not what @Beep6581 said. The dual file mechanism is 1 image file + 1 sidecar pp3 file, nothing more.

Just for the record, each step in the History is a full blown pp3 parameter set. E.g., some disabled tools won't create an History entry if you move one of its slider, let's call it slider A (unfortunately, all tools doesn't behave like that). This seem logical since the tool is disabled, you won't notice the difference anyway. But then if you move another slider from another tool, the History entry will store the new value of slider A too, because it takes a snapshot of the whole parameter set.

That's why, with the actual History mechanism (and that's not going to change anytime soon), storing the History would mean storing huge files for rare use case, IMHO.

At some point, the user creates a snapshot. The history of the snapshot is initially empty (upon snapshot creation). However, as soon as the user works on the snapshot, its history gets populated, but nothing is added to the history of the image the snapshot was created from.

That's more easily doable.

Beep6581 commented 5 years ago

I'm :-1: for storing history. It's an unnecessary complication, bloat, and time would be better spent elsewhere, e.g. getting @agriggio 's Exiv2 branch merged.

Hombre57 commented 5 years ago

@Beep6581 We need subframe metadata support.

fferreres commented 4 years ago

Is this idea dead? I have pictures stemming from the same RAW, that each have quite different settings. Series of macro shots, where the image looks like Neon, Pearl, normal, etc. Is there any way to do versions of settings at least? I don't care (much) about History. Only not to lose my work or versions to a flat JPG.

Absent that, I will just duplicate the RAW at least to save the file settings.

Thanatomanic commented 4 years ago

This idea has halted, mostly because time and availability of our main developers is limited and no one has yet had the courage to take up this (complicated) project.

Duplicating RAW's however, is quite an elaborate workaround! You can save different processing parameters to different pp3-files as mentioned here: https://github.com/Beep6581/RawTherapee/issues/2624#issuecomment-367333050