Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stable gimp-darktable calling interface #16193

Merged
merged 1 commit into from Feb 2, 2024

Conversation

jenshannoschwalm
Copy link
Collaborator

@jenshannoschwalm jenshannoschwalm commented Jan 27, 2024

As we want to support darktable as a gimp plugin in the best possible way we put the burden of "how to do so" on the dt dev team instead of having that on the gimp side as we likely know better to achieve this. Please note, this pr does not require a working lua system so not depending on what a distribution offers. See https://gitlab.gnome.org/GNOME/gimp/-/issues/10572

EDIT for latest status


Summary of currently implemented darktable features

  • always use default resources
  • don't print any logs
  • while being in darkroom ignore any requests to change view
  • use memory database and never write sidecar files

The GIMP support cli API 

Added --gimp <mode> option to the cli interface. <mode> is always a string, can be
"version" "file" "thumb"

Whenever we call 'darktable --gimp' the result is a string <res> written to stdout.
Also the returncode is 0.

In case of errors <res> is "error" and returncode will be 1

As darktable makes use of external libraries <ret> might be preceeded by other text,
the <res> is always the last written line string (eol before and after)

version            Returns the current API version (for initial commit will be 1)
file <path>        Starts darktable in darkroom mode using image defined by <path>.
                   When closing the darkroom window the file is exported as an
                   xcf file to a temporary location.
                   The full path of the exported file is returned as <res>
thumb <path> <dim> Write a thumbnail jpg file to a temporary location.
                   <path> is the image file
                   <dim> (in pixels) is used for the greater of width/height and ratio is kept.
                   The returned <res> has the following format:
                   * The full path of the exported file on the first line
                   * The width and height as space-separated integers on the second line.
                     These dimensions are informational and may not be accurate.
                     For raw files this is sensor size containing valid data.

@jenshannoschwalm jenshannoschwalm added feature: new new features to add scope: software support wiring external libs and software: LittleCMS, colord, G'MIC, enfuse/enblend, etc. labels Jan 27, 2024
@jenshannoschwalm jenshannoschwalm added this to the 4.8 milestone Jan 27, 2024
@parafin
Copy link
Member

parafin commented Jan 27, 2024

Does current dt (before this PR) return non-zero exit code when called with --gimp test option?

@jenshannoschwalm
Copy link
Collaborator Author

Does current dt (before this PR) return non-zero exit code when called with --gimp test option?

Yes.

@TurboGit
Copy link
Member

TurboGit commented Jan 29, 2024

  1. Crash 1

$ /opt/darktable/bin/darktable --gimp file ~/Images/Darktable/2cat/IMG_5031.CR2

When I run this command and exit from darkroom all is ok. If I return to lighttable and then quit darktable I get a crash.

  1. Missing check

$ /opt/darktable/bin/darktable --gimp file /tmp

There is not check that file's argument is really an existing file. The above command should be rejected.

@TurboGit
Copy link
Member

This works:

$ /opt/darktable/bin/darktable --gimp thumb 250 ~/Images/Darktable/2cat/IMG_5031.CR2

But I had to look at the code to find the syntax :) Just noting here for the documentation.

darktable --gimp thumb [size] [path-name]

src/common/darktable.c Outdated Show resolved Hide resolved
@TurboGit
Copy link
Member

Alternatively we may want to disable completely switching views when in gimp mode. This would avoid the crash mentioned above and potentially other issues (using map, print view...).

If this looks a good option, we need to make dt_view_manager_switch_by_view a no-op when in gimp mode.

@jenshannoschwalm
Copy link
Collaborator Author

Thanks for all the input.

@jenshannoschwalm jenshannoschwalm force-pushed the gimp_interface branch 2 times, most recently from a589378 to 63bebed Compare January 30, 2024 18:49
@jenshannoschwalm
Copy link
Collaborator Author

  1. Fixed darkroom exporting mode
  2. Disable viewport changes while in --gimp file mode
  3. For thumbs the argument order has been changed for consistency to <path> <dim>
  4. updated API docs

Currently thumb export does not work, not fully sure what gimp folks really want here.

@jenshannoschwalm
Copy link
Collaborator Author

Fixed problems related to making view-switch disabled while in gimping mode.

@Jehan would you have a look at current API proposal.

@jenshannoschwalm
Copy link
Collaborator Author

@TurboGit would appreciate another round of review :-)

Seems good to me except we would want to completely disable logs after merging if in gimp mode.

@Jehan
Copy link

Jehan commented Feb 1, 2024

@Jehan would you have a look at current API proposal.

Sure! I'm only looking at the MR description, not the code itself. Hope it's fine. :-)

version Returns the current API version

For this first implementation, it will be 0? 1? Something else?

Also I'm wondering if test and version commands cannot just be merged into one command (just version). I mean right now, trying darktable --gimp version is enough (either it fails because on current and older darktable, it will return 1, or it succeeds and gives us the dt-gimp protocol version; so we get 2 information at once anyway so why bother calling darktable twice, with each flag?). Unless I'm missing something…

On our side, how I see we'll use the algorithm is:

  1. testing with darktable --gimp version;
  2. if it fails, we'll test the legacy way (with lua scripting, etc.) and try to call darktable this way;
  3. if it succeeds, depending on the returned version, we would call the right command depending on version (darktable --gimp file <file> in the current API version).

This way, we'll be able to support as many versions of darktable as possible.

thumb <path> <dim> Write a thumbnail jpg file to a temporary location.
                   <path> is the image file
                   <dim> (in pixels) is used for the greater of width/height and ratio is kept.
                   The full path of the exported file is returned as <res>

It's good and could be enough. Now if possible, there could be some more information returned:

  • Our thumbnail procedure can optionally return the width/height of the image (the original one, not the thumbnail's), which serves as information to people before loading the actual image. The current implementation of the GIMP-darktable plug-in was returning this with dt.database[1].width and dt.database[1].height in lua scripting and the following comment:
          /* the size reported by raw files isn't precise,
           * but it should be close enough to get an idea.
           */
  • The image type, such as RGB, RGBA, Grayscale, Grayscale with alpha, etc. The current plug-in was not returning this info, but if you can, it's also something thumbnail procedures are able to display to people in file dialogs.
  • The number of "layers" in the image: I'm not sure if there is any raw image format where this concept makes sense (are they raw images which can contain several "layers" or images? Maybe there are cameras saving formats containing multiple-exposure photos for HDR imaging? I don't really follow much what happens in photography these days! 😅).

Anyway these are the 3 additional (optional) data which our thumbnail procedures can make use of and expose to people in file dialogs. So if you want to stop with current API, it's ok. But if <res> could also return these infos, it would be even better. Maybe:

thumb <path> <dim> Write a thumbnail jpg file to a temporary location.
                   <path> is the image file
                   <dim> (in pixels) is used for the greater of width/height and ratio is kept.
                   The returned <res> has the following format:
                   * The full path of the exported file on the first line
                   * The width and height as space-separated integers on the second line: these dimensions are informational and may not be accurate; also if the information cannot be extracted without developing the raw, 0 0 is returned
                   * RGB, RGBA, grayscale, grayscaleA on the third line
                   * Number of layers contained in the file

Or something like this. :-)

You may also choose to only return some of the info (like width/height).

Anyway globally it looks very good to me!

@TurboGit
Copy link
Member

TurboGit commented Feb 1, 2024

  • The number of "layers" in the image: I'm not sure if there is any raw image format where this concept makes sense (are they raw images which can contain several "layers" or images? Maybe there are cameras saving formats containing multiple-exposure photos for HDR imaging? I don't really follow much what happens in photography these days! 😅).

Not for RAW files, but darktable can write .xcf with layers for the module's masks.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested yet.

src/views/view.c Outdated Show resolved Hide resolved
src/common/gimp.c Outdated Show resolved Hide resolved
@jenshannoschwalm
Copy link
Collaborator Author

Also I'm wondering if test and version commands cannot just be merged into one command (just version).

Fully agreed and done.

  • Our thumbnail procedure can optionally return the width/height of the image (

Also done.

  • The image type, such as RGB, RGBA, Grayscale, Grayscale with alpha, etc. The current plug-in was not returning this info, but if you can, it's also something thumbnail procedures are able to display to people in file dialogs.

We can deliver that, not sure what 'exactly' we should check for.

@jenshannoschwalm jenshannoschwalm force-pushed the gimp_interface branch 2 times, most recently from 9f96c60 to 39b8c9d Compare February 1, 2024 18:59
@wpferguson
Copy link
Member

Can a connection be initiated from the darktable side? We currently use a lua script to export then launch gimp with the image and then import the result.

Will this work on windows?

@TurboGit
Copy link
Member

TurboGit commented Feb 1, 2024

We currently use a lua script to export then launch gimp with the image and then import the result.

Will this work on windows?

Yes, should work as not modifying this part. The support here is for Gimp to be able to use darktable to demosaic the RAW and create a .xcf (which is then imported into Gimp).

src/common/gimp.c Outdated Show resolved Hide resolved
As we want to support darktable as a gimp plugin in the best possible way we put the
burden of "how to do so" on the dt dev team instead of having that on the gimp side as
we likely know better to achieve this.

Summary of currently implemented darktable features
- always use default resources
- don't print any logs
- while being in darkroom ignore any requests to change view

**************************************************************************************
The GIMP support cli API 

Added --gimp <mode> option to the cli interface. <mode> is always a string, can be
"version" "file" "thumb"

Whenever we call 'darktable --gimp' the result is a string <res> written to stdout.
Also the returncode is 0.

In case of errors <res> is "error" and returncode will be 1

As darktable makes use of external libraries <ret> might be preceeded by other text,
the <res> is always the last written line string (eol before and after)

version            Returns the current API version (for initial commit will be 1)
file <path>        Starts darktable in darkroom mode using image defined by <path>.
                   When closing the darkroom window the file is exported as an
                   xcf file to a temporary location.
                   The full path of the exported file is returned as <res>
thumb <path> <dim> Write a thumbnail jpg file to a temporary location.
                   <path> is the image file
                   <dim> (in pixels) is used for the greater of width/height and ratio is kept.
                   The returned <res> has the following format:
                   * The full path of the exported file on the first line
                   * The width and height as space-separated integers on the second line.
                     These dimensions are informational and may not be accurate.
                     For raw files this is sensor size containing valid data.
Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit TurboGit merged commit a5a3680 into darktable-org:master Feb 2, 2024
5 checks passed
@TurboGit TurboGit added documentation-pending a documentation work is required release notes: pending labels Feb 2, 2024
@jenshannoschwalm jenshannoschwalm deleted the gimp_interface branch February 3, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-pending a documentation work is required feature: new new features to add release notes: pending scope: software support wiring external libs and software: LittleCMS, colord, G'MIC, enfuse/enblend, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants