-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for specification overloads #107
Conversation
Thanks Christian, this is great. Apologies for not having the opportunity to review this sooner. I agree that offering explicit overrides/extensions is the right approach to manage the fact that a single OpenGL function has two effectively different signatures. One thought: As an alternative to explicit overrides, what if we offered an additional XML file that introduced a new namespace, so we'd have something like |
Thank you for the reply. (I still need to update this PR with the proper suffix for the one example.) Also, you use the term "override" while I took "overload". I specifically chose "overload" as the new function does not hide the original and at least one parameter is different - two properties that "overriding" does not have. |
Makes sense, thanks for the explanation. Your change largely LGTM but I have a few nits. |
// An Overload describes an alternative signature for the same function. | ||
type Overload struct { | ||
Name string // C name of the original function | ||
GoName string // Go name of the original function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this value be pulled from the parent struct? similarly for Name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name
turned out not to be necessary. As for GoName
, I was not able to figure out an easy way to access GoName
from the parent from the sub-template overloadCall
. Hence I kept this field
Comments handled. |
Thank, this looks great. I have one very tiny documentation change to request, and otherwise I'm happy to merge! |
Thanks again! |
Sure thing - thank you! |
This pull request is a proposal for the solution of current problems with
checkptr
and theunsafe.Pointer
/uintptr
converions. There are several issues across several projects already - I suppose go-gl/gl#80 is the most central one.With this code change, it is possible to have (manually written) XML files under
<xmlDir>/overload
with same name as the main spec files. So, forxml/spec/gl.xml
the filexml/overload/gl.xml
would be matched.I included a sample overload file, to showcase how this is to be used:
gl.VertexAttribPointer(..., unsafe.Pointer)
it creates agl.VertexAttribOffset(..., uintptr)
overloadgl.GetVertexAttribPointerv(..., *unsafe.Pointer)
it creates agl.GetVertexAttribOffsetv(..., **uintptr)
overloadgl.DrawElements(..., unsafe.Pointer)
it creates agl.DrawElementsWithOffset(..., uintptr)
overloadThe created code looks like this - example for
VertexAttribPointer
:I wanted to stay clear of the core specification files, which is why a separate set of files is used.
I also don't know of all the affected cases. My guess (hope) would be, that, once accepted, people provide corresponding overloads via dedicated pull requests. Perhaps it's also a small subset of affected functions. Return types are not handled (are there cases?)
Also not clear is the naming. Should all the overloads have a suffix
WithOffset
? In the special case ofVertexAttribPointer
for instance, the WebGL variant even calls itVertexAttribIPointer
(Note the extraI
- also, their overload has one parameter less, something that could also be replicated...)For all these uncertainties, I propose to continue the discussion in go-gl/gl#80 .