Skip to content

macOS: Add required onnxruntime dependency.#20504

Open
TurboGit wants to merge 1 commit intomasterfrom
po/mac-onnx
Open

macOS: Add required onnxruntime dependency.#20504
TurboGit wants to merge 1 commit intomasterfrom
po/mac-onnx

Conversation

@TurboGit
Copy link
Member

No description provided.

@TurboGit TurboGit added this to the 5.6 milestone Mar 13, 2026
@TurboGit TurboGit added the scope: macos support macos related issues and PR label Mar 13, 2026
@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

please add it to packaging/macosx/1_install_hb_dependencies.sh as well

@TurboGit TurboGit changed the title Brewfile: Add requierd onnxruntime dependency. macOS: Add requierd onnxruntime dependency. Mar 13, 2026
@TurboGit
Copy link
Member Author

Just done. Thanks!

@kmilos
Copy link
Contributor

kmilos commented Mar 13, 2026

The same is needed for Windows? MSYS2 has it, and DLL should be included automatically during packaging...

Scratch that, it's not the runtime.

@andriiryzhkov
Copy link
Contributor

@TurboGit: brew onnxruntime package does not come with CoreML acceleration included. That limits ONNX to running only on CPU on macOS. That would not be a problem for object masking feature, because it does not use acceleration (translation computational graphs to CoreMl costs more time than CPU inference itself), but it might matter for longer tasks like AI denoise.

That was my deliberate decision at some point to get rid of onnxruntime brew package and use binary from GitHub, which comes with CoreML pre-built.

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

@andriiryzhkov :

That was my deliberate decision at some point to get rid of onnxruntime brew package and use binary from GitHub, which comes with CoreML pre-built.

How is this supposed to work when darktable is running from the bundle darktable.app?

@andriiryzhkov
Copy link
Contributor

@zisoft :
cmake downloads package from GitHub. But I did not check yet bundle packaging. Looking at it now.

@kmilos
Copy link
Contributor

kmilos commented Mar 13, 2026

Sorry for the late feedback here.

IMHO USE_AI should be OFF by default, or at least it shouldn't willy-nilly go downloading stuff without user's explicit request. There's quite a bit of cross-platform packaging stuff to address here.

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

The bundling script is packaging/macosx/3_make_hb_darktable_package.sh.
It is all done by simple script commands, no bundling software involved.

@kmilos
Copy link
Contributor

kmilos commented Mar 13, 2026

USE_AI should be OFF by default

By this I mean please take it out from .ci/ci-script.sh and put it in ECO where appropriate so it's clearly done with purpose and introduced gradually. For example, we might not want it in all the jobs in the matrix.

option(ONNXRUNTIME_OFFLINE "Disable automatic download of ONNX Runtime" OFF)

Maybe it's just me, but I really don't like the idea of someone else deciding what to download (and bundle) behind my back OOTB, so I'd prefer this to be a user explicit action.

@andriiryzhkov
Copy link
Contributor

It's only package installation during build process. It is either downloaded from brew server or from GitHub. Nothing more.

Event with USE_AI=ON all AI features will be OFF by default in global parameters. To start using AI features "user explicit action" is required to enable AI in parameters and press at least "download default".

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

In packaging/macosx/3_make_hb_darktable_package.sh:

./build/_deps/onnxruntime.<version>.dylib needs to be manually copied to the bundle directory.
Somewhere before line 305

@TurboGit
Copy link
Member Author

IMHO USE_AI should be OFF by default

We want the code to at least be compiled during CI builds.

@andriiryzhkov
Copy link
Contributor

@zisoft :

./build/_deps/onnxruntime..dylib needs to be manually copied to the bundle directory.
Somewhere before line 305

working on this

@TurboGit
Copy link
Member Author

@TurboGit: brew onnxruntime package does not come with CoreML acceleration included. That limits ONNX to running only on CPU on macOS. That would not be a problem for object masking feature, because it does not use acceleration (translation computational graphs to CoreMl costs more time than CPU inference itself), but it might matter for longer tasks like AI denoise.

So we will revisit/fix that at some point but the immediate need is to have a fully working binaries.

@kmilos
Copy link
Contributor

kmilos commented Mar 13, 2026

We want the code to at least be compiled during CI builds.

Yes, but not even all of the CI builds. Please move to ECO as suggested.

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

@TurboGit :

but the immediate need is to have a fully working binaries.

So this PR should fix this until we have a decision

@andriiryzhkov
Copy link
Contributor

I am already testing packaging solution. Will be ready today

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

@andriiryzhkov :

brew onnxruntime package does not come with CoreML acceleration included.

Is there a special reason for that?
Maybe a PR at the homebrew package may be the better solution.
I have done that in the past for several other packages (i.e. GMIC)

@andriiryzhkov
Copy link
Contributor

Is there a special reason for that?

The Homebrew onnxruntime formula doesn't include CoreML support – there's an open issue requesting it ([homebrew-core#177367](Homebrew/homebrew-core#177367)) that was closed as "not planned." The flag --use_coreml has to be explicitly passed at build time, and the Homebrew maintainers decided the additional Xcode SDK dependency wasn't worth taking on for a niche feature.

@zisoft
Copy link
Collaborator

zisoft commented Mar 13, 2026

another way is to rebuild the package from source during the homebrew package install.
I have implemented this with graphicsmagick to make it bundle-able:

# rebuild graphicsmagick for macOS bundling
export HOMEBREW_NO_INSTALL_FROM_API=1
gm_formula=`brew edit --print-path graphicsmagick`
cat $gm_formula | sed 's/--with-modules/--disable-installed/1' > gm_formula.tmp
mv gm_formula.tmp $gm_formula
brew reinstall --build-from-source --force graphicsmagick

And this is done for the nightly builds only, not for CI.

@andriiryzhkov
Copy link
Contributor

#20507 fixes ONNX Runtime packaging on macOS

@TurboGit TurboGit changed the title macOS: Add requierd onnxruntime dependency. macOS: Add required onnxruntime dependency. Mar 13, 2026
@TurboGit
Copy link
Member Author

@andriiryzhkov : So IIUC I can close this PR in favor of #20507?

@zisoft
Copy link
Collaborator

zisoft commented Mar 15, 2026

@TurboGit : I think this can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: macos support macos related issues and PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants