AcademySoftwareFoundation / openfx

OpenFX effects API
Other
393 stars 119 forks source link

Opencl support: use Conan to get OpenCL headers & lib #149

Closed garyo closed 3 months ago

garyo commented 3 months ago

This sets up OpenCL as a Conan dependency per #148, in all cases except Mac where it still uses the system OpenCL. Reviews appreciated before this gets merged. Should Mac use the conan OpenCL too? Are there any cases where it will cause trouble? Should we use the hpp headers everywhere as suggested in https://github.com/AcademySoftwareFoundation/openfx/issues/148#issuecomment-1997186341 ? And is OpenCL 2.0 a good baseline?

SonyDennisAdams commented 3 months ago

@garyo, sorry about my delay in review; I was on vacation last week.

Thank you for these changes, as they are likely an improvement over what we had. I know I had to manually get OpenCL dependencies prepared on my machines to get this to work. That said, I don't know Conan, so I'm not qualified to review the actual changes you made. Would it be helpful if I tried this as a clean branch on my systems to see if they can now build without manual setup?

To the question "is OpenCL 2.0 a good baseline?", I do know that a lot of people still target OpenCL 1.2 since it is more widespread than 2.0. Only since 3.0 came out is there some traction in getting those old platforms updated, but a few still exist. Our application's minimum requirement is OpenCL 1.1 (and we support one or two OpenCL 1.2 extensions if they are available). The OpenFX extensions for OpenCL do not use anything beyond OpenCL 1.1.

To the question about Conan for Mac; I'm afraid I also don't know the best solution there. We have always just used our own OpenCL headers, and on Mac we link against System/Library/Frameworks/OpenCL.framework

revisionfx commented 3 months ago

Apple only officially support 1.2.

Pierre

garyo commented 3 months ago

These changes should certainly not prohibit anyone from supporting older OpenCL especially if they link against system libs e.g. on Mac, but we want to ensure that our build process can be replicated by plugin developers and will give good results.

I can confirm that the built plugins on Mac do not have any unexpected dependencies, which is to say Conan didn't pull in anything extra that the plugin would need to find at runtime:

% otool -L build/Release/Support/Plugins/example-GPUGain-support.ofx
    /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2420.0.0)
    /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 2420.0.0)
    /System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 343.14.0)
    /System/Library/Frameworks/OpenCL.framework/Versions/A/OpenCL (compatibility version 1.0.0, current version 1.0.0)
    /System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
    /System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1345.100.2)
    /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1700.255.0)
    /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)

@janimatic since these are your suggestions, would you take a look as well? What should I add to ensure the built example plugins (specifically gpu gain) will still work with OpenCL 1.2?

@SonyDennisAdams yes, it would be great if you can try this branch on a clean machine. The build-cmake.sh script should do everything you need, as long as you have recent conan installed. Pass the "-C" argument to build with OpenCL support. (To install conan, do pip install conan assuming you have a decent python3.)

janimatic commented 3 months ago

Hello everyone, i am not an expert of opencl at all, but according to https://www.khronos.org/blog/opencl-3.0-specification-finalized-and-initial-khronos-open-source-opencl-sdk-released opencl1.2 and opencl2.0 apps will run on opencl 3.0 with no code changes. Furthermore, if any particular case arise that would requieres to force a specific opencl version, would it be for testing purpose, we can always use the preprocessor directives to use specific opencl version (else v3.0 is assumed).

#define CL_HPP_MINIMUM_OPENCL_VERSION 120
#define CL_HPP_TARGET_OPENCL_VERSION 120
#include <CL/opencl.hpp>

I understand that opencl3.0 spec was designed to be backward compatible from the start (in my very humble opinion...). Like I said, I've compilled the opencl branch from scratch without any problem, adding these lines to the gpu gain example, and other opencl plugins that I'm working on now, with those edits headers. I am testing in resolve/windows which, if i am not mistaken, installs an opencl3.0 dll in C:\Windows\System32. I am logging detailled opencl compiler status codes and CL_PROGRAM_BUILD_LOG, i didn't meet any issue on that side (except that i can only use opencl buffers and no opencl images! but maybe that's me...) I would trust the khronos page, but if you guys can test on other machines with older dll/so/dylibs, that would be even better ?

Off topic, Resolve seems to be more problematic with openfx api versions actually, as far as i understand - the problem is that there is not much info available on that topic. I just found replacing src->getPixelComponents() != OFX::ePixelComponentRGBA by mapPixelComponentEnumToStr( src->getPixelComponents()) != kOfxImageComponentRGBA would fix some resolve openfx sample code, but i might be wrong, sorry for the off topic

garyo commented 3 months ago

Thanks @janimatic . Sounds like we should be OK then without any additional changes, as long as the GPUGain example doesn't use anything beyond OpenCL 1.2 which I expect it doesn't.

As for your other query, the OpenFX C++ support lib has not always been kept up to date, so it's certainly possible there are bugs there. Please submit a separate issue if you like.

revisionfx commented 3 months ago

My understanding is:

nVidia supports v3 (Win and Lin) Apple supports v1.2 Intel supports v3 (win and lin) AMD Windows supports 2.2 I think but v3 is by design compatible with code compiled against v1.2 but not all v2 will work with nVidia cards. Essentially, the official Khronos release made the V2 additions from AMD optional in V3 - as extensions basically. And AMD linux support is real mysterious as somehow integrated with RocM ???? And varies per card. In my book there is now only OpenCL 1.2 and v3 if you check card vendor unless one wants to only work on AMD GPU and sometimes. Atomics is one thing different I remember worth it in V3.

There was an issue with AMD driver not registering OpenCL, we dlopen it ourselves on linux - had too many issues.

https://github.com/ptrumpis/OpenCL-AMD-GPU

Recall: https://www.extremetech.com/computing/309842-opencl-3-0-kicks-off-with-a-huge-step-backwards If someone forgot that v3 was (v2 - some stuff) - an API rewind.

https://github.khronos.org/OpenCL-CLHPP/

I see that Adobe now greys out OpenCL support if v2.1 is not supported by driver on Windows, but plugins compiled using v1.2 still work. No client has complained yet. The only issue we had way back when AMD started to ship V2 is a bunch of new enumerated types we did not know about in V1.2 showed up, and we had some else { in our code for last v1 known one and we crashed :) Looks like there is some llama version that comes with AMD now that forced them recently to correctly deal with OpenCL, am not clear (what google list in recent search about OpenCL and AMD). AMD like Intel also support OpenCL integrated GPU. Works except needed to put a lot of flushes in code at some point as we use OpenGL interop image/texture. I can ask someone at AMD who deals with OpenCL historically to comment on this thread if someone is interested.