I made a few fixes to JOCL - how do I contribute them back?

Hello,

I was using JOCL for a project that is already running on JOGL and ran into a few issues. After much investigation, I found that a few of those issues were caused by bugs in the JOCL library.

These are simple bugs - 3 cases of arguments passed in the wrong order and 1 case of an unsupported define necessary for GL context sharing in Mac OS X. In the case of the missing int define, I just passed a hard-coded integer, since I don't know if I can put it in the CL.java file (as it appears to be generated during the build process).

My fixes are simple and crude, but I need them for my project to work correctly (and to work on Mac). Since more people might have been affected by these bugs, I assume that contributing these fixes back could help others. Is that the case? If so, how should I proceed?

Hello,

I was using JOCL for a project that is already running on JOGL and ran into a few issues. After much investigation, I found that a few of those issues were caused by bugs in the JOCL library.

These are simple bugs - 3 cases of arguments passed in the wrong order and 1 case of an unsupported define necessary for GL context sharing in Mac OS X. In the case of the missing int define, I just passed a hard-coded integer, since I don't know if I can put it in the CL.java file (as it appears to be generated during the build process).

My fixes are simple and crude, but I need them for my project to work correctly (and to work on Mac). Since more people might have been affected by these bugs, I assume that contributing these fixes back could help others. Is that the case? If so, how should I proceed?

---

If you reply to this email, your message will be added to the discussion below:

http://forum.jogamp.org/I-made-a-few-fixes-to-JOCL-how-do-I-contribute-them-back-tp4029976.html

To start a new topic under jocl, email [hidden email]
To unsubscribe from jogamp, click here.
NAML

You may start with sending a patch of each file you changed to this list. This allows us to understand the problem better.

You may also open a bugreport for each issue you observe using JOCL.
This is the best way to report issues and make sure we fix them in futher releases.
https://jogamp.org/bugzilla/

If you have a github account clone the JogAmp/jocl git https://github.com/JogAmp/jocl and send a pull request with your changes.

Cheers
Xerxes

Thank you for your answer.

I followed your instructions, creating an account on Bugzilla and GitHub.

On Bugzilla, the bugs related to this are #553 (posted previously by someone else), #824 and #825. I believe the bug reports explain the issues; let me know if you need more information.

On GitHub, this is the pull request.

Attached, here, is the .patch file I generated: fixes.patch.

Let me know if there is anything else I can do to help.

The patch looks very reasonable to me, I'm also not sure what should be done with the missing defined constant, if
you remove that single line to its own commit, I'd suggest at least the argument-orderig fixes should be pulled.

Sven, your call...but seems to do what it says.

Harvey

I'm not sure that one of your changes will work on all supported platforms, why do you need to use CL_CONTEXT_PROPERTY_USE_CGL_SHAREGROUP_APPLE? The rest is ok, unit tests are more than welcome.

Hello,

I do not know either if the changes to CL context creation on Mac OS X will work on all supported platforms - they should only affect Mac OS X, and I am only able to test it on 10.8.4.

As far as I have been able to understand it, CL_CONTEXT_PROPERTY_USE_CGL_SHAREGROUP_APPLE is the correct way to specify the GL sharing group ID on Apple platforms (at least as of 10.8.4). Using CL_CGL_SHAREGROUP_KHR, as the previous code did, simply causes the context creation to throw a CL_INVALID_VALUE error on Mac OS X 10.8.4.

I've read that this constant is only defined on Apple's version of the "cl_gl_ext.h" file. I have digged around to check for this and I found this comment: "Introduced in Mac OS X 10.6". It is mentioned in OpenCL's Wikipedia entry "OpenCL 1.0 has been released with Mac OS X Snow Leopard". Snow Leopard IS 10.6, so apparently the CL_CONTEXT_PROPERTY_USE_CGL_SHAREGROUP_APPLE constant was added at the same time that OpenCL became supported on Mac OS X... I'd find it safe to assume that using this constant is the correct way to create CLGL contexts on Mac platforms where OpenCL is supported.

As for unit tests, I'll admit to my limitations and say that I have no idea of what those are and how they work.

If you changed that commit to not include the constant changes, I'm sure that would get pulled quickly
and then the other part can get fixed separately.

Out of curiosity, what are you using JOCL for?

Harvey

I am not very proficient with Git and never did a pull request before, so I do not know how to actually change a commit already on the repository. If I knew beforehand that this would be an issue, I would've committed those changes separately. Live and learn, I guess.

Any tips on how do split the changes? Easiest way I can think of is deleting the repository and re-forking so I can do separate commits.

As for my JOCL usage, I'm using it to run custom image processing algorithms on 3D textures inside the video card so I can get better speed and save on memory usage (since the textures are already in the video card for rendering at a later point). Still in the prototyping stages, but looks promising so far.

If you redo the commits, you can then push them again using the --force option to rewrite the branch you
have already created on you github repo.

Pop by the freenode #jogamp channel if you'd like any assistance.

Harvey

Hi

I've found this article concerning this trouble:
http://oscarbg.blogspot.fr/2009/10/about-opencl-opengl-interop.html

Yes you're right, this change only affects Mac OS X and it seems ok since Mac OS X 10.6. Sorry for the confusion. However, unit tests are still welcome as only a very few people here have good skills in OpenCL and it would be nice to modify JOCL safely. Maybe an existing test could be used.

Unfortunately I don't have the skills or knowledge necessary to help create unit tests, so I apologize if my contribution is limited.

Ok, don't be sorry, thanks for your contributions but have you run existing tests?

I don't think I have; like I said, I don't know what those are or how can I use them.

On 09/06/2013 06:45 PM, lixoman100 [via jogamp] wrote:
> I don't think I have; like I said, I don't know what those are or how can I
> use them.
>

Thank you LixoMan .. merged, refined a bit and pushed.

See Bug 553, Bug 824 and Bug 825.

~Sven

Thank you! Seems to be fine.

Thank you both Lixoman and Sven, those bugs were there for ages.