Project

General

Profile

Feature #9094

Support for multiple instance of iop modules

Added by Henrik Andersson almost 6 years ago.

Status:
Fixed
Priority:
Medium
Assignee:
-
Category:
Darkroom
Start date:
11/27/2012
Due date:
% Done:

100%

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

Description

A side task to my work on masks is support for multiple instances which means
a module can be cloned and reused several times with different settings so
one can reuse a module for several masks with different settings. Foe example
the exposure module having 3 instance where the first one sets exposure of full
image, the other instance is hooked up with a mask for "burn" and the third
with another mask for "dodge"...

This task is somewhat big and i dont have time to take this one, so i report it
here and maybe one brave heart might embrace this into his arms...

dt_instances.jpg (87.4 KB) Aldric Renaudin, 12/01/2012 10:02 AM

dt_instances2.jpg (99.6 KB) Aldric Renaudin, 12/01/2012 11:08 AM

tabs.jpg (25.1 KB) Aldric Renaudin, 12/03/2012 08:49 AM

combo.jpg (25.3 KB) Aldric Renaudin, 12/03/2012 08:49 AM

rows.jpg (23.3 KB) Aldric Renaudin, 12/03/2012 08:49 AM

History

#1 Updated by juan luis fernández gallo almost 6 years ago

It would be great to have multiple instances for every plugin; for example, we could use colorize twice with different colors and blend modes to give different effects, or use local and controlled sharpen in combination with masks... nice feature, I hope someone will help with this!

#2 Updated by Mikko Ruohola almost 6 years ago

  • Project changed from website to darktable
  • Category set to Darkroom

#3 Updated by Henrik Andersson almost 6 years ago

juan luis fernández gallo wrote:

It would be great to have multiple instances for every plugin; for example, we could use colorize twice with different colors and blend modes to give different effects, or use local and controlled sharpen in combination with masks... nice feature, I hope someone will help with this!

Ofcourse the intention is for any plugin, i just used exposure as an eaxmple.

#4 Updated by Richard Wonka almost 6 years ago

Yes! +1 I've been fostering this idea for a long time. Started missing it when I wanted two watermarks in an image but couldn't be bothered to get into inkscape to create the necessary files.

Thank you thank you! :-)

#5 Updated by Aldric Renaudin almost 6 years ago

I just want to let you know that I made some progress here.
I can now duplicate/remove instance of modules inside dt directly.
You can check it here : https://github.com/AlicVB/darktable branch pipe if you want, but please remember that it is not stable at all.
It's just a very quick hack for instance.
presets, enable/disable, etc... doesn't work for instance.
ui is quite horrible
there is certainly a lot of memory leaks...
but it basically works.

I add a screenshot

#6 Updated by Aldric Renaudin almost 6 years ago

Another screenshot more "descriptive" :
Base : the image is monochrome
then I've added 3 instance of gnd module with different colors and angles

#7 Updated by Ivan Tarozzi almost 6 years ago

Wow! really interesting. thank you!

In screenshot I see the button on every tab to delete the istance. What about a button to enable/disable sigle istance instead of a global enable button?

I hope to have some time to test your branch.

#8 Updated by Aldric Renaudin almost 6 years ago

Here is some mockups for the ui part.
You may notice that the preset button has disappear, that's just because it's hidden in my branch for instance (but will reappear soon, don't worry)
tabs :
- should be hidden if only one instance
- take a lot of place
- you can quickly enable/disable/remove instances to see effects
combobox :
- take less place
- not very intuitive, as you have to select an instance to enable/disable/remove it
rows :
- we should add some buttons on the header : duplicate/remove/reorder some may go in a menu like for presets
- can clutter the right panel if you have multiple instances of multiple modules (but the user will know why !)
- in the future, we may add some "grouping" facilities, like we have on filmstrip

I tend to prefer the third way. It's the more intuitive, imho (and it's the easiest to implement... and to debug as it wouldn't involve to much code change)

#9 Updated by Ivan Tarozzi almost 6 years ago

May be I prefer the tabs solution, but I agree with you the choice must consider the ease of development and debug :)

Here my consideration about tabs:

1. It maintains more compact the right panel. And switching tabs I have plugin controls in place, without scroll up/down

2. I know tabs solution could be problematic (for space reasons) when a lot of instance are present. But let's think about a point: how many case to use more than 3/4 instance in real case?

3. important point: using tabs I can enable/disable all instance with one click. And apply a preset then could contains more instances (...)

just my 2c

#10 Updated by Mikko Ruohola almost 6 years ago

Ivan Tarozzi wrote:

2. I know tabs solution could be problematic (for space reasons) when a lot of instance are present. But let's think about a point: how many case to use more than 3/4 instance in real case?

just my 2c

Adding more 2c to the pile.
I think that the tabs solution is greate. just add a "+" tab when more than 4 tabs. it brings up a list like presets is now...

#11 Updated by Jérémy Rosen almost 6 years ago

what I see as strenght and drawbacks of each solution

  • combo
    • very easy to develop for a first prototype
    • the name of the layer is very visible
    • there is a good sense of grouping of instances of iop
    • the multi-instance aspect is not very visible, it's more a parameter of an iop than something showing that there are multiple iop
    • reordering of instance would badly fit in the UI
    • some space wasted when there is only one instance
  • tabs
    • the "natural widget" to express multiple instance of the same thing
    • very good grouping of multiple instances, it makes sense visually
    • names are totally invisible, basically naming instances makes no sense in such a small UI
    • reordering would fit weel with drag and drop
    • uses lot of real-estate when there is only one instance (most common case)
  • rows
    • namings is very visible
    • each instance has a whole header which leaves space to add buttons (presets, on/off individuall, add/delete button)
    • it's hard to have an instinctive reordering UI, drag and drop would work but it's not something that is obvious
    • there is no visible grouping (except for iop spatial proximity) that could be arranged, but it's not the case in this mockup
    • no space wasted when there is only one instance

some of the drawbacks could be worked out (like hiding tabs when there is only one instance) but overall the "rows" approch is the one that I think fits the best in DT. It needs some refining (how to rename, how to add/delete instances, how to reorder, but it's my personal favorite...

#12 Updated by Aldric Renaudin almost 6 years ago

For the testers, I've recode the "row" approach from scratch (the pipe branch was really messy with all the tests I've done...)
you can find it https://github.com/AlicVB/darktable branch multi_row
all is not in place at the moment so, please be patient ! (same for the header icons ;)

#13 Updated by Ivan Tarozzi almost 6 years ago

Thanks! I'm playing with this and pipe version :)

Just few notes:

1. have you changed the widget aspect? In original darktable, all widget are enclosed in a grey box, in your branches I can see black border around each widget

2. are you thinking about how exclude some modules from multi-instance feature?
because I think could not have sense multi-instance for:
- all base group
- levels?
- tone curves?
- in/out color profile
- ... may be others

#14 Updated by Henrik Andersson almost 6 years ago

2. are you thinking about how exclude some modules from multi-instance feature?
because I think could not have sense multi-instance for:
- all base group
- levels?
- tone curves?
- in/out color profile
- ... may be others

You can define a new IOP_FLAG so that module can flag if they support clones, however
there are even fewer the Ivan mention above that doesnt make sense.
exposure, levels tonecurve do make sens when in conjunction with maska/blendif however
color in/out does not..

#15 Updated by Aldric Renaudin almost 6 years ago

I think the multi_row approach is ready for preliminary tests.
you can find it https://github.com/AlicVB/darktable branch multi_row
Implemented :
- add/delete instances
- move up/down instances
- apply presets to instances
- exclude flags for iop

Not implemented yet :
- styles
- rename instances

List of excluded module is deliberately very restraint for the moment. Let's test which iop cause problems.
As usual : be careful, this may destroy/corrupt your database, xmp... so backup everything before test !

#16 Updated by Tobias Ellinghaus almost 6 years ago

  • Target version set to Candidate for next minor release
  • % Done changed from 0 to 50
  • Status changed from New to In Progress

I'm setting this to "In Progress" since I am not sure how much work is left. The basic functionality is in master. Feel free to set it to "Fixed".

#17 Updated by Wolfgang Kühnel over 5 years ago

I wanted to mention that the multi-instance feature currently has an issue in combination with the
"compress history stack" feature.

Two active instances of an iop module show up as two separate entries in the history stack in an "on" state.
Activating the "compress history stack" feature disables/removes/deactivates the lower (older), but still
active instance of the iop module and is resetting it to it's default values at the same time.

This might not the behavior the user expects, as she would not have openend two active
instances of the same module. I suggest that the multi-instances of a iop-module are "chained" in
respect of the history stack, so that they appear as a single entry in the history stack. I don't
have a patch for this, although.

Best regards!

#18 Updated by Simon Spannagel over 5 years ago

  • % Done changed from 50 to 20
  • Affected Version set to 1.1.1
  • Tracker changed from Feature to Bug
  • Status changed from In Progress to Triaged
  • Priority changed from Medium to Critical

Uh, thank you Wolfgang!

This is a serious bug which causes data loss. This has to be fixed soon!
Changing tracker to Bug and Priority to Critical.

#19 Updated by Aldric Renaudin over 5 years ago

Thanks for the report, I will have a look ASAP.

#20 Updated by Aldric Renaudin over 5 years ago

ok, this should be fix in master.
thanks for the report

#21 Updated by Jérémy Rosen over 5 years ago

  • Priority changed from Critical to Medium
  • Tracker changed from Bug to Feature
  • Target version changed from Candidate for next minor release to Candidate for next major release

Ok I have updated to what makes sense to me, but I may be wrong...

why is that a bug and not a FR ? I moved it but there might be a something I missed

and afaik this hasn't been released yet, so I changed to "next major release" but again I could be wrong so feel free to correct

#22 Updated by Simon Spannagel over 5 years ago

  • % Done changed from 20 to 100
  • Status changed from Triaged to Fixed

Sorry for the misunderstanding. Of course the feature is a feature request and the attached big should have been treated separately. But since the feature is ready and in git master I abused this ticket to focus on the history stack bug.

Anyway, it's implemented, so I'm closing this.

@Wolfgang: if the bug still exists please open a separate issue for this. Thanks!

#23 Updated by Wolfgang Kühnel over 5 years ago

Works perfectly now. No need to file a bug.
Great work. Thanks!

Also available in: Atom PDF