search

darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.73k stars 1.14k forks source link

Combining mask in Exclusive mode not working properly #8086

Closed RawConvert closed 3 years ago

RawConvert commented 3 years ago

dt 3.4 release Ubuntu 18.04

Suppose I have a parametric and drawn mask and the combine masks value is at its default of "exclusive". If I change to something else then back to exclusive, the mask has gone, i.e. the yellow has disappeared.

To reproduce -

  1. Load raw Screenshot from 2021-02-03 14-29-25

  2. Create new Exposure instance, do "para+drawn", adjust g sliders and get this - Screenshot from 2021-02-03 14-23-58

  3. Draw an area on the left side to get this - Screenshot from 2021-02-03 14-24-22 Note combine masks is set at exclusive.

  4. Set this to inclusive and inverted - Screenshot from 2021-02-03 14-24-40

  5. Set back to exclusive, get this - Screenshot from 2021-02-03 14-24-48 Clearly this should be the same as at step 3.

todd-prior commented 3 years ago

I commented on this type of behaviour back in Dec. The issue was actually brought up by @aurelienpierre . It does not seem to have generated any activity... https://github.com/darktable-org/darktable/issues/7372#issue-771604230

TurboGit commented 3 years ago

Works for me on master. And just to avoid confusion I'm pretty sure this is not a mask issue (the mask is properly set, check with some visible iop setting) but a pipe refresh issue.

todd-prior commented 3 years ago

@TurboGit As you can see in the screencast if you toggle away from exclusive you lose your mask. You have to reset the module. I would think you should be able to toggle back and forth...disabled opencl just to check same thing....

https://user-images.githubusercontent.com/44409419/107077963-9ae24f00-67bb-11eb-9ebd-c9de771111ee.mp4

TurboGit commented 3 years ago

Sorry, I still don't see that the mask is not working (as the title says). The mask (yellow) is not displayed but my point is that I think it is still there. Go away from the module, come back, just display the mask after the last step in your video and I'm pretty sure it will be there in yellow. Again, I think it is a refresh issue, can you check that?

todd-prior commented 3 years ago

I will and perhaps we should say the mask display not the mask. However another weird thing can happen in that the drawn portion will only show if you mouse over it and it ignores the toggle to display it so selecting that several times does not refresh it but if you mouse over it the circle becomes visible...in any care for sure you used to be able to move between the mask modes and not lose the display of the the mask ie the yellow overlay.....

todd-prior commented 3 years ago

After doing the same thing in the video I tried to add modules and come back..still the gray screen when the mask overlay is enabled and the module does not seem to be affecting the image so +3ev -3EV in exposure no effect with the overlay on or off....I will try again to confirm. I have even closed DT and opened the file and the situation holds...only resetting the module cleared it...Using todays build but I have seen this for some time..3.5+947 and tested in multiple modules same behaviour

RawConvert commented 3 years ago

@TurboGit , re. "Go away from the module, come back, just display the mask after the last step in your video and I'm pretty sure it will be there in yellow" I'm not finding that (on 3.5.0 947). I go and switch another module on and off, come back to the exposure mask, toggle the mask ("yellow") button, but there's no yellow. Then alter the exposure, but no change. (The trace of the drawn part is still visible though)

TurboGit commented 3 years ago

Ok, so I was wrong and this is very worrisome ! But still I cannot reproduce, so we need to find a 100% reproducible scenario to be able to fix. Can you reproduce this all the time ? Which modules are used ? Is that we clean edits ? All information you can find / report will be helpful. Thanks.

RawConvert commented 3 years ago

This is easy for me to reproduce. In fact there's no need for a drawn mask as well, so here's a simpler recipe. This seems to work reliably on various raw files.

  1. Take any raw file into darktable.
  2. Turn off all modules except "mandatory" (eg demosaic).
  3. New exposure instance, click "parametric"
  4. Take the "g" slider, move the left most pair of markers so there's a good amount of yellow, so click mask button also).
  5. Change "combine masks" from exclusive to "inclusive and inverted" then back to exclusive. (I think any change causes the problem)
  6. See the yellow mask has gone. Click the mask display button to go from grey image to normal.
  7. Change exposure and see no effect.
todd-prior commented 3 years ago

@TurboGit @RawConvert Exactly what I see. I tried monochrome , local contrast exposure...maybe a couple of others and selecting anything after exclusive will remove the yellow overlay and the module now will have no impact on the image....move any slider and no effect is noted....and I turned off opencl just incase any thing weird there still the same result...

TurboGit commented 3 years ago 
5\. Change "combine masks" from exclusive to "inclusive and inverted" then back to exclusive. (I think any change causes the problem)

No, for me this works and even tested many switches between both modes!

But I found that I can trigger this issue with:

In fact, as soon as one use "inclusive" or "inclusive inverted" the mask is not working anymore.

RawConvert commented 3 years ago

@TurboGit , well good, that's progress. But I don't understand your last post! We must be at crossed purposes! You start by saying "No" i.e. you're saying changing to incl. inverted and back works properly. Then you go on to say any use of incl. causes the problem. ??? !!!

TurboGit commented 3 years ago

Yes sorry read too fast :) You indeed said "inclusive"... Anyway PR will be there in moment to be tested.

TurboGit commented 3 years ago

Can you check #8110 ? TIA.

RawConvert commented 3 years ago

I'll have a look later today. Presumably you've checked it's basically working now?

TurboGit commented 3 years ago

Yes, working on my side. But this is a delicate area so I'd like to see if I didn't break something else. My first testing are promising though.

RawConvert commented 3 years ago

How do I clone your git space pls? I tried git clone https://github.com/TurboGit/fix-combine-channels ~/DT-turbogit-6Feb/darktable-code but it then asked for a username for github.com so I thought this must be wrong. And then do I do git checkout fix-combine-channels ? Ta

RawConvert commented 3 years ago

Ok, worked it out. Got build 1.1+21534~gf7a5b89c9-dirty. Testing to come.

RawConvert commented 3 years ago

Ah, window out of date so only just seen " added a commit..." Hopefully doesn't matter...

RawConvert commented 3 years ago

Tested ok. The bug has gone. I also re-ran a recent pic with several masks, though admittedly no complicated combining of the masks, and it produced the same jpeg output.

TurboGit commented 3 years ago

Great, thanks for testing! No the added commit is not a problem it was just a left over of a unused variable.

I'll wait for comment from @hlecleme who had introduced this part of code to be sure I'm not overlooking something.

todd-prior commented 3 years ago

Nice detective work guys

TurboGit commented 3 years ago

@RawConvert : I have a second version which is an optimization of my previous fix. Would be nice if you could check it too. TIA.

RawConvert commented 3 years ago

What is the command to update the code on my machine pls? I've tried various git pull things but they're not right.

RawConvert commented 3 years ago

cd ~/DT-turbogit-6Feb/darktable-code git pull remote: Enumerating objects: 104, done. remote: Counting objects: 100% (104/104), done. remote: Compressing objects: 100% (14/14), done. remote: Total 118 (delta 94), reused 97 (delta 90), pack-reused 14 Receiving objects: 100% (118/118), 66.61 KiB | 520.00 KiB/s, done. Resolving deltas: 100% (94/94), completed with 49 local objects. From https://github.com/TurboGit/darktable

git pull origin po/fix-combine-channels error: Pulling is not possible because you have unmerged files. hint: Fix them up in the work tree, and then use 'git add/rm ' hint: as appropriate to mark resolution and make a commit. fatal: Exiting because of an unresolved conflict.

RawConvert commented 3 years ago

@TurboGit or anyone, pls could you provide a Pull command so I can refresh what I previously downloaded, as above.

TurboGit commented 3 years ago

First go to current master and reset:

$ git checkout master
$ git reset --hard origin/master

Then create a tmp branch and apply the modification on my branch:

$ git checkout -b tmp
$ git pull https://github.com/TurboGit/darktable.git po/fix-combine-channels

And build/install from this tmp branch. We done you can just delete this branch.

$ git checkout master
$ git branch -D tmp
RawConvert commented 3 years ago

@TurboGit , I think there may be a problem with this Combine Masks setting. Looking at the manual, 3.5.3.4-combining drawn & parametric masks, I take "inclusive" to be an "OR" operation. That is, if I have a para mask and a drawn one, using inclusive will give a mask that comprises both para and drawn. However I'm finding this doesn't work. This may or may not be related to this issue 8086. To demonstrate: take any image using your build, add an exposure instance, choose para+draw, create a para mask (I did hue, hz). Now draw something that's not currently yellow. All yellow disappears; ok. Change "exclusive" to "inclusive". Now the whole image is yellow. I would expect the yellow now to be the para yellow plus the drawn area. @todd-prior , any thoughts?!

RawConvert commented 3 years ago

Thanks for the git commands, I will have a go.

RawConvert commented 3 years ago

Afraid I've fallen at the first fence!

paulssd2@shovel2:~/DT-turbogit-6Feb/darktable-code$ git checkout master src/develop/blend_gui.c: needs merge error: you need to resolve your current index first paulssd2@shovel2:~/DT-turbogit-6Feb/darktable-code$

I think it's better if you do the sorting out of the code sets! If this problem is at my end, let me know and I'll start again with a Clone...

todd-prior commented 3 years ago

@RawConvert

I'm not sure its an or operation but i did do this some time ago to try and wrap my head around all the possible combinations... Combined Mask Breakdown.pdf

TurboGit commented 3 years ago

Afraid I've fallen at the first fence!

Ok, start with:

$ git reset --hard

TurboGit commented 3 years ago

I think there may be a problem with this Combine Masks setting. Looking at the manual, 3.5.3.4-combining drawn & parametric masks, I take "inclusive" to be an "OR" operation.

Let's separate issues. My commit here is to unbreak the setting and should behave as for 3.4.

TurboGit commented 3 years ago

I have finally merged my patch. The current situation is so broken that the patch cannot be worst :) So just build from current master.

todd-prior commented 3 years ago

Looks to be working…built from master….nice job…in the end what was it….

image

From: Pascal Obry notifications@github.com Sent: February 9, 2021 9:23 AM To: darktable-org/darktable darktable@noreply.github.com Cc: Prior,Todd priort@mcmaster.ca; Comment comment@noreply.github.com Subject: Re: [darktable-org/darktable] Combining mask in Exclusive mode not working properly (#8086)

First go to current master and reset:

$ git checkout master

$ git reset --hard origin/master

Then create a tmp branch and apply the modification on my branch:

$ git checkout -b tmp

$ git pull https://github.com/TurboGit/darktable.git po/fix-combine-channels

And build/install from this tmp branch. We done you can just delete this branch.

$ git checkout master

$ git branch -D tmp

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/darktable-org/darktable/issues/8086#issuecomment-775976112, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKS2ESYAPF7V6VNZDHBW4PTS6FANZANCNFSM4XA66KAA.

TurboGit commented 3 years ago

@todd-prior : A bit-field storing the activated channels was not properly cleared when the output channels were hidden (now the default). If the output channels are visible then the bug was not present BTW. This is what put me on the right track.

todd-prior commented 3 years ago

Well done…..

From: Pascal Obry notifications@github.com Sent: February 9, 2021 5:06 PM To: darktable-org/darktable darktable@noreply.github.com Cc: Prior,Todd priort@mcmaster.ca; Mention mention@noreply.github.com Subject: Re: [darktable-org/darktable] Combining mask in Exclusive mode not working properly (#8086)

@todd-priorhttps://github.com/todd-prior : A bit-field storing the activated channels was not properly cleared when the output channels were hidden (now the default). If the output channels are visible then the bug was not present BTW. This is what put me on the right track.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/darktable-org/darktable/issues/8086#issuecomment-776275641, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKS2ES7L4AUZNIA7RXG2UTDS6GWTVANCNFSM4XA66KAA.