Skip to content

Spektrafilm analog film simulation module - #21967

Merged
TurboGit merged 2 commits into
darktable-org:masterfrom
piratenpanda:spektrafilm
Sep 5, 2026
Merged

Spektrafilm analog film simulation module#21967
TurboGit merged 2 commits into
darktable-org:masterfrom
piratenpanda:spektrafilm

Conversation

@piratenpanda

@piratenpanda piratenpanda commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

after my git mistake, here's another PR. Sorry for the noise. Continuing from #21534

@piratenpanda

piratenpanda commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

@kofa73 this should address all the issues your bot found

@andriiryzhkov

andriiryzhkov commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Is each of your commits compilable?
We recently had issues with PR containing transient commits which were not compilable. Please see discussion in #21881.

@piratenpanda

piratenpanda commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Yes, this should be true as I manually test each one before in my PKGBUILD and do local testing

@piratenpanda
piratenpanda force-pushed the spektrafilm branch 2 times, most recently from e950b50 to d648a87 Compare August 23, 2026 15:42
@TurboGit

TurboGit commented Aug 23, 2026

Copy link
Copy Markdown
Member

This is a new module, I'll squash all commits together anyway.

@TurboGit TurboGit added this to the 5.8 milestone Aug 23, 2026
@TurboGit TurboGit added feature: new new features to add difficulty: hard big changes across different parts of the code base scope: image processing correcting pixels labels Aug 23, 2026
@piratenpanda

piratenpanda commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

will do a bit of UI changes later today, then I'll collect presets

@kofa73

kofa73 commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Note that I did not ask the bots the compare that the processing algo matches Spektrafilm (they were told to look for generic maths issues like NaN). Let me know if you need that reviewed.

@piratenpanda

piratenpanda commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, I already have a script for that myself

@Phemisters Phemisters added the documentation: pending a documentation work is required label Aug 25, 2026
@piratenpanda
piratenpanda force-pushed the spektrafilm branch 2 times, most recently from bfba93f to 74c3122 Compare August 26, 2026 15:09
@piratenpanda

piratenpanda commented Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

latest commit moves out of the way for #22042, will update once the linked PR is merged

@da-phil

da-phil commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

latest commit moves out of the way for #22042, will update once the linked PR is merged

Ah, funny incident that we did essentially the same thing for different reasons 😁
For me it's a prerequisite to start an OpenCL implementation for the tone equalizer ;)

@piratenpanda

piratenpanda commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

happy to give the 42 to you ;)

@da-phil

da-phil commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

I hope you'll not get angry about another opencl program appearing in another PR soon 😬
Let's just both share number 43 for the time being and wait whichever PR get's merged first, shouldn't be a big merge-conflict resolution effort anyway ;)

@piratenpanda

piratenpanda commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

I hope you'll not get angry about another opencl program appearing in another PR soon 😬

now that the 42 is gone it doesn't matter anymore :D

@da-phil

da-phil commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

I hope you'll not get angry about another opencl program appearing in another PR soon 😬

now that the 42 is gone it doesn't matter anymore :D

Ah, now I get it, sometimes it takes a while for me 😅

rafaelcgs10 added a commit to rafaelcgs10/spektrafilm-art-darktable that referenced this pull request Aug 29, 2026
@rafaelcgs10 @claude
Upgrade darktable spektrafilm to PR #21967 head d0b4e9c
10e9acf 
The upstream PR moved: #21534 was closed by accident and superseded by
darktable-org/darktable#21967 (same piratenpanda spektrafilm branch).
Two runtime changes ride along with the bump:

- The data pack repo moved to the darktable.org-managed
  darktable-org/darktable-spektrafilm (module default SF_DEFAULT_REPOSITORY
  changed with it). data-pack.nix now mirrors that repo; the pack content is
  byte-identical (lut_hash 565f4ec4, pack_format 2, pack 0.3.3).
- Packs now resolve under g_get_user_data_dir() (~/.local/share/darktable)
  instead of the config dir, so the runtime wrapper links the pack there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTw3vKZxSvgXYh2E4P6ZFE
@piratenpanda
piratenpanda force-pushed the spektrafilm branch 2 times, most recently from 43109ea to 6b6dcd6 Compare September 2, 2026 04:32
@piratenpanda

piratenpanda commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Ready from my side except from a final dtdocs update and the release notes entry

@piratenpanda

piratenpanda commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

I have no strong opinion on it so I'll do whatever you prefer @TurboGit

@TurboGit

TurboGit commented Sep 2, 2026

Copy link
Copy Markdown
Member

No strong opinion on my side either, just wanted to be sure this point was properly discussed. So if some users may use Darktable for this only, let's add a specific workflow for it. I would prefer in this PR if possible.

@TurboGit

TurboGit commented Sep 2, 2026

Copy link
Copy Markdown
Member

We also need to ensure that the new code is Gtk4 ready (see all the gtk4-prep merged PR and also the not yet merged popover conversion by @zisoft #19657).

@piratenpanda
piratenpanda force-pushed the spektrafilm branch 2 times, most recently from 562cd29 to e75453c Compare September 3, 2026 05:04
@piratenpanda

piratenpanda commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

all fine from my side, will build with 19657 tonight and test locally

TurboGit
TurboGit requested changes Sep 3, 2026
View reviewed changes

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please review all tooltip, we have:

Image

Note the missing dot at the end. For all paragraph we need a dot. A dot is omitted only for single line title (for example the tooltip for "halation strength" is ok as a single line title).

Also not sure the -- on the middle of the text is a correct English typography? If yes, keep it otherwise we could use dot here I suppose.

Those comments apply to many tooltips.

@TurboGit

TurboGit commented Sep 3, 2026

Copy link
Copy Markdown
Member

@piratenpanda : This time I have tested the following scenario:

  • Enable the spektrafilm workflow and reset the image's history -> fine as specktrafilm is enabled by default.
  • After developing a picture with spektrafilm, I quit Darktable and delete the downloaded spektrafilm data. When I restart the spektrafilm ask me for downloading, after doing that I do get the exact same output.

So all good, we are very close to merging.

@piratenpanda
piratenpanda force-pushed the spektrafilm branch 2 times, most recently from 34f62f7 to 84a3fec Compare September 4, 2026 04:30
@piratenpanda

piratenpanda commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

reworked tooltips and squashed commits

TurboGit
TurboGit reviewed Sep 4, 2026
View reviewed changes
Comment thread src/iop/spektrafilm.c Outdated
@piratenpanda

piratenpanda commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

fixed some more stale comments

@andriiryzhkov andriiryzhkov mentioned this pull request Sep 4, 2026
Open
@piratenpanda

piratenpanda commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Will rename function names according to #22150

@piratenpanda

piratenpanda commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

I moved the coefficients into common/gaussian. I'll leave the commit for review and then I'll squash. I think the weighted Gaussian bank can be made common as well, but I think this can also happen after this is merged and I would opt to not delay this further.

TurboGit
TurboGit approved these changes Sep 5, 2026
View reviewed changes

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We are good I'd say now. Let's move forward and merge this for more field testing before the release. Thanks for the hard work on this new module!

@TurboGit
TurboGit merged commit d81ccfe into darktable-org:master Sep 5, 2026
6 checks passed
@andriiryzhkov

andriiryzhkov commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

@piratenpanda : Great job! Thank you.

@dmt-hub

dmt-hub commented Sep 5, 2026

Copy link
Copy Markdown

@piratenpanda Thanks alot! 🙏

@andriiryzhkov

andriiryzhkov commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

I'd improve a bit release note for this module - explain that this module is actually a tone mapper, not just creative effects module. And maybe move it up in the list as it is the biggest feature as of now.

@piratenpanda

piratenpanda commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, typical understatement me :D See #22161

@jenshannoschwalm

jenshannoschwalm commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Tried this just now, absolutely great!

@Phemisters Phemisters added documentation: complete needed documentation is merged in dtdocs and removed documentation: pending a documentation work is required labels Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TurboGit TurboGit TurboGit approved these changes

Assignees

No one assigned

Labels

difficulty: hard big changes across different parts of the code base documentation: complete needed documentation is merged in dtdocs feature: new new features to add scope: image processing correcting pixels

Projects

None yet

Milestone

5.8

Development

Successfully merging this pull request may close these issues.

9 participants

@piratenpanda @andriiryzhkov @TurboGit @kofa73 @da-phil @anoderay @dmt-hub @jenshannoschwalm @Phemisters