AFAIK, CG is written by hand. The change looks correct, is there reference or spec that backs this up? I'd to cross-check changes before commiting

------
OpenTK

Some notes:
-- in terms of reference I am not sure what you require. But compare to the NVidia CG Reference if you will.

-- Interestingly, the C# comments all list the correct function signatures - they are just not implemented correctly in many cases

-- Replacing all out occurrences (as I suggested above) is definitely not correct. There are various cgGetXXX functions that should return values using the out mechanism (we must still make sure that the array specifier [] is used correctly)

-- Essentially someone should go through the code by hand and figure out which signatures have been translated incorrectly. There are at least a handful. The Comment signature should be ok as reference.

-- UPDATE: ok, I've gone through the file. You can get it here: http://download.yousendit.com/1965078851DB6237 Someone should still sanity check it!

Many thanks for your effort! I've checked the updates, and they look good, but for a couple of small problems:

1. 
- public static string cgGetString(int sname)
- {
- return Marshal.PtrToStringAnsi(_cgGetString(sname));
- }
-
- [DllImport(...)]
- private static extern IntPtr _cgGetString(int sname);
+ [DllImport(...)]
+ public static extern string cgGetString(int sname);

   You cannot return a managed string directly from a DllImport, you have to marshal it through Marshal.PtrToStringAnsi(). The reason is that the return value is a pointer to unmanaged memory, not a managed string object!

2. Erroneous "out" parameters should be corrected by adding both a "ref" and an array overload (this way they will be much more usable).

Could you make a small effort and make these changes, now that they are still fresh on your mind? We cannot commit the file as long as #1 is not taken care of (and you shouldn't use it either, as wrong handling of unmanaged memory can cause nasty crashes). #2 is also important, but probably not a blocking issue.

------
OpenTK

To #1:
I never touched the string functions, so if those are incorrect somebody should look at that independently (I am fairly certain that I am using the latest release version, but please check)

To #2:
This is an interesting one. I am kinda new to .NET, so I don't have much experience with Marshalling. I took inspiration from CgGl.cs, where I found (e.g.):

public static extern void cgGLGetParameter1f(IntPtr param, [Out] float[] values);

Note, that this includes the [Out] Marshalling directive, but omits any of the ref/out directives. That's why I assumed they weren't necessary.

The way CG works, it expects a correctly-sized array that it can place its values in, i.e. it will NOT create the array for you and pass that back out (this is my understanding and it makes sense from a malloc point of view). Now, these are the options for arrays:

1.) You would use 'out' if the called function creates a new array and returned that via a parameter. The called function would not be allowed to access elements in the array before creating the array itself (as mentioned above, this is NOT what is happening)

2.) You would use 'ref' similarly, but in this case the array has to be initialized by the calling function. You can then use the array elements in the function, but also send out a new array that you created.

3.) Last, as arrays are passed by reference anyway (as in: they are not copied), the called function is allowed to change the values in the array and these changes will be reflected in the calling function. A new array assignment in the called function would NOT be reflected in the calling function, as nothing is passed out via the parameter itself.

Both option 2&3 are viable in our case, because only values are changed, so there is no real benefit to using the 'ref' keyword and it just presents a hassle in my opinion. The CsGl code also seems to take that approach.

If all that is correct, then #2 shouldn't be a problem and somebody just needs to fix #1.

Over and [Out]

I was writing a lengthy reply, but the browser crashed (don't you hate this?)

[#1: string functions]
My mistake, I thought the file was against SVN. I'll re-create the patch against the actual release.

[#2: ref/out and arrays]
I suggested adding both because the opengl api contains functions like glGenBuffers, where it's clunky to allocate a whole array just to get 1 value out.

The cg api is more sane in this regard (cgSetParameter1fv/cgSetParameter1f), so we can go ahead and use arrays only.

Last, the [In], [Out] and [In,Out] attributes indicate the direction of data marshaling, i.e. they are just an optimization. They are different from ref/out which indicate the method of parameter passing (by value or by reference).

All in all, the patch is fine as is. I'll recreate the patch against the correct version and upload it here for review. If we don't find any problems, we'll commit it for the next version.

------
OpenTK