gpu / JOCL

Java bindings for OpenCL
http://www.jocl.org
Other
183 stars 33 forks source link

Add cl_APPLE_gl_sharing bindings #36

Closed LlemonDuck closed 3 years ago

LlemonDuck commented 3 years ago

Closes #35.

I've gone ahead and added the binding myself, but haven't yet had the chance to test it on a native macOS machine. Compiled in a VM.

I'll let you know once I've had the chance to try it on a native machine.

LlemonDuck commented 3 years ago

Works on a native machine, got a context and was able to do some computing and rendering in a shared GL context 👍

Switching off of draft mode.

gpu commented 3 years ago

Wow, thanks! It's rare to receive a proper PR in such a case. I'll have another closer look, but from quickly skimming it, it looks like it could be merged soon after I tried it out.

gpu commented 3 years ago

I have tried it out, and it appears to work (in terms of compilation on Windows - of course, I cannot test the actual functionality so easily). I think it will not be necessary to put this single function into a dedicated class like CLApple. When the function is not available, it will throw in any case. (It would be a different thing if there were 20 Apple-specific functions, but for a single one, adding a dedicated class does not appear to make much sense).


One thing from a "high-level" perspective that I wondered about:

I noticed that (according to the header part that you posted in https://github.com/gpu/JOCL/issues/35#issuecomment-770077934 ), the function is declared as CL_EXT_SUFFIX__VERSION_1_0 CL_DEPRECATED(10.6, 10.14);. The tech note that you linked to says that it's from "Late 2013". I wondered whether there may be some newer functionality, to achieve the desired behavior - maybe via the context properties, similar to what is done in the samples at https://github.com/gpu/JOCLSamples/blob/master/src/main/java/org/jocl/samples/JOCLSimpleLWJGL.java#L466 ?

(That's not really an "issue" and not preventing the PR from being merged, because even if it's deprecated, it can still be there, with the burden of managing the deprecation being at the side of the user. I'm just curious at that point)


And a very low-level thing, that I'd fix in master after the merge (i.e. you don't have to care about that now) : The nativeParam_name should be declared

cl_gl_platform_info nativeParam_name = 0;

instead of

long nativeParam_name = 0;

i.e. the "Native variables declaration" should always have the type that is required for the function (any casts to the "right" type happen in the "Obtain native variable values" part).


But in general, I'd merge this soon. Could you provide the MacOS binaries for the release after the merge? And @blueberry : Would you be able to provide the Linux ones? (If not, I can compile them in a VM, but having the tests run on an actual machine always brings a tad more confidence...)

LlemonDuck commented 3 years ago

the function is declared as CL_EXT_SUFFIX__VERSION_1_0 CL_DEPRECATED(10.6, 10.14);. The tech note that you linked to says that it's from "Late 2013". I wondered whether there may be some newer functionality

Apple deprecated all of OpenCL in macOS 10.14. The "alternative" is to use Metal, unfortunately.

I can provide both Linux and macOS binaries.

libJOCL-natives-2-0-3.zip

gpu commented 3 years ago

Thanks @LlemonDuck for the binaries!

There has been another PR in the meantime. (I wonder where this sudden attention for JOCL comes from - may this be related to https://blogs.oracle.com/javamagazine/programming-the-gpu-in-java ?). If it's not too much effort, I'd create the release based on this new state. Otherwise, I'll create the 2.0.3 release based on the (then current) state of master, with your extension, and using your binaries.

gpu commented 3 years ago

I'll create the release based on the state after your PR. The changes from the other PR are not affecting the API in that regard, and will thus be part of the next release then.