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

Add an exporthaldclut lighttable module #247

Merged
merged 13 commits into from
Jul 6, 2020

Conversation

noahwc
Copy link
Contributor

@noahwc noahwc commented May 4, 2020

This script creates a lighttable module which exports all user styles in the database as haldcluts.

Usage

  • Select the identity haldCLUT.
  • Select a folder to output the generated haldCLUTs to
  • Press export
  • This will generate haldCLUTs for all user styles currently in the database.

Unfortunately due to limitations within the lua API I was unable to find a way to verify the a generated haldCLUTs is valid. Currently the script has only been tested on Linux (Ubuntu) but I would expect it to work on all OSs.

require "exportLut"

Given a haldCLUT identity file this script generates haldCLUTS from all the user's
styles and exports them to a location of their choosing.
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide a link to an explanation of a haldCLUT identity file? I didn't know what one was but I found https://rawpedia.rawtherapee.com/Film_Simulation, which provided an explanation and a way to create one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a direct link to the identity haldCLUT: https://rawpedia.rawtherapee.com/images/8/83/Hald_CLUT_Identity.tif

Ideally I feel like it would be better from a usability standpoint to include it with the script. I wasn't able to find a clearly defined way to include extra resources with a script, however, so I left it up to the user to provide their own identity haldCLUT.

contrib/exportLUT.lua Outdated Show resolved Hide resolved
}

local function export_luts()
identity = dt.database.import(file_chooser_button.value)
Copy link
Member

Choose a reason for hiding this comment

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

Should you create a separate film roll for this file? if you just do an import, then the file is imported into the active film_roll at time of invocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Film rolls have always been a little mysterious to me so it's entirely possible I'm missing something here but I don't seem to encounter this behaviour.

contrib/exportLUT.lua Outdated Show resolved Hide resolved
contrib/exportLUT.lua Outdated Show resolved Hide resolved
@wpferguson
Copy link
Member

This is a neat idea. I'm curious to see how it will work in practice, meaning that the applied LUT may not meet people's expectations depending on what the style contains. I'll test it with some of the film styles and see what I get.

@noahwc
Copy link
Contributor Author

noahwc commented May 5, 2020

Thanks for the feedback, looks like I have some digging to do into the lua API 😄. Hopefully in the next week or so I can push up some changes.

I think you're correct in stating that getting the plugin to be intuitive for users may be difficult. I used the script to create LUTs for the t3mujin film simulation pack and while the results were good I did run into some trouble with the pixel pipe order (here's a discussion on that). Beyond operations like cropping and adding frames will break the outputted LUT. Unfortunately I don't see a way around this (beyond documentation) since as far as I can tell the lua API lacks the ability to inspect the edits being performed by styles.

contrib/exportLUT.lua Outdated Show resolved Hide resolved
@wpferguson
Copy link
Member

I found where you got the api_major_version > 6. I guess I need to fix the moduleExample.lua file.

You might also consider enabling translation. You can use the gettextExample.lua file to see how it's done.

Tested it and it works fine.

@wpferguson
Copy link
Member

I had one more thought. I tested using the film emulation styles, which I have set up in a style hierarchy. When a style gets converted to a LUT, the name is Film|Color|Agfa|Agfaxxxxxxx.png. Would it be better to put these styles in a folder hierarchy in the LUT folder similar to the style hierarchy?

@noahwc
Copy link
Contributor Author

noahwc commented May 11, 2020

Both those (the translation and folder structure) seem like good ideas. I'll do my best to push out an update in a relatively timely manner (uni is kicking back in so unfortunately I have some time constraints now 😞)

@supertobi
Copy link
Contributor

@noahwc What is the status of this PR?

@noahwc
Copy link
Contributor Author

noahwc commented Jun 15, 2020

@supertobi hopefully I'll have everything resolved within the next week. Definitely still committed to getting this done/merged.

@noahwc
Copy link
Contributor Author

noahwc commented Jun 16, 2020

@wpferguson I've enabled translation (I think) and luts are now exported to a folder hierarchy.

@noahwc noahwc requested a review from wpferguson June 20, 2020 09:38
Copy link
Member

@wpferguson wpferguson left a comment

Choose a reason for hiding this comment

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

Tested and it works fine (once I used a good identity file :)). The changes with translation are the only things needed.

contrib/exportLUT.lua Show resolved Hide resolved
@noahwc noahwc requested a review from wpferguson July 5, 2020 06:21
Copy link
Member

@wpferguson wpferguson left a comment

Choose a reason for hiding this comment

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

Looks good to me

@wpferguson wpferguson merged commit 48b915b into darktable-org:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants