Project

General

Profile

Bug #11879

Spot removals are not saved in styles

Added by Gerd Sussner 4 months ago. Updated 3 months ago.

Status:
Triaged
Priority:
Low
Assignee:
-
Category:
Darkroom
Target version:
-
Start date:
12/24/2017
Due date:
% Done:

20%

Affected Version:
2.4.0rc2
System:
all
bitness:
64-bit
hardware architecture:
amd64/x86

Description

When I create a style from the history the contained spot removals are not stored (at least they are not applied) when the style is applied to a different image. The spot removal action is listed in the history of the new picture but it does not contain any spot.

History

#1 Updated by Gerd Sussner 3 months ago

I found out that the operation 'spot removal' is not enabled for stored in styles, i.e. the method flags() is missing IOP_FLAGS_INCLUDE_IN_STYLES.

#2 Updated by Gerd Sussner 3 months ago

Question: Is this done on purpose? In my opinion saving the spot removal make very much sense if you have a series of pictures in a row which all contain the same spots.

#3 Updated by Tobias Ellinghaus 3 months ago

  • Target version deleted (Candidate for next patch release)
  • System changed from Ubuntu to all

It would make sense, but the masks used are not part of the parameters, thus they won't end up in the style. So just adding the flag won't help.

#4 Updated by Gerd Sussner 3 months ago

Is there any reason why the masks are not contained in a style? Could it be done? Maybe only for specific modules.

#5 Updated by Gerd Sussner 3 months ago

I noticed that 'spot removal' uses masks of type circle, ellipse and path internally. However, they are not showing up in the mask manager.

This ticket could be closed if those masks would be displayed in the mask-manager and if ticket '11807' would be available. By this combination, it would be possible to apply a bunch of spot-removals to different images which contain the very same spots.

Question: Should this ticket be renamed to 'Add internal mask of spot removal to mask manager' (I am not able to modify the ticket name) or should I create a new ticket?

#6 Updated by Tobias Ellinghaus 3 months ago

  • Status changed from New to Triaged
  • % Done changed from 0 to 20

I don't think that would be a good idea. For one, that ticket is about exporting the masks to be used in other programs. And even if dt would be able to read them back in (which I don't consider viable) that would still be a nightmare of a workflow. To fix this properly we have to come up with a way to add masks used in modules to the presets and styles created.
Not showing the spot removal masks in the mask manager is on purpose, those quickly pile up and make the whole thing too crowded to be useful.

#7 Updated by Gerd Sussner 3 months ago

Suppose I want to support development on that bug. Is there some background information how things are organized? I.e. how operations or masks are stored in the database and what must be considered to avoid unwanted side-effects.
The tex-documents I found in doc/pdf/ looked promising but does not compile and the content seems to be outdated and very basic.

#8 Updated by Tobias Ellinghaus 3 months ago

Unfortunately I have to tell you that masks isn't exactly the best designed part of darktable's code. You would probably need to add a new (optional) API for modules to return a list of mask ids when handed a param blob. Then use that when writing styles and pasting params to also garb the masks used by a module and (in the case of styles) write them out or (when pasting) duplicate the mask and use yet another to-be-written API to update the param to use the new masks. Alternatively the module could do the duplication of the masks itself in a new callback.

#9 Updated by Gerd Sussner 3 months ago

Hmmm, maybe it's time for sitting back and think about a re-factoring. I have already wondered why you were going for an object oriented 'c'-approach - instead of using real c++. Modules, styles, etc. - would be a perfect example to use classes and inheritance.
Would it be an option? I would happy to participate in the huge task.
Personally, I think Darktable is going to be a real alternative to Aperture, Lightroom or any other RAW-developing software. The more it evolves the harder it would be to maintain it.
This also means that there should be a 'concept' written down somewhere. It would be very useful for new-comers to see what's going on instead of digging through the code.

What do you think?

#10 Updated by Tobias Ellinghaus 3 months ago

I don't think that doing a rewrite is either feasible nor wanted. We enjoy writing C code, and my experience is that C++ might be easier to write it is a lot harder to read and understand when getting to know a new code base. With C it's straight forward.

#11 Updated by Gerd Sussner 3 months ago

Allright. I hope you still will address this issue.

Also available in: Atom PDF