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.81k stars 1.14k forks source link

RFH: Instrumenting to determine when an image analysis/processing action is complete #13745

Closed wpferguson closed 1 year ago

wpferguson commented 1 year ago

RFH = Request for Help

@dterrahe created darktable.gui.action() and darktable.gui.mimic() which make it possible to automate the GUI.

When I started scripting actions I realized that I sometimes needed to wait until one action was complete before starting another, so I added an event, pixelpipe-processing-complete (develop/develop.c:652) to fire when the pixelpipe finished. This works most of the time, but I still have to resort to adding darktable.control.sleep() because it seems that sometimes pixelpipe-processing-complete isn't when the GUI is ready for the next input.

If I create a style with multiple modules set but turned off, I can turn them on from a script using pixelpipe-processing-complete to let me know when and it works every time.

If I use rotate and perspective and click one of the analyze functions, then pixelpipe-processing-complete doesn't mean the GUI is necessarily ready for the next input, so I have to resort to adding a sleep.

Other things that seem to work this way are

There is also this thread, https://discuss.pixls.us/t/working-with-modules-from-lua-scripts-in-darktable/32934 that discusses some issues with determining when actions are complete.

So it seems that I need a way to detect when modules that "analyze" the image are finished and the GUI is ready for another input.

@jenshannoschwalm, @TurboGit any ideas or at least a starting place to start investigating?

jenshannoschwalm commented 1 year ago

I feel absolutely unsure about the gtk signals. Could it be, the modules / situations you describe fire some add-to-history leading to a reprocessing?

dterrahe commented 1 year ago

Some of the modules might themselves be waiting for the signal and then update sliders. So if lua receives the signal first (which might be if it install the handler at startup?), the sliders wouldn't have been updated yet. Gtk itself has g_signal_connect_after to make sure you receive a signal after everybody else has handled it, but our own signals don't support that.

wpferguson commented 1 year ago

I wonder if I could hook g_signal_connect_after to call a function that simply fires a lua event. Or I could find all of the g_signal_connect_after targets and add an event to them.

At least it's a place to start looking

wpferguson commented 1 year ago

Just tried an experiment with tone equalizer masking. I ran with -d lua -d perf -d opencl.

When I open an image and click the mask exposure compensation eye dropper I get a pixelpipe-processing-complete event. When I click the mask contrast compensation eye dropper, I don't get a pixelpipe-processing-complete event. However, @jenshannoschwalm [dev_process_preview] message does show up so maybe I need to move my event there.

I'll test with a couple more of the "problem" modules and see if it's just a badly placed event.

wpferguson commented 1 year ago

exposure spot measurement doesn't fire the event either.

I'll try moving to near the [dev_process_preview] message and see if that works better

dterrahe commented 1 year ago

wonder if I could hook g_signal_connect_after to call a function that simply fires a lua event.

I'm only now (sorry) noticing that in develop.c dt_dev_process_image_job you are throwing the lua pixelpipe-processing-complete signal just before DT_DEBUG_CONTROL_SIGNAL_RAISE(darktable.signals, DT_SIGNAL_DEVELOP_UI_PIPE_FINISHED); is raised. If you change those two around you might have more luck. But you might not get guarantees because all of this is supposedly asynchronic. However, since dt.gui.action ends up in the same main loop queue behind the earlier signal, my gut says you should be ok...

wpferguson commented 1 year ago

@dterrahe took your suggestion and tried it. Not sure it really made any difference.

I changed pixelpipe-processing-complete to image-pixelpipe-processing-complete and then added a preview-pixelpipe-processing-complete event to see what that gave me.

I noticed in ALMOST all instances both events fired though the order wasn't always the same.

I did find that exposure spot exposure mapping measure only fires the preview-pixelpipe-processing-complete event which makes sense because you are only marking the area to use and not applying anything.

There was some discussion on pixls.us about loading an image into darkroom and when changes could be applied. I noticed that preview-pixelpipe-processing-complete alsays followed image-pixelpipe-processing-complete when an image was loaded into darkroom view so it might be necessary to wait for the preview to finish calculating before starting.changes

dterrahe commented 1 year ago

This piece of code and commentary seems potentially relevant: https://github.com/darktable-org/darktable/blob/34752ba03252cc74e28d415209d43344a51ebfe9/src/gui/color_picker_proxy.c#L309-L333

wpferguson commented 1 year ago

Right now I'm catching both events (preview and image), except for the one case, trying to make sure everything is finished.

Now I have a question for you. In tone equalizer if I click on the eye dropper next to mask exposure compensation or mask contrast compensation they finish in the off state. If I do dt.gui.action("iop/toneequal/mask xxxxxx compensation", "button", "toggle", 1.0) they get left in the "on" state. Think I figured it out. Just took away the 1.0 so I had dt.gui.action("iop/toneequal/mask xxxxxx compensation", "button", "toggle"). The pixelpipe ran and the eye droppers ended up off.

dterrahe commented 1 year ago

both events (preview and image)

How about DT_SIGNAL_CONTROL_PICKERDATA_READY?

Just took away the 1.0 so I had dt.gui.action("iop/toneequal/mask xxxxxx compensation", "button", "toggle").

Are you trying to read or set the status of the button?

wpferguson commented 1 year ago

How about DT_SIGNAL_CONTROL_PICKERDATA_READY?

I'll try it

Are you trying to read or set the status of the button?

I'm trying to automatically set mask xxxx compensation. If I'm in the GUI, I click the eye dropper, it changes the values, and the eye dropper ends up in the "off" (grayed out) state. If I do it with dt.gui.action("iop/toneequal/mask xxxxxx compensation", "button", "toggle", 1.0) the picker gets left in the active state (it doesn't toggle).

wpferguson commented 1 year ago

Looks like adding an event to _color_picker_proxy_preview_pipe_callback works.

wpferguson commented 1 year ago

For some reason adding the event in _color_picker_proxy_preview_pipe_callback causes it to react a little slower and more in line with what I think I should see.

wpferguson commented 1 year ago

@dterrahe I think we've got it. I just processed a 10 image loop from lighttable, loading each image and adding 0.5ev exposure with no sleeps and no errors.

I've been testing all of the scenarios that gave me problems and I've got them all working without sleeps.

wpferguson commented 1 year ago

13758 fixes the issue that came up while we were working on the original issue. The original issue will be fixed by #13749.