Upgrade oidn (OpenImageDenoise) to the latest version (and include arm64 support)

[WilliamTambellini]

Describe the project you are working on

Some games.

Describe the problem or limitation you are having in your project

Godot (master and also 3.x) embeds atm oidn 1.1, the latest release of oidn is 1.3 with oneDNN 1.6.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

For godot, the main advantage of upgrading OIDN (OpenImageDenoise) :
https://github.com/godotengine/godot/tree/master/thirdparty/oidn
is to get better perf and get access to the recent oneDNN sublibrary:
https://github.com/OpenImageDenoise/mkl-dnn/tree/eb3e9670053192258d5a66f61486e3cfe25618b3

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

No user facing change, but from the oidn logs, hopefully some perf improvements :
https://github.com/OpenImageDenoise/oidn/releases

If this enhancement will not be used often, can it be worked around with a few lines of script?

Hard to upgrade a core lib with "few lines of scripts".

Is there a reason why this should be core and not an add-on in the asset library?

oidn is already in core. RealTime DNN inference should also be core.

[JFonS]

At the moment we are stuck with the current version of OIDN, mostly because newer versions added ISPC as a hard requirement for building it. Scons has no built-in support for ISPC, so we need to add it ourselves. And even then, we don't want to enforce having ISPC as a requirement to build the engine, so it should all be optional.

I did spend some time looking into using ISPC in Scons, mainly to have support for ARM64 platforms, but it wasn't trivial. The benefits weren't big enough to justify the time I would need to spend getting it to work, so it's on my backlog for now.

Also, I'd like to clarify a couple of things. OIDN is not really in "core", in the sense that it's a module that only gets compiled in editor builds that target x86_64 platforms. It's just an optional feature as a "nice to have" for the platforms that we can easily build OIDN for. If the goal is to have generalized support for oneDNN in Godot, upgrading OIDN won't really make a difference. I would open a separate proposal for that.

So in general, upgrading OIDN is a good idea, but it needs some research and work on detecting ISPC and using it when available, while not making it a requirement to build the engine. I won't be looking into upgrading in the near future, so if anyone is interested in the issue and wants to give it a go, they are more than welcome :)

[WilliamTambellini]

Hi @JFonS
Good to know. I dont remember issues with ISPC when using cmake, but did tn tried with scons.
I m indeed more interested by oneDNN than OIDN, and was trying to do 2 in a row, but as OIDN is not embedded in any runtime engine build (meaning the game exports), then OIDN is not that interesting to me.
That being said if I could give a hand, just ping me.
Cheers
W.

Ref:
https://ispc.github.io/

[akien-mga]

See #48082 for relevant discussion on arm64 support and how it could be achieved with sse2neon/SIMDe if we can't upgrade OIDN. But sooner or later we'll want access to improvements and bugfixes in OIDN so figuring out ISPC usage is likely the way to go, even if I don't like adding build time dependencies.

[petterreinholdtsen]

I suspect this issue also affect arm64, ppc64el and s390x, blocking the latest version from becoming part of Debian stable, see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1031132.

[akien-mga]

You should disable the denoise module on non-x86_64 for Godot 3, as I do for Mageia and Fedora packages:
https://src.fedoraproject.org/rpms/godot3/blob/rawhide/f/godot3.spec#_235

For Godot 4, the arch handling has been improved so it's properly disabled by scons automatically.

[petterreinholdtsen]

[Rémi Verschelde]

You should disable the denoise module on non-x86_64 for Godot 3: https://src.fedoraproject.org/rpms/godot3/blob/rawhide/f/godot3.spec#_235

Thank you. This worked great, and the patch is already commited to the Debian package git repository.

For Godot 4, the arch handling has been improved so it's properly disabled by scons automatically.

I look forward to seing that result. It find it better to test for working features instead of assuming architecture features never change, and hope the new arch handling will automatically detect what platforms support. :)

…

-- Happy hacking Petter Reinholdtsen

[akien-mga]

Superseded by godotengine/godot-proposals#7640.