Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread Aurélien Pierre
Hi,

I'm the author of the lighttable's compress history button. White
balance and highlight reconstruction should never be turned off, so what
would be the purpose of having them disabled in the first place ? Then,
we can discuss what the history compression should do/avoid/prevent.

Aurélien.

Le 12/06/2019 à 11:40, dt-l...@stefan-klinger.de a écrit :
> thokster (2019-Jun-11, excerpt):
>> Hi,
>>
>> Am 11.06.19 um 15:32 schrieb dt-l...@stefan-klinger.de:
>>
>>> This option is not about saving disk space, but rather about cleaning
>>> up.
>> Is the "compress history" button in lighttable view doing anything else?
> Woha.  I have been talking about "compress history" in darkroom, not
> lighttable.
>
> They seem not to use the same code internally.
>
> I have to admit that I have not thought about the "compress history"
> button in lighttable.  That one already seems to remove switched-off
> modules in current master [1].  And it seems to do this incorrectly
> wrt. parafin's email:
>
> * In darkroom, disable "white balance" and/or "highlight
>   reconstruction".
>
> * Go to lighttable, select that image, and apply "history stack →
>   compress history"
>
> * When you open the image again, "white balance" and "highlight
>   reconstruction" will be enabled again.
>
> So if compressing should not change the image, then my current
> implementation [2] is even more correct, although it only is
> applicable from darkroom, not lighttable.
>
> Cheers
> Stefan
>
> 
> [1] 2.7.0+1443~g9bfbb225e
> [2] 
> https://github.com/darktable-org/darktable/compare/master...s5k6:compressHistory
>
>
> --
> http://stefan-klinger.deo/X
> I prefer receiving plain text messages, not exceeding 32kB. /\/
>   \
> ___
> darktable developer mailing list
> to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org
>

___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org

Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread Edgardo Hoszowski
Edgardo Hoszowski (2019-Jun-12, excerpt):
> > > Edgardo Hoszowski (2019-Jun-11, excerpt):
> > > > I usually want to keep the (off) modules when compressing history,
> > > > so the current behavior should be kept.
> > >
> > > My implementation already takes care about all these exceptional
> > > modules I'm aware of: parafin has pointed out "highlight
> > > reconstruction" and "white balance".  There's also "orientation" which
> > > internally seems to be done by the same module as "highlight
> > > reconstruction".
> >
> > My point is that you should use the setting instead of hard-codding the
> > modules.
>
> Hmmm.  I'm not sure I understand.  Are you saying that it schould be a
> configuration option which modules to remove when disabled?
>

There's a flag somewhere, don't remember if in the iop's .c or at compile
time, that tell if a module is enabled by default, the process should read
that flag.


> This would add quite some complexity.
>
> In order to entirely delete one individual module from the history
> stack, I'd rather slightly extend the "multiple instances → delete"
> menu item the IOPs.  It does exactly that for non-last instances.
> This menu item is inactive if it's the last instance (although that is
> buggy as well), but could easily be extended to just remove this
> module's entries from the stack.
>
> Also, remember that the exemption to not delete some particular
> modules when they are disabled is a work-around for the questionable
> decision to not include these modules in the history stack, although
> active, and although they can be disabled.  If this inconsistency is
> resolved, there would not need to be such an exception.
>
>
> --
> http://stefan-klinger.deo/X
> I prefer receiving plain text messages, not exceeding 32kB. /\/
>   \
> ___
> darktable developer mailing list
> to unsubscribe send a mail to
> darktable-dev+unsubscr...@lists.darktable.org
>
>

___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org



Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread dt-list
Edgardo Hoszowski (2019-Jun-12, excerpt):
> > Edgardo Hoszowski (2019-Jun-11, excerpt):
> > > I usually want to keep the (off) modules when compressing history,
> > > so the current behavior should be kept.
> >
> > My implementation already takes care about all these exceptional
> > modules I'm aware of: parafin has pointed out "highlight
> > reconstruction" and "white balance".  There's also "orientation" which
> > internally seems to be done by the same module as "highlight
> > reconstruction".
>
> My point is that you should use the setting instead of hard-codding the
> modules.

Hmmm.  I'm not sure I understand.  Are you saying that it schould be a
configuration option which modules to remove when disabled?

This would add quite some complexity.

In order to entirely delete one individual module from the history
stack, I'd rather slightly extend the "multiple instances → delete"
menu item the IOPs.  It does exactly that for non-last instances.
This menu item is inactive if it's the last instance (although that is
buggy as well), but could easily be extended to just remove this
module's entries from the stack.

Also, remember that the exemption to not delete some particular
modules when they are disabled is a work-around for the questionable
decision to not include these modules in the history stack, although
active, and although they can be disabled.  If this inconsistency is
resolved, there would not need to be such an exception.


--
http://stefan-klinger.deo/X
I prefer receiving plain text messages, not exceeding 32kB. /\/
  \
___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org



Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread Edgardo Hoszowski
> Patrick Shanahan (2019-Jun-11, excerpt):
> > important is to not *break* already processed images by a change.
>
> Yes, I'm aiming at that.  AFAIK, my current implementation would not
> break anything, see below.  (The implementation reachable from
> lighttable, which is already in master and was not built by me, can
> break images, do so in bulk mode, not undoable, and without showing
> the effect in lighttable).
>
> Edgardo Hoszowski (2019-Jun-11, excerpt):
> > I usually want to keep the (off) modules when compressing history,
> > so the current behavior should be kept.
>
> My implementation is default-off, and will only differ from how it's
> currently done when explicitly enabled.
>
> > As already said somewhere here, some (off) modules can't be removed
> > from history, or it will change the edit.
>
> My implementation already takes care about all these exceptional
> modules I'm aware of: parafin has pointed out "highlight
> reconstruction" and "white balance".  There's also "orientation" which
> internally seems to be done by the same module as "highlight
> reconstruction".
>

My point is that you should use the setting instead of hard-codding the
modules.


> Would anyone feel inclined to test this?
>
> https://github.com/s5k6/dt-test/tree/compressHistory
>
>
> --
> http://stefan-klinger.deo/X
> I prefer receiving plain text messages, not exceeding 32kB. /\/
>   \
> ___
> darktable developer mailing list
> to unsubscribe send a mail to
> darktable-dev+unsubscr...@lists.darktable.org
>
>

___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org

Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread dt-list
Patrick Shanahan (2019-Jun-11, excerpt):
> important is to not *break* already processed images by a change.

Yes, I'm aiming at that.  AFAIK, my current implementation would not
break anything, see below.  (The implementation reachable from
lighttable, which is already in master and was not built by me, can
break images, do so in bulk mode, not undoable, and without showing
the effect in lighttable).

Edgardo Hoszowski (2019-Jun-11, excerpt):
> I usually want to keep the (off) modules when compressing history,
> so the current behavior should be kept.

My implementation is default-off, and will only differ from how it's
currently done when explicitly enabled.

> As already said somewhere here, some (off) modules can't be removed
> from history, or it will change the edit.

My implementation already takes care about all these exceptional
modules I'm aware of: parafin has pointed out "highlight
reconstruction" and "white balance".  There's also "orientation" which
internally seems to be done by the same module as "highlight
reconstruction".

Would anyone feel inclined to test this?

https://github.com/s5k6/dt-test/tree/compressHistory


--
http://stefan-klinger.deo/X
I prefer receiving plain text messages, not exceeding 32kB. /\/
  \
___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org



Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread dt-list
thokster (2019-Jun-11, excerpt):
> Hi,
>
> Am 11.06.19 um 15:32 schrieb dt-l...@stefan-klinger.de:
>
> > This option is not about saving disk space, but rather about cleaning
> > up.
>
> Is the "compress history" button in lighttable view doing anything else?

Woha.  I have been talking about "compress history" in darkroom, not
lighttable.

They seem not to use the same code internally.

I have to admit that I have not thought about the "compress history"
button in lighttable.  That one already seems to remove switched-off
modules in current master [1].  And it seems to do this incorrectly
wrt. parafin's email:

* In darkroom, disable "white balance" and/or "highlight
  reconstruction".

* Go to lighttable, select that image, and apply "history stack →
  compress history"

* When you open the image again, "white balance" and "highlight
  reconstruction" will be enabled again.

So if compressing should not change the image, then my current
implementation [2] is even more correct, although it only is
applicable from darkroom, not lighttable.

Cheers
Stefan


[1] 2.7.0+1443~g9bfbb225e
[2] 
https://github.com/darktable-org/darktable/compare/master...s5k6:compressHistory


--
http://stefan-klinger.deo/X
I prefer receiving plain text messages, not exceeding 32kB. /\/
  \
___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org



Re: [darktable-dev] implementation question: remove all unused modules

2019-06-12 Thread jys
On Tue, Jun 11, 2019, at 15:34, Patrick Shanahan wrote:

> I doubt I would ever use "compress history" nor remove dup mod's.

Ok, more specifically, is there anyone who *is* interested in this feature, who 
also would prefer that it be implemented as an operation distinct from the 
current "compress history" operation? If not, I retract my suggestion that it 
should be a separate operation.

-- 
jys
___
darktable developer mailing list
to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org